[Discussion] Dots and spaces in GPC variable names

php.internals

Andrey Andreev

9 years ago
Hi all, With parse_str() usage without a second parameter being deprecated, I was looking to possibly drop the behavior where it replaces spaces and dots with underscores in the result array, and ... As it often turns out - it's not that simple, because it re-uses the code that handles GPC vars. I didn't even know this mangling happened with GPC vars. I can only assume that's legacy from the the register_global days, as I certainly don't see a reason for it now. Am I missing something? And more importantly, can we change that? It's a significant BC break, yes, but the current behavior is horribly broken IMO. Cheers, Andrey.

Christoph Becker

9 years ago
On 02.05.2017 at 12:56, Andrey Andreev wrote:
> With parse_str() usage without a second parameter being deprecated, I > was looking to possibly drop the behavior where it replaces spaces and > dots with underscores in the result array, and ... As it often turns > out - it's not that simple, because it re-uses the code that handles > GPC vars. > > I didn't even know this mangling happened with GPC vars. I can only > assume that's legacy from the the register_global days, as I certainly > don't see a reason for it now. Am I missing something? > > And more importantly, can we change that? It's a significant BC break, > yes, but the current behavior is horribly broken IMO.
This issue has been already brought to the list quite a while ago[1]. IMHO, the big problem is the subtle BC break. [1] <http://marc.info/?l=php-internals&m=144416398306733&w=2>
-- Christoph M. Becker

Andrey Andreev

9 years ago
Hi, On Tue, May 2, 2017 at 5:07 PM, Christoph M. Becker <cmbecker69@gmx.de> wrote:
> On 02.05.2017 at 12:56, Andrey Andreev wrote: > >> With parse_str() usage without a second parameter being deprecated, I >> was looking to possibly drop the behavior where it replaces spaces and >> dots with underscores in the result array, and ... As it often turns >> out - it's not that simple, because it re-uses the code that handles >> GPC vars. >> >> I didn't even know this mangling happened with GPC vars. I can only >> assume that's legacy from the the register_global days, as I certainly >> don't see a reason for it now. Am I missing something? >> >> And more importantly, can we change that? It's a significant BC break, >> yes, but the current behavior is horribly broken IMO. > > This issue has been already brought to the list quite a while ago[1]. > IMHO, the big problem is the subtle BC break. > > [1] <http://marc.info/?l=php-internals&m=144416398306733&w=2> >
Thanks for linking that. It's encouraging as I'd expect a previous discussion to have gathered mostly negative opinions due to the BC break. That old thread is from just the time when 7.0 was supposed to be released, so it was certainly an opportunity missed by a few months. I'm not surprised by the suggestion for targeting 8.0, but perhaps we could think of a way to conditionally disable the mangling in 7.x and then remove it entirely in 8? I hate to say INI directives, but that's surely one way for a smoother transition. Cheers, Andrey.

Andrew Faulds

9 years ago
Hey Andrey, Andrey Andreev wrote:
>I'm not surprised by the suggestion for targeting 8.0, but perhaps we > could think of a way to conditionally disable the mangling in 7.x and > then remove it entirely in 8? > I hate to say INI directives, but that's surely one way for a smoother > transition.
Another approach I'd previously thought of was to keep in memory whatever information would be needed to undo the mangling process for the superglobals, and provide a function which would undo it. Then in 8.x it could become a no-op. That wouldn't really help for parse_str, though. But for that function it could just be a flag. Thanks for bringing this up again!
-- Andrea Faulds https://ajf.me/

Andrey Andreev

9 years ago
Hi Andrea, On Tue, May 2, 2017 at 11:08 PM, Andrea Faulds <ajf@ajf.me> wrote:
> Hey Andrey, > > Andrey Andreev wrote: >> >> I'm not surprised by the suggestion for targeting 8.0, but perhaps we >> could think of a way to conditionally disable the mangling in 7.x and >> then remove it entirely in 8? >> I hate to say INI directives, but that's surely one way for a smoother >> transition. > > > Another approach I'd previously thought of was to keep in memory whatever > information would be needed to undo the mangling process for the > superglobals, and provide a function which would undo it. Then in 8.x it > could become a no-op. >
Sounds hacky to me. Plus, a function call means we can't have environments that do it by default pre-8.0, and that in turn means a harder migration. But hey, as long as we get rid of the mangling, I wouldn't really care that much! :) Cheers, Andrey.

Lauri Kenttä

9 years ago
On 2017-05-03 13:55, Andrey Andreev wrote:
> Hi Andrea, > > On Tue, May 2, 2017 at 11:08 PM, Andrea Faulds <ajf@ajf.me> wrote: >> Hey Andrey, >> >> Andrey Andreev wrote: >>> >>> I'm not surprised by the suggestion for targeting 8.0, but perhaps we >>> could think of a way to conditionally disable the mangling in 7.x and >>> then remove it entirely in 8? >>> I hate to say INI directives, but that's surely one way for a >>> smoother >>> transition. >> >> >> Another approach I'd previously thought of was to keep in memory >> whatever >> information would be needed to undo the mangling process for the >> superglobals, and provide a function which would undo it. Then in 8.x >> it >> could become a no-op. >> > > Sounds hacky to me. Plus, a function call means we can't have > environments that do it by default pre-8.0, and that in turn means a > harder migration. > But hey, as long as we get rid of the mangling, I wouldn't really care > that much! :) > > Cheers, > Andrey.
Someone has to say this ;), so: Why not a new API? You could provide access to parameters with same name instead of overwriting the existing; consider a=0&a[]=1&a[0]=2&a=3, where each part overwrites the previous one and the result is a=3. You could provide support for JSON input. You could create all kinds of crazy bloat.
-- Lauri Kenttä

Sara Golemon

9 years ago
On Tue, May 2, 2017 at 5:56 AM, Andrey Andreev <narf@devilix.net> wrote:
> With parse_str() usage without a second parameter being deprecated, I > was looking to possibly drop the behavior where it replaces spaces and > dots with underscores in the result array, and ... As it often turns > out - it's not that simple, because it re-uses the code that handles > GPC vars. > > I didn't even know this mangling happened with GPC vars. I can only > assume that's legacy from the the register_global days, as I certainly > don't see a reason for it now. Am I missing something? > > And more importantly, can we change that? It's a significant BC break, > yes, but the current behavior is horribly broken IMO. >
BC breaking the default handling of GPCSE vars aside, I'd be 100% in favor of either adding a flag param to parse_str() for this. parse_str($_SERVER['QUERY_STRING'], $_GET, PARSE_STR_NOMANGLE); -Sara ((Or add a new function which doesn't mangle and returns by value rather than by ref because *ugh* at that by-ref semantic))