Adieu register_globals

php.internals

Pierre Joye

20 years ago
Hello, As discussed last year, register_globals will be removed in php6. This patch is the first step: http://pear.php.net/~pierre/remove_register_globals.txt The only thing I find confusing is a comment in main/php_variables.c php_autoglobal_merge. The comment says something and the tests something else (register_globals AND instead of register_globals OR). I will review all the affected tests if you consider this patch ok to be commited. Some will be updated, other simply removed. I did not run benchmarks, but I feel like it speeds up php a bit :) Comments and feedbacks welcome, --Pierre

Marcus Börger

20 years ago
Hello Pierre, Sunday, March 5, 2006, 4:21:28 PM, you wrote:
> Hello,
> As discussed last year, register_globals will be removed in php6. This > patch is the first step:
> http://pear.php.net/~pierre/remove_register_globals.txt
> The only thing I find confusing is a comment in main/php_variables.c > php_autoglobal_merge. The comment says something and the tests > something else (register_globals AND instead of register_globals OR). I > will review all the affected tests if you consider this patch ok to be > commited. Some will be updated, other simply removed.
> I did not run benchmarks, but I feel like it speeds up php a bit :)
I see the same.
> Comments and feedbacks welcome,
"Wipe them out - *all* of them" just as discussed. But take care of the tests first. Best regards, Marcus

Pierre Joye

20 years ago
On Sun, 5 Mar 2006 16:21:28 +0100 pierre.php@gmail.com (Pierre) wrote:
> Hello, > > As discussed last year, register_globals will be removed in php6. This > patch is the first step: > > http://pear.php.net/~pierre/remove_register_globals.txt > > The only thing I find confusing is a comment in main/php_variables.c > php_autoglobal_merge. The comment says something and the tests > something else (register_globals AND instead of register_globals OR). > I will review all the affected tests if you consider this patch ok to > be commited. Some will be updated, other simply removed. > > I did not run benchmarks, but I feel like it speeds up php a bit :) > > Comments and feedbacks welcome,
Still welcome... The patch includes now: - the all tests fixes (session) * tests being useless now are skiped (to allow review, will removed later) - functions removed: * session_register * session_unregister * session_is_registered They need register_globals to work I will wait until wednesday for feedbacks or objections, and commit on thursday. --Pierre

Marcus Börger

20 years ago
Hello Pierre, looks even better becuase it includes the next two steps after your first patch. If it was up to me, i'd say just apply. marcus Monday, March 6, 2006, 9:46:19 PM, you wrote:
> On Sun, 5 Mar 2006 16:21:28 +0100 > pierre.php@gmail.com (Pierre) wrote:
>> Hello, >> >> As discussed last year, register_globals will be removed in php6. This >> patch is the first step: >> >> http://pear.php.net/~pierre/remove_register_globals.txt >> >> The only thing I find confusing is a comment in main/php_variables.c >> php_autoglobal_merge. The comment says something and the tests >> something else (register_globals AND instead of register_globals OR). >> I will review all the affected tests if you consider this patch ok to >> be commited. Some will be updated, other simply removed. >> >> I did not run benchmarks, but I feel like it speeds up php a bit :) >> >> Comments and feedbacks welcome,
> Still welcome...
> The patch includes now: > - the all tests fixes (session) > * tests being useless now are skiped (to allow review, will removed > later) > - functions removed: > * session_register > * session_unregister > * session_is_registered > They need register_globals to work
> I will wait until wednesday for feedbacks or objections, and commit on > thursday.
> --Pierre
Best regards, Marcus

Andrei Zmievski

20 years ago
Pierre, I think you should apply it. Kill the f***ing thing. -Andrei On Mar 6, 2006, at 12:46 PM, Pierre wrote:

Zeev Suraski

20 years ago
A part of the decision was to supply the few lines of code necessary to emulate register_globals in userspace. Volunteers? :) Zeev At 22:46 06/03/2006, Pierre wrote:

Jason Garber

20 years ago
Hello Zeev, I'd be happy to do this. Do you want this placed in a function or just sample code to post somewhere?
-- Best regards, Jason mailto:jason@ionzoft.com Monday, March 6, 2006, 6:38:34 PM, you wrote: ZS> A part of the decision was to supply the few lines of code necessary ZS> to emulate register_globals in userspace. Volunteers? :) ZS> Zeev ZS> At 22:46 06/03/2006, Pierre wrote:

Hannes Magnusson

20 years ago
On 3/7/06, Zeev Suraski <zeev@zend.com> wrote:
> A part of the decision was to supply the few lines of code necessary > to emulate register_globals in userspace. Volunteers? :) >
http://php.is/bugs/register_globals/register_globals.phps :) - Hannes

Stefan Walk

