Fixes T8850. Previously, if a user's preamble script mangled $_SERVER['REMOTE_ADDR'] or somehow set it to null, the user would get errors when performing certain actions. Now those errors shouldn't occur, and instead the user will be warned that there is a setup issue related to their preamble script.
Details
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T8850: Add a setup warning to make sure users haven't wiped out $_SERVER['REMOTE_ADDR']
- Commits
- rPa88dc2afc2a9: Added a setup check for empty REMOTE_ADDR
Create a preamble script that contains $_SERVER['REMOTE_ADDR'] = null; then navigate to /config/issue/. There should be a warning there about REMOTE_ADDR not being available.
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| src/applications/config/check/PhabricatorPHPConfigSetupCheck.php | ||
|---|---|---|
| 212–221 | This sort of fragment ("%s and double-check") may be difficult to translate into other languages, since they may want to change word order or need to adjust articles depending on noun gender. A rephrasing that uses fewer fragments, maybe like this one, may be easier to translate in a natural-sounding way:
Where the replacement is just a link titled "Configuring a Preamble Script". (This is maybe-possibly slightly better for assistive technology / screenreader stuff too, since it makes "read all links on this page" more meaningful, although probably not too substantively.) | |
| 219 | Just omit 'rel' -- we automatically manage it (currently, by adding 'noreferrer' to non-local links) and specifying it will override whatever behavior we're implementing. (It's possible that we should change the behavior, e.g., to include noopener, although the use-window.opener-to-do-phishing-stuff attack doesn't currently seem hugely concerning to me. If you want to make an argument for changing the defaults, feel free to file a task.) | |