mirror of
https://github.com/cypht-org/cypht.git
synced 2026-04-25 04:56:03 +03:00
[GH-ISSUE #304] Drop scripts/config_gen.php #263
Labels
No labels
2fa
I18N
PGP
Security
Security
account
advanced_search
advanced_search
announcement
api_login
authentication
awaiting feedback
blocker
bug
bug
bug
calendar
config
contacts
core
core
devops
docker
docs
duplicate
dynamic_login
enhancement
epic
feature
feeds
framework
github
github
gmail_contacts
good first issue
help wanted
history
history
imap
imap_folders
inline_message
installation
keyboard_shortcuts
keyboard_shortcuts
ldap_contacts
mobile
need-ssh-access
new module set
nux
pop3
profiles
pull-request
question
refactor
release
research
saved_searches
smtp
strategic
tags
tests
themes
website
wordpress
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/cypht#263
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @iredmail on GitHub (Nov 20, 2018).
Original GitHub issue: https://github.com/cypht-org/cypht/issues/304
Originally assigned to: @jasonmunro on GitHub.
Dear Jason,
I checked what
scripts/config_gen.phpdoes. Since i have more experience with Roundcube, i’d like to compare cypht with Roundcube, and share my thoughts below.I understand dropping
scripts/config_gen.phpmay leads to big changes in other php files, but i believe it's better drop it as early as possible, and use php file as config file directly.check_php(): Check PHP for correct supportparse_module_ini_files(): include module ini files in the main configRoundcube uses php file as config file, this is very easy for solution integrator and sysadmin:
config_gen.phpto generate a final config file. This saves admin's time, but most importantly, it doesn’t require admin to remember extra procedure to update a parameter. The easier the better, the simpler the better.require_onceat the end of main config file. as a solution provider, we pre-define some settings in main config file, also has arequire_oncedirective at the bottom of main config file to load admin’s custom config file. this way we can achieve fearless upgrade.build_config(): Entry point into the configuration processMaybe move PHP function existence checks to
check_php()? But my purpose is dropping this whole file directly. hahasite settings
Why not simply check and load extra config file like
<site>.ini(what i really mean is modular config file<site>.inc.php)?Benefits of config file generated by
config_gen.php?config_gen.php? better performance or any advantages?Roundcube uses php files as config files directly, i didn't see any performance issue at all.
@jasonmunro commented on GitHub (Nov 20, 2018):
@iheinrich I appreciate the feedback and your thoughts on this. You are not the only one who does not like the config_gen script :) It would be a very large change to try to remove it, and there would be a performance penalty. The most important thing it does is build a map of modules to page ids, so it calculates a map of assigned modules to the request identifier and stores that in the generated output. Removing this step would require calculating this on each request which I would very much prefer not to do. It's also a large data structure that would be difficult to hard code into a php config file.
Another thing the script does is prepare the production site and combine and minify browser assets like javascript and css includes. I can use cypht on my phone over 3G and the performance is excellent. The entire Cypht app (depending on what modules you have activated and what options enabled) is on average less than 50KB to the browser with only a single js and single css include.
One of Cypht's design goals is to be lightweight on the server and the client, and the config gen script is an important part of what makes that happen. As I said, less than 50K sent to the browser on a typical page load, and on the server a Cypht request on average uses less than 3 MB of memory. I understand this might not be important for your use case(s), but it is for mine :) I'm not saying there could not be a better way, and if someone is interested in doing the work and submitting a PR I would definitely consider it, however at this time I am not going to remove the script.
I usually deploy the latest Cypht code several times a week and it's not hard to automate away the steps to do so in an update.sh type script such that I only have to run a single command to do the update.
@ulfgebhardt commented on GitHub (Nov 21, 2018):
Regarding minifying css and js files:
I do the following:
This way the minify process takes place only once and on the fly, but generates an overhead for the collecting, identifying of files and hashing of the resulting string.
@zeigerpuppy commented on GitHub (Nov 26, 2018):
Thanks @jasonmunro for the explanation. The main advantage that I see in cypht over other webclients is it's speed and consistency in the UI. The handling of folders is also excellent.
As a sysadmin, I don't find the build script to be such a big issue; deployment usually involves a script or two anyway.
I'm also a huge fan of the use of a build process if it means a smaller attack surface (I would hazard a wager that cypht is more secure-by-design than roundcube!).
@jasonmunro commented on GitHub (Nov 27, 2018):
@zeigerpuppy thanks for the feedback. You are correct that the build process reduces the attack surface as well. With the exception of assets required by module sets (only 2 come to mind, the SMTP set for HTML and Markdown formatting, and the themes css files), only 3 files are inside the document root when running Cypht in production mode: the auto-generated index.php file, and the combined site.js and site.css files.
We don't use any writable directories in the document root as that gives attackers a handy target for code injection (I'm honestly surprised by how many web apps allow this), but we also get some extra protection by keeping 99% of the php code outside the document root. I have seen many exploits over the years that started by loading a php file directly in the browser that was not intended to be used that way.
@jasonmunro commented on GitHub (Feb 13, 2019):
I appreciate this feedback, and completely understand the reasoning behind the request, but at this time I am not going to remove the build process. Thanks to everyone for the feedback. I'm going to close this issue as a part of just general housekeeping.