20 years ago
Hannes Magnusson wrote:
> On 3/7/06, Zeev Suraski <zeev@zend.com> wrote: >> A part of the decision was to supply the few lines of code necessary >> to emulate register_globals in userspace. Volunteers? :) >> > > http://php.is/bugs/register_globals/register_globals.phps :) > > - Hannes >
script.php?_SESSION[foo]=bar Read the warning at http://www.php.net/extract Regards, Stefan

Zeev Suraski

20 years ago
At 01:59 07/03/2006, Hannes Magnusson wrote:
>On 3/7/06, Zeev Suraski <zeev@zend.com> wrote: > > A part of the decision was to supply the few lines of code necessary > > to emulate register_globals in userspace. Volunteers? :) > > > >http://php.is/bugs/register_globals/register_globals.phps :)
Looks good, I guess only the nuked session_ functions are missing :) Zeev

Hannes Magnusson

20 years ago
On 3/7/06, Zeev Suraski <zeev@zend.com> wrote:
> At 01:59 07/03/2006, Hannes Magnusson wrote: > >On 3/7/06, Zeev Suraski <zeev@zend.com> wrote: > > > A part of the decision was to supply the few lines of code necessary > > > to emulate register_globals in userspace. Volunteers? :) > > > > > > >http://php.is/bugs/register_globals/register_globals.phps :) > > Looks good, I guess only the nuked session_ functions are missing :)
http://php.is/bugs/register_globals/register_globals-session.phps It should be noted that I haven't run any real-world tests on this - Hannes

Andrew Yochum

20 years ago
On Tue, Mar 07, 2006 at 01:38:34AM +0200, Zeev Suraski wrote:
> A part of the decision was to supply the few lines of code necessary > to emulate register_globals in userspace. Volunteers? :)
FWIW, I've used this successfully when tieing two systems with opposing register_globals requirements: extract($_GET,EXTR_REFS); extract($_POST,EXTR_REFS); extract($_COOKIES,EXTR_REFS); Gets the job done simply. Andrew

Johannes Schlueter

20 years ago
Hi, On Tuesday 07 March 2006 01:10, Andrew Yochum wrote:
> On Tue, Mar 07, 2006 at 01:38:34AM +0200, Zeev Suraski wrote: > > A part of the decision was to supply the few lines of code necessary > > to emulate register_globals in userspace. Volunteers? :) > > FWIW, I've used this successfully when tieing two systems with opposing > register_globals requirements: > extract($_GET,EXTR_REFS); > extract($_POST,EXTR_REFS); > extract($_COOKIES,EXTR_REFS); > > Gets the job done simply.
Even simpler: http://php.net/import-request-variables johannes

Michael Vergoz

20 years ago
Finally ! Good job Pierre :D +1 Michael ----- Original Message ----- From: "Pierre" <pierre.php@gmail.com> To: <internals@lists.php.net>; <pierre.php@gmail.com> Sent: Monday, March 06, 2006 9:46 PM Subject: [PHP-DEV] Re: Adieu register_globals

Stefan Esser

20 years ago
Stop for a moment... What you guys are doing at the moment will completely break tons of applications. The ini settings that previously allowed activating/deactivating register_globals/magic_quotes MUST stay or atleast be emulated. It must be possible for an application to detect that the settings are OFF. Yours, Stefan Esser
-- -------------------------------------------------------------------------- Stefan Esser sesser@php.net Hardened-PHP Project http://www.hardened-php.net/ GPG-Key gpg --keyserver pgp.mit.edu --recv-key 0x15ABDA78 Key fingerprint 7806 58C8 CFA8 CE4A 1C2C 57DD 4AE1 795E 15AB DA78 --------------------------------------------------------------------------

Rasmus Lerdorf

20 years ago
Stefan Esser wrote:
> Stop for a moment... > > What you guys are doing at the moment will completely break tons of > applications. > The ini settings that previously allowed activating/deactivating > register_globals/magic_quotes MUST stay or atleast be emulated. > It must be possible for an application to detect that the settings are OFF.
That was the plan. Checking for register_globals should return false. -Rasmus

Stefan Esser

20 years ago
> That was the plan. Checking for register_globals should return false.
Pierre's list says it removes things like get_magic_quotes_gpc() ..., which is a NONO. Stefan
-- -------------------------------------------------------------------------- Stefan Esser sesser@php.net Hardened-PHP Project http://www.hardened-php.net/ GPG-Key gpg --keyserver pgp.mit.edu --recv-key 0x15ABDA78 Key fingerprint 7806 58C8 CFA8 CE4A 1C2C 57DD 4AE1 795E 15AB DA78 --------------------------------------------------------------------------

Derick Rethans

20 years ago
On Tue, 7 Mar 2006, Stefan Esser wrote:
> > > That was the plan. Checking for register_globals should return false. > > Pierre's list says it removes things like get_magic_quotes_gpc() ..., > which is a NONO.
yeah, agreed. And we should also throw errors when people make the setting, as per http://www.php.net/~derick/meeting-notes.html#register-globals regards, Derick

Pierre Joye

