While doing a test translation run on the generic Drupal strings (which are not extracted well still), I found that PHP module has some outdated and insecure explanations on how one should do PHP code.
- "register_globals is now set to off" suggested that this is a recent change, while it is not
- the help text suggested using _GET and _POST directly for form data handling, which is insecure so I replaced it with a suggestion to look at the Form API
- direct usage of the username is better replaced with using a placeholder (although the username can only contain a restricted amount of chars, this is better for education purposes also)
- how the actual example block is added was not explained
Patch attached, please review. "Unfortunately" I was unable to resist the temptation to break apart this big block of text to smaller chunks, just as it is done at every other help text, so the patch is actually bigger then the actual number of textual changes.
Comment | File | Size | Author |
---|---|---|---|
#9 | php_input_filter_2.patch | 6.03 KB | keith.smith |
#7 | php_input_filter_1.patch | 6 KB | keith.smith |
#5 | php_input_filter.patch | 5.88 KB | keith.smith |
#1 | php.help_.security.patch | 5.15 KB | Gábor Hojtsy |
php.help_.security.patch | 5.16 KB | Gábor Hojtsy | |
Comments
Comment #1
Gábor HojtsyDoh, just realized a coding style error introduced with my patch and a mention of "sidebar boxes" at the end from the original text, which is obviously not a correct term anymore ("sidebar blocks" would not be correct either). Fixed these.
Comment #2
Gerhard Killesreiter CreditAttribution: Gerhard Killesreiter commented" It will be executed when the page is viewed and dynamically embedded into the page. "
Should probably be:
" It will be executed when the page is viewed and any output dynamically embedded into the page. "
Also, let me state that I really dislike PHP blocks/pages. They have on occassion been the cause of a failed Drupal upgrade and are hard to track don if you didn't set up the site yourself.
I usually recommend that people put the PHP code into a function in template.php or (better) a site specific module. If you go with a module you can then directly crate a block hook and save yourslf some troubles.
Maybe this advice could be integrated.
Comment #3
keith.smith CreditAttribution: keith.smith commentedThere is some help text that has recently been committed for other parts of the PHP module that can be reused here, for consistency's sake. I'll post an updated patch later today; assigning to myself in the interim.
Comment #4
Gábor HojtsyI also noticed that the PHP module removal notice is actually a bit misleading. Depending on the default input format assigned to the nodes once the PHP module got removed, the PHP code might get visible or might get stripped (ie. could result in empty nodes). Also not all content submitted with the PHP input format are affected but only those which actually used PHP code, which might be a nitpick but still.
Comment #5
keith.smith CreditAttribution: keith.smith commentedAttached is a patch that reuses some of the language in the PHP Filter help page committed earlier, but attempts to communicate all of the items found in the previous patch (#1), along with the comments in #2 and #4.
:) Please review.
Comment #6
cburschkaCould the relevant text link to the Form API on api.drupal.org? Given that the users who read this are likely to have never been to api.drupal.org before, this would save quite some effort in finding the API.
Comment #7
keith.smith CreditAttribution: keith.smith commentedGood idea. I had intended to do that but zoned out apparently, instead.
The attached patch contains a link to http://api.drupal.org/api/group/form/6; is this the link you had in mind?
Comment #8
cburschkaYeah, that's probably the best place to start for the form API. Well, I have nothing more to add; the patch looks good now.
Comment #9
keith.smith CreditAttribution: keith.smith commentedChanged a
the site
toyour site
. Otherwise, exactly the same.Comment #10
Dries CreditAttribution: Dries commentedI read the changes and I agree that they are much-needed. It's sort of bizarre that this went unnoticed that long. I guess no one reads documentation (neither do I ;)).
I think this is RTBC so I committed it to CVS HEAD. Thanks folks.
Comment #11
Gábor HojtsyDries, translators read these texts and even translate them. Some of them have no knowledge of Drupal programming or realize too late that such changes need to be made, as they do translation after release.
Comment #12
(not verified) CreditAttribution: commentedAutomatically closed -- issue fixed for two weeks with no activity.