Resolve open_basedir paths on ini update #10987
Conversation
a7ac8a1 to
d1cc4b5
Compare
| *p = ZSTR_VAL(new_value); | ||
| return SUCCESS; | ||
| } | ||
|
|
||
| /* Shortcut: When we have a open_basedir and someone tries to unset, we know it'll fail */ |
There was a problem hiding this comment.
Not pointing to this comment.
But the comment above: PHPAPI ZEND_INI_MH(OnUpdateBaseDir) says:
Allows any change to open_basedir setting in during Startup and Shutdown events,
or a tightening during activation/runtime/deactivation
However, we are updating the INI setting without any checks in:
if (stage == PHP_INI_STAGE_STARTUP || stage == PHP_INI_STAGE_SHUTDOWN || stage == PHP_INI_STAGE_ACTIVATE || stage == PHP_INI_STAGE_DEACTIVATE)
So this is confusing, wondering what the actual php.net docs say as they may also be misleading.
There was a problem hiding this comment.
I think it's more just a shortcut in the code as it is clear this will fail the check so we don't bother to do the actual check. Pretty much what it says so not sure what's confusing about this?
d1cc4b5 to
f2e095d
Compare
f2e095d to
0becb9f
Compare
|
@bukka Could you have another look? |
|
@bukka If we still want to adjust this behavior we should do very soon 🙂 Do you prefer this approach over what is in |
There was a problem hiding this comment.
@iluuu1994 I just went through and re-read our previous comms and it looks good to me.
main/php.h
Outdated
| @@ -295,7 +295,7 @@ ssize_t pread(int, void *, size_t, off64_t); | |||
| #endif | |||
|
|
|||
| BEGIN_EXTERN_C() | |||
| void phperror(char *error); | |||
There was a problem hiding this comment.
I don't think it was, must've sneaked in. I'll revert.
| *p = ZSTR_VAL(new_value); | ||
| return SUCCESS; | ||
| } | ||
|
|
||
| /* Shortcut: When we have a open_basedir and someone tries to unset, we know it'll fail */ |
There was a problem hiding this comment.
I think it's more just a shortcut in the code as it is clear this will fail the check so we don't bother to do the actual check. Pretty much what it says so not sure what's confusing about this?
|
@bukka Great, thanks for the review! I'll merge this soon then. |
0becb9f to
82e5ca6
Compare
82e5ca6 to
804aa29
Compare
The whole ini handling is quite messy... Not sure if there's a way to improve it.