20 years ago
On 3/7/06, Derick Rethans <derick@php.net> wrote:
> On Tue, 7 Mar 2006, Stefan Esser wrote: > > > > > > That was the plan. Checking for register_globals should return > > > false. > > > > Pierre's list says it removes things like > > get_magic_quotes_gpc() ..., which is a NONO. > > yeah, agreed. And we should also throw errors when people make the > setting, as per > http://www.php.net/~derick/meeting-notes.html#register-globals
What is the point of detecting something that does not exist anymore? It is not a problem to add these checks and errors, only senseless. What is the reason to do it? PHP6 will require most applications to be ported, this problem will be their smallest problem and can be easily emulated in userland: function get_magic_quotes_gpc() { return false; } --Pierre

Stefan Esser

20 years ago
>What is the point of detecting something that does not exist anymore? > >It is not a problem to add these checks and errors, only senseless. > >What is the reason to do it? PHP6 will require most applications to be >ported, this problem will be their smallest problem and can be easily >emulated in userland: > >
Forgive me if I am wrong, but MOST PHP applications are not using OOP. And I don't see why these applications have to be ported at all. But once you remove the possibility to detect that these features are turned off the applications can become insecure. And instead of removing the function set_magic_quotes_runtime() it should better give a clear FATAL ERROR imho. Otherwise we will have myriads of bugreports: set_magic_quotes_runtime() or (whatever function removed) doesn't exist in my PHP anymore. Giving the warning that the script uses a feature that no longer exists and pointing to a document describing why will save us a lot of work and trouble. Stefan

Pierre Joye

20 years ago
On 3/7/06, Stefan Esser <sesser@php.net> wrote:
> > >What is the point of detecting something that does not exist anymore? > > > >It is not a problem to add these checks and errors, only senseless. > > > >What is the reason to do it? PHP6 will require most applications to be > >ported, this problem will be their smallest problem and can be easily > >emulated in userland: > > > > > Forgive me if I am wrong, but MOST PHP applications are not using OOP.
OOP is only a (small) part of the changes.
> And I don't see why these applications have to be ported at all. But > once you remove the possibility to detect that these features are turned > off the applications can become insecure.
Removing these features do not make application more or less secure. Developers who care about security do not rely on them.
> And instead of removing the function set_magic_quotes_runtime() it > should better give a clear FATAL ERROR imho. Otherwise we will have > myriads of bugreports: set_magic_quotes_runtime() or (whatever function > removed) doesn't exist in my PHP anymore.
I can take care of these bugs report and bogus them, really not a problem.
> Giving the warning that the > script uses a feature that no longer exists and pointing to a document > describing why will save us a lot of work and trouble.
It is a migration howto and a documentation problem. Either we keep them or we remove them, but having faked functions are not a good idea, or do we want to keep everything until php10? --Pierre

Zeev Suraski

20 years ago
At 12:27 07/03/2006, Pierre wrote:
>On 3/7/06, Derick Rethans <derick@php.net> wrote: > > On Tue, 7 Mar 2006, Stefan Esser wrote: > > > > > > > > > That was the plan. Checking for register_globals should return > > > > false. > > > > > > Pierre's list says it removes things like > > > get_magic_quotes_gpc() ..., which is a NONO. > > > > yeah, agreed. And we should also throw errors when people make the > > setting, as per > > http://www.php.net/~derick/meeting-notes.html#register-globals > >What is the point of detecting something that does not exist anymore?
The point is that breakage is aggregated, not binary. The more stuff we break, the more difficult it is to port, and frankly, it's quite likely that a non OO app could migrate fairly cleanly even to PHP 6 with unicode disabled (perhaps with minor fixes). get_magic_quotes_gpc() is designed for apps to do something differently depending on the value of magic_quotes_gpc. It's fine that it's always off in PHP 6, but there's no reason not to keep this function (to always return false) so that you don't have to fix God knows how many lines of code to remove it. Zeev

Lukas Smith

20 years ago
Zeev Suraski wrote:
> The point is that breakage is aggregated, not binary. The more stuff we > break, the more difficult it is to port, and frankly, it's quite likely > that a non OO app could migrate fairly cleanly even to PHP 6 with > unicode disabled (perhaps with minor fixes). get_magic_quotes_gpc() is > designed for apps to do something differently depending on the value of > magic_quotes_gpc. It's fine that it's always off in PHP 6, but there's > no reason not to keep this function (to always return false) so that you > don't have to fix God knows how many lines of code to remove it.
yes .. php script thats worked with gpc off (and that had some code to detect if its on and act accordingly) should still work without change. regards, Lukas

Mike Lively

20 years ago
Just a silly little brainstorm.... What about moving those functions to a separate php extension where they can eventually be phased out? At least at that point you can get it all out of the core but still provide an easy way for people who need the BC to have it. (is there already a deprec extension somewhere?) On Tue, 2006-03-07 at 16:08 +0200, Zeev Suraski wrote: