Changeset View
Changeset View
Standalone View
Standalone View
support/startup/preamble-utils.php
| Show All 10 Lines | function preamble_trust_x_forwarded_for_header($layers = 1) { | ||||||||
| if (!is_int($layers) || ($layers < 1)) { | if (!is_int($layers) || ($layers < 1)) { | ||||||||
| echo | echo | ||||||||
| 'preamble_trust_x_forwarded_for_header(<layers>): '. | 'preamble_trust_x_forwarded_for_header(<layers>): '. | ||||||||
| '"layers" parameter must an integer larger than 0.'."\n"; | '"layers" parameter must an integer larger than 0.'."\n"; | ||||||||
| echo "\n"; | echo "\n"; | ||||||||
| exit(1); | exit(1); | ||||||||
| } | } | ||||||||
| if (!isset($_SERVER['HTTP_X_FORWARDED_FOR'])) { | if (!isset($_SERVER['HTTP_X_FORWARDED_FOR'])) { | ||||||||
valerio.bozzolan: Yeah the NULL check is already done. | |||||||||
| return; | return; | ||||||||
| } | } | ||||||||
| $forwarded_for = $_SERVER['HTTP_X_FORWARDED_FOR']; | $forwarded_for = $_SERVER['HTTP_X_FORWARDED_FOR']; | ||||||||
| if (!phutil_nonempty_string($forwarded_for)) { | if (!strlen($forwarded_for)) { | ||||||||
Not Done Inline ActionsTo reduce count of double-negatives, let's add phutil_is_empty_string() function. avivey: To reduce count of double-negatives, let's add `phutil_is_empty_string()` function. | |||||||||
Not Done Inline Actionsactually, it might end up more confusing with NULL inputs. avivey: actually, it might end up more confusing with `NULL` inputs. | |||||||||
Not Done Inline Actions
I suggest a strict check there. I also strongly would like to push this that is what we exactly want, so, exclude NULL, have a string, and have it non-empty: if(is_string($forwarded_for) && $forwarded_for !== '') {valerio.bozzolan: I suggest a strict check there.
I also strongly would like to push this that is what we… | |||||||||
Not Done Inline ActionsOuch note that both the patch and my suggestions are incorrect. The correct way is the total reverse, so: if ($forwarded_for === null || !strlen($forwarded_for)) {Or, what I would like to use: if(!is_string($forwarded_for) || $forwarded_for === '') {valerio.bozzolan: Ouch note that both the patch and my suggestions are incorrect.
The correct way is the total… | |||||||||
| return; | return; | ||||||||
| } | } | ||||||||
| $address = preamble_get_x_forwarded_for_address($forwarded_for, $layers); | $address = preamble_get_x_forwarded_for_address($forwarded_for, $layers); | ||||||||
| $_SERVER['REMOTE_ADDR'] = $address; | $_SERVER['REMOTE_ADDR'] = $address; | ||||||||
| } | } | ||||||||
| ▲ Show 20 Lines • Show All 45 Lines • Show Last 20 Lines | |||||||||
Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under Apache 2.0 or other open source licenses. · CC BY-SA 4.0 · Apache 2.0
Yeah the NULL check is already done.