The very simple attached patch adds an option to disable POST data
processing, which implies the data can only be read in a stream fashion
through php://input.
As far as I know, PHP offers no way to inhibit processing RFC 1867 data
and one has to use very hacky means to accomplish that. This is often
required (or at least convenient) in order to, e.g., proxy requests or
handle file uploads in memory.
For other types of requests, the default processing of POST data may also
be a problem. Take a non-application/x-www-form-urlencoded POST requests
(say, some kind of RPC with a big XML payload) -- PHP is very memory
inefficient as it will hold the whole POST data into memory and duplicate
it twice (from SG(request_info).post_data to $HTTP_RAW_POST_DATA -- even
if always_populate_raw_post_data=0 -- and SG(request_info).raw_post_data).
This introduces a new ini setting, disable_post_data_processing, but it's
a benign one. No incompatibilities between setups will arise because no
one will enable it globally (it would be insane), only selectively to the
scripts that require it. The reason for an ini setting is that it must be
set early in the request life.
Thoughts?
--
Gustavo Lopes
On Tue, Dec 7, 2010 at 8:08 AM, Gustavo Lopes glopes@nebm.ist.utl.ptwrote:
The very simple attached patch adds an option to disable POST data
processing, which implies the data can only be read in a stream fashion
through php://input.As far as I know, PHP offers no way to inhibit processing RFC 1867 data and
one has to use very hacky means to accomplish that. This is often required
(or at least convenient) in order to, e.g., proxy requests or handle file
uploads in memory.For other types of requests, the default processing of POST data may also
be a problem. Take a non-application/x-www-form-urlencoded POST requests
(say, some kind of RPC with a big XML payload) -- PHP is very memory
inefficient as it will hold the whole POST data into memory and duplicate it
twice (from SG(request_info).post_data to $HTTP_RAW_POST_DATA -- even if
always_populate_raw_post_data=0 -- and SG(request_info).raw_post_data).This introduces a new ini setting, disable_post_data_processing, but it's a
benign one. No incompatibilities between setups will arise because no one
will enable it globally (it would be insane), only selectively to the
scripts that require it. The reason for an ini setting is that it must be
set early in the request life.Thoughts?
I like the idea.
Tyrael
The very simple attached patch adds an option to disable POST data
processing, which implies the data can only be read in a stream
fashion through php://input.This introduces a new ini setting, disable_post_data_processing, but
it's a benign one. No incompatibilities between setups will arise
because no one will enable it globally (it would be insane), only
selectively to the scripts that require it. The reason for an ini
setting is that it must be set early in the request life.Thoughts?
Definitely +1.
The very simple attached patch adds an option to disable POST data
processing, which implies the data can only be read in a stream fashion
through php://input.As far as I know, PHP offers no way to inhibit processing RFC 1867 data and
one has to use very hacky means to accomplish that. This is often required
(or at least convenient) in order to, e.g., proxy requests or handle file
uploads in memory.For other types of requests, the default processing of POST data may also be
a problem. Take a non-application/x-www-form-urlencoded POST requests (say,
some kind of RPC with a big XML payload) -- PHP is very memory inefficient
as it will hold the whole POST data into memory and duplicate it twice (from
SG(request_info).post_data to $HTTP_RAW_POST_DATA -- even if
always_populate_raw_post_data=0 -- and SG(request_info).raw_post_data).This introduces a new ini setting, disable_post_data_processing, but it's a
benign one. No incompatibilities between setups will arise because no one
will enable it globally (it would be insane), only selectively to the
scripts that require it. The reason for an ini setting is that it must be
set early in the request life.Thoughts?
--
Gustavo Lopes
As I understand things, the super globals are already populated by the
time the script starts execution.
So, ini_set() will have no impact.
Can you set an ini option for a single script via some other method?
--
Richard Quadling
Twitter : EE : Zend
@RQuadling : e-e.com/M_248814.html : bit.ly/9O8vFY
The very simple attached patch adds an option to disable POST data
processing, which implies the data can only be read in a stream fashion
through php://input.As far as I know, PHP offers no way to inhibit processing RFC 1867 data and
one has to use very hacky means to accomplish that. This is often required
(or at least convenient) in order to, e.g., proxy requests or handle file
uploads in memory.For other types of requests, the default processing of POST data may also be
a problem. Take a non-application/x-www-form-urlencoded POST requests (say,
some kind of RPC with a big XML payload) -- PHP is very memory inefficient
as it will hold the whole POST data into memory and duplicate it twice (from
SG(request_info).post_data to $HTTP_RAW_POST_DATA -- even if
always_populate_raw_post_data=0 -- and SG(request_info).raw_post_data).This introduces a new ini setting, disable_post_data_processing, but it's a
benign one. No incompatibilities between setups will arise because no one
will enable it globally (it would be insane), only selectively to the
scripts that require it. The reason for an ini setting is that it must be
set early in the request life.Thoughts?
--
Gustavo LopesAs I understand things, the super globals are already populated by the
time the script starts execution.So,
ini_set()will have no impact.Can you set an ini option for a single script via some other method?
Maybe thru an .htaccess file? That should work.
Otherwise, +1 for the patch from me.
John Mertic
jmertic@gmail.com | http://jmertic.wordpress.com
The very simple attached patch adds an option to disable POST data
processing, which implies the data can only be read in a stream fashion
through php://input.As far as I know, PHP offers no way to inhibit processing RFC 1867 data and
one has to use very hacky means to accomplish that. This is often required
(or at least convenient) in order to, e.g., proxy requests or handle file
uploads in memory.For other types of requests, the default processing of POST data may also be
a problem. Take a non-application/x-www-form-urlencoded POST requests (say,
some kind of RPC with a big XML payload) -- PHP is very memory inefficient
as it will hold the whole POST data into memory and duplicate it twice (from
SG(request_info).post_data to $HTTP_RAW_POST_DATA -- even if
always_populate_raw_post_data=0 -- and SG(request_info).raw_post_data).This introduces a new ini setting, disable_post_data_processing, but it's a
benign one. No incompatibilities between setups will arise because no one
will enable it globally (it would be insane), only selectively to the
scripts that require it. The reason for an ini setting is that it must be
set early in the request life.Thoughts?
--
Gustavo LopesAs I understand things, the super globals are already populated by the
time the script starts execution.So,
ini_set()will have no impact.Can you set an ini option for a single script via some other method?
Maybe thru an .htaccess file? That should work.
Otherwise, +1 for the patch from me.
John Mertic
jmertic@gmail.com | http://jmertic.wordpress.com
Can you set an ini option via .htaccess for a single script?
If the script has to reside in its own directory then that makes
sense, but I don't know how to set an ini option for a single script.
--
Richard Quadling
Twitter : EE : Zend
@RQuadling : e-e.com/M_248814.html : bit.ly/9O8vFY
The very simple attached patch adds an option to disable POST data
processing, which implies the data can only be read in
a stream fashion through php://input.
Does without this patch, reading POST data from php://input work?
This introduces a new ini setting, disable_post_data_processing, but
it's a benign one. No incompatibilities between setups will arise
because no one will enable it globally (it would be insane),
Because they can, people will do it though... I'm not fan of more ini
settings.
Derick
The very simple attached patch adds an option to disable POST data
processing, which implies the data can only be read in
a stream fashion through php://input.Does without this patch, reading POST data from php://input work?
yep.
you will get the raw post data with this.
same as $HTTP_RAW_POST_DATA the only difference, that the latter isn't
always available.
This introduces a new ini setting, disable_post_data_processing, but
it's a benign one. No incompatibilities between setups will arise
because no one will enable it globally (it would be insane),Because they can, people will do it though... I'm not fan of more ini
settings.
so because in the past ini settings was abused,we won't add ini settings
ever?
Tyrael
The very simple attached patch adds an option to disable POST data
processing, which implies the data can only be read in
a stream fashion through php://input.Does without this patch, reading POST data from php://input work?
Maybe.
In case of a multipart/form-data request, it won't.
For other requests, it will. However, php://input will be reading from
memory, not from the SAPI, because the entire POST data will be in memory.
The new option makes the behavior for a POST request similar to that of
e.g. a PUT request, i.e., data can only be read with php://input and it
will not be copied entirely into memory before the user code executes.
Of course the user can use something like file_get_contents('php://input')
to do that, but, depending on the application, he may take advantage of
the low memory usage a stream provides (for the XML payload example, he
could use a SAX-like parser, for proxying or saving the entire request
into disk -- possibly filtered -- he could use stream_copy_to_stream,
etc.).
This introduces a new ini setting, disable_post_data_processing, but
it's a benign one. No incompatibilities between setups will arise
because no one will enable it globally (it would be insane),Because they can, people will do it though... I'm not fan of more ini
settings.
I'm sure someone somewhere will enable it eventually without being aware
of what it does (despite the obvious name...); my point is that
programmers don't need to program against it (unlike say, magic quotes)
because no working PHP setup will have it enabled, as it breaks every
script that handles POST data.
--
Gustavo Lopes
Hi,
On Tue, Dec 7, 2010 at 2:08 AM, Gustavo Lopes glopes@nebm.ist.utl.ptwrote:
The very simple attached patch adds an option to disable POST data
processing, which implies the data can only be read in a stream fashion
through php://input.As far as I know, PHP offers no way to inhibit processing RFC 1867 data and
one has to use very hacky means to accomplish that. This is often required
(or at least convenient) in order to, e.g., proxy requests or handle file
uploads in memory.For other types of requests, the default processing of POST data may also
be a problem. Take a non-application/x-www-form-urlencoded POST requests
(say, some kind of RPC with a big XML payload) -- PHP is very memory
inefficient as it will hold the whole POST data into memory and duplicate it
twice (from SG(request_info).post_data to $HTTP_RAW_POST_DATA -- even if
always_populate_raw_post_data=0 -- and SG(request_info).raw_post_data).This introduces a new ini setting, disable_post_data_processing, but it's a
benign one. No incompatibilities between setups will arise because no one
will enable it globally (it would be insane), only selectively to the
scripts that require it. The reason for an ini setting is that it must be
set early in the request life.Thoughts?
This is a very, very good idea as more and more requests will be in
non-urlencoded format (e.g.: JSON), especially in REST architectures. A
little performance boost would not do any harm.
+1.
--
Gustavo Lopes
--
Nicolas A. Bérard-Nault (nicobn@gmail.com)
This introduces a new ini setting, disable_post_data_processing, but
it's a benign one. No incompatibilities between setups will arise
because no one will enable it globally (it would be insane), only
selectively to the scripts that require it. The reason for an ini
setting is that it must be set early in the request life.
I'm ok with it. It's a bit awkward having disable=on to disable
something though. Generally we prefer enable=off because it causes
fewer brain injuries.
-Rasmus
On Tue, 07 Dec 2010 07:08:34 -0000, Gustavo Lopes glopes@nebm.ist.utl.pt
wrote:
The very simple attached patch adds an option to disable POST data
processing, which implies the data can only be read in a stream fashion
through php://input.
I've committed to trunk the patch with the name of the ini option changed
from disable_post_data_processing to enable_post_data_reading.
--
Gustavo Lopes
hi,
The more I look at this option the more I think it is confusing. I'm
not sure the gain is worth this confusion either. However I would
prefer to bring back a proposal we had a couple of years ago, to
totally disable post data. When disabled, the POST data will be
totally ignored, no matter if php://input, raw data or whatever other
ways we may have to access it. The data given by the server/sapi will
be ignored.
This option has the benefit to be very simple and solves one known
attack vector in a very clean way.
Cheers,
On Tue, 07 Dec 2010 07:08:34 -0000, Gustavo Lopes glopes@nebm.ist.utl.pt
wrote:The very simple attached patch adds an option to disable POST data
processing, which implies the data can only be read in a stream fashion
through php://input.I've committed to trunk the patch with the name of the ini option changed
from disable_post_data_processing to enable_post_data_reading.--
Gustavo Lopes--
--
Pierre
@pierrejoye | http://blog.thepimp.net | http://www.libgd.org
hi,
The more I look at this option the more I think it is confusing. I'm
not sure the gain is worth this confusion either. However I would
prefer to bring back a proposal we had a couple of years ago, to
totally disable post data. When disabled, the POST data will be
totally ignored, no matter if php://input, raw data or whatever other
ways we may have to access it. The data given by the server/sapi will
be ignored.This option has the benefit to be very simple and solves one known
attack vector in a very clean way.
Yeah, but that idea isn't solves the problem which the current one tries to
address.
So while I think that it would be a good security measure to alllow to
disable POST altogether, but that should be discussed/voted in a different
thread.
So currently we didn't talk about security measures, but performance gains:
If somebody wants to write a script, which handles big file uploads, but
only writes it to somewhere (to file, or another stream), then currently you
have to allocate the memory for the post data twice(see the first email),
which is very inefficient if you don't need the $_POST at all.
Tyrael
hi,
So currently we didn't talk about security measures, but performance gains:
If somebody wants to write a script, which handles big file uploads, but
only writes it to somewhere (to file, or another stream), then currently you
have to allocate the memory for the post data twice(see the first email),
which is very inefficient if you don't need the $_POST at all.
yes, but that's something very confusing right now, the naming and the
other ways to access POST data. The goal of this idea is a good thing
to do, but the naming and its implementation are confusing (processing
vs reading vs used at all).
Cheers,
Pierre
@pierrejoye | http://blog.thepimp.net | http://www.libgd.org
hi,
So currently we didn't talk about security measures, but performance
gains:
If somebody wants to write a script, which handles big file uploads, but
only writes it to somewhere (to file, or another stream), then currently
you
have to allocate the memory for the post data twice(see the first email),
which is very inefficient if you don't need the $_POST at all.yes, but that's something very confusing right now, the naming and the
other ways to access POST data. The goal of this idea is a good thing
to do, but the naming and its implementation are confusing (processing
vs reading vs used at all).
agree, but it's not helping, if we introduce another similar idea for a
whole different purpose to the conversation. :)
the best would be a nice and clean RFC with the current status, the known
problems, the suggested solutions, and a common and well-understood
consistent naming convention. (disable POST processing, disable POST
population, etc.)
Tyrael
On Thu, 16 Dec 2010 12:47:43 -0000, Pierre Joye pierre.php@gmail.com
wrote:
As I said in IRC, I see no value in having an option that disables
accessing POST data completely (i.e. the behavior of
enable_post_data_reading=Off + disallowing reading it through
php://input).
With enable_post_data_reading=Off, the only way to access the data is
through php://input; if the script uses the wrapper to read the data is
because it presumably needs, or expects, such data. If the script doesn't
need it, it won't read php://input. Activating a putative option that
completely disabled reading POST data would break the first script and
would have no effect on the second. And it's not like there would be
performance benefits from introducing such as option.
More importantly, the enable_post_data_reading is to allow efficient
handling of POST data and access to raw POST data with multipart requests.
Adding an option that disables accessing POST data completely would not
help.
So currently we didn't talk about security measures, but performance
gains:
If somebody wants to write a script, which handles big file uploads,
but
only writes it to somewhere (to file, or another stream), then
currently you have to allocate the memory for the post data twice(see
the first email), which is very inefficient if you don't need the
$_POST at all.yes, but that's something very confusing right now, the naming and the
other ways to access POST data. The goal of this idea is a good thing
to do, but the naming and its implementation are confusing (processing
vs reading vs used at all).
I don't thinks it's that confusing. This new option just prevents PHP from
doing any automatic reading and processing of the POST data, leaving it
entirely on the hands of the programmer to that, if he wishes, through
php://input.
What is confusing is the different behavior of PHP with
$HTTP_RAW_POST_DATA and the influence always_populate_post_data and the
different handling of urlencoded vs multipart vs other content-type POST
requests vs non-POST requests with a request body.
--
Gustavo Lopes
There are a lot of values to disable POST completely. That's also why
thinking the option you are proposing while keeping in mind the whole
picture makes sense.
There are different existing modes and one or two new modes (to be
introduced, like disabling POST). Having a clean way to choose which
mode should be used for a request makes sense and will improve the
user experience. Right now and as shown the confusing discussions on
IRC (or partially here) shows that the current situation is a mess.
Again, I'm not saying that the idea behind this patch is bad, only
that there is a better way to do it while being less confusing and
making the whole thing cleaner to define.
2010/12/16 Gustavo Lopes glopes@nebm.ist.utl.pt:
On Thu, 16 Dec 2010 12:47:43 -0000, Pierre Joye pierre.php@gmail.com
wrote:As I said in IRC, I see no value in having an option that disables
accessing POST data completely (i.e. the behavior of
enable_post_data_reading=Off + disallowing reading it through
php://input).With enable_post_data_reading=Off, the only way to access the data is
through php://input; if the script uses the wrapper to read the data is
because it presumably needs, or expects, such data. If the script doesn't
need it, it won't read php://input. Activating a putative option that
completely disabled reading POST data would break the first script and
would have no effect on the second. And it's not like there would be
performance benefits from introducing such as option.More importantly, the enable_post_data_reading is to allow efficient
handling of POST data and access to raw POST data with multipart requests.
Adding an option that disables accessing POST data completely would not
help.So currently we didn't talk about security measures, but performance
gains:
If somebody wants to write a script, which handles big file uploads,
but
only writes it to somewhere (to file, or another stream), then
currently you have to allocate the memory for the post data twice(see
the first email), which is very inefficient if you don't need the
$_POST at all.yes, but that's something very confusing right now, the naming and the
other ways to access POST data. The goal of this idea is a good thing
to do, but the naming and its implementation are confusing (processing
vs reading vs used at all).I don't thinks it's that confusing. This new option just prevents PHP from
doing any automatic reading and processing of the POST data, leaving it
entirely on the hands of the programmer to that, if he wishes, through
php://input.What is confusing is the different behavior of PHP with
$HTTP_RAW_POST_DATA and the influence always_populate_post_data and the
different handling of urlencoded vs multipart vs other content-type POST
requests vs non-POST requests with a request body.--
Gustavo Lopes
--
Pierre
@pierrejoye | http://blog.thepimp.net | http://www.libgd.org
There are a lot of values to disable POST completely. That's also why
thinking the option you are proposing while keeping in mind the whole
picture makes sense.There are different existing modes and one or two new modes (to be
introduced, like disabling POST). Having a clean way to choose which
mode should be used for a request makes sense and will improve the
user experience. Right now and as shown the confusing discussions on
IRC (or partially here) shows that the current situation is a mess.Again, I'm not saying that the idea behind this patch is bad, only
that there is a better way to do it while being less confusing and
making the whole thing cleaner to define.
http://en.wikipedia.org/wiki/Nirvana_fallacy
http://en.wikipedia.org/wiki/Nirvana_fallacyIf you think that there is a
better alternative, please write an RFC and or a patch, but I don't think
that we should give up a good idea which has support and patch for a vague
idea, which is maybe better, but nobody going to put effort in it to write
RFC, write the patch, etc.
but that is my 2 cents of course.
Tyrael
doing something badly designed only because we can do it is a very bad idea.
http://en.wikipedia.org/wiki/Nirvana_fallacy
If you think that there is a better alternative, please write an RFC and or
a patch, but I don't think that we should give up a good idea which has
support and patch for a vague idea, which is maybe better, but nobody going
to put effort in it to write RFC, write the patch, etc.
but that is my 2 cents of course.
Tyrael
--
Pierre
@pierrejoye | http://blog.thepimp.net | http://www.libgd.org
doing something badly designed only because we can do it is a very bad
idea.
you didn't said that this patch is badly designed, you just said, that with
putting some other ideas together we could come up with a more better
solution.
if you think that this is a bad idea, then feel free to point out the errors
out, but I think we shouldn't halt this proposal and wait for the better one
to write itself...
could you tell me that when will you, or somebody else have time to look
into the current situation, and start working on your proposal (eg. complete
rewamp on the current input handling/superglobals population)?
the PHP6 or the php.net redesign showed us that not always the
all-or-nothing is the best approach for doing things.
Tyrael
There are a lot of values to disable POST completely. That's also why
thinking the option you are proposing while keeping in mind the whole
picture makes sense.There are different existing modes and one or two new modes (to be
introduced, like disabling POST). Having a clean way to choose which
mode should be used for a request makes sense and will improve the
user experience. Right now and as shown the confusing discussions on
IRC (or partially here) shows that the current situation is a mess.Again, I'm not saying that the idea behind this patch is bad, only
that there is a better way to do it while being less confusing and
making the whole thing cleaner to define.
Seriously, disabling POST via a php.ini setting is considered a
"sensible" option? Has nobody ever developed a RESTful application?
The only way I can see such an action being "sensible" is if it's also
runtime configurable (i.e., via ini_set()) - otherwise I foresee a ton
of issues between security-paranoid sysadmins and developers when code
is pushed to production and simply stops working...
I understand the rationale behind Pierre's assertion, but it's
incredibly short-sighted when you consider a full application, where
some actions need to accept raw POST/PUT data (man, would I love a $_PUT
superglobal...), others need to accept file uploads, and most others
need neither.
2010/12/16 Gustavo Lopes glopes@nebm.ist.utl.pt:
On Thu, 16 Dec 2010 12:47:43 -0000, Pierre Joye pierre.php@gmail.com
wrote:As I said in IRC, I see no value in having an option that disables
accessing POST data completely (i.e. the behavior of
enable_post_data_reading=Off + disallowing reading it through
php://input).With enable_post_data_reading=Off, the only way to access the data is
through php://input; if the script uses the wrapper to read the data is
because it presumably needs, or expects, such data. If the script doesn't
need it, it won't read php://input. Activating a putative option that
completely disabled reading POST data would break the first script and
would have no effect on the second. And it's not like there would be
performance benefits from introducing such as option.More importantly, the enable_post_data_reading is to allow efficient
handling of POST data and access to raw POST data with multipart requests.
Adding an option that disables accessing POST data completely would not
help.So currently we didn't talk about security measures, but performance
gains:
If somebody wants to write a script, which handles big file uploads,
but
only writes it to somewhere (to file, or another stream), then
currently you have to allocate the memory for the post data twice(see
the first email), which is very inefficient if you don't need the
$_POST at all.yes, but that's something very confusing right now, the naming and the
other ways to access POST data. The goal of this idea is a good thing
to do, but the naming and its implementation are confusing (processing
vs reading vs used at all).I don't thinks it's that confusing. This new option just prevents PHP from
doing any automatic reading and processing of the POST data, leaving it
entirely on the hands of the programmer to that, if he wishes, through
php://input.What is confusing is the different behavior of PHP with
$HTTP_RAW_POST_DATA and the influence always_populate_post_data and the
different handling of urlencoded vs multipart vs other content-type POST
requests vs non-POST requests with a request body.--
Gustavo Lopes
--
Matthew Weier O'Phinney
Project Lead | matthew@zend.com
Zend Framework | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc
On Thu, Dec 16, 2010 at 3:18 PM, Matthew Weier O'Phinney
weierophinney@php.net wrote:
There are a lot of values to disable POST completely. That's also why
thinking the option you are proposing while keeping in mind the whole
picture makes sense.There are different existing modes and one or two new modes (to be
introduced, like disabling POST). Having a clean way to choose which
mode should be used for a request makes sense and will improve the
user experience. Right now and as shown the confusing discussions on
IRC (or partially here) shows that the current situation is a mess.Again, I'm not saying that the idea behind this patch is bad, only
that there is a better way to do it while being less confusing and
making the whole thing cleaner to define.Seriously, disabling POST via a php.ini setting is considered a
"sensible" option? Has nobody ever developed a RESTful application?
No comment.
The only way I can see such an action being "sensible" is if it's also
runtime configurable (i.e., viaini_set()) - otherwise I foresee a ton
of issues between security-paranoid sysadmins and developers when code
is pushed to production and simply stops working...I understand the rationale behind Pierre's assertion, but it's
incredibly short-sighted when you consider a full application, where
some actions need to accept raw POST/PUT data (man, would I love a $_PUT
superglobal...), others need to accept file uploads, and most others
need neither.
I never said it should be a php.ini option, or only a php.ini option.
But having 300 ways to do the same things, or to change options is
bad. You should also keep in mind that even if it is a php.ini option,
it should obviously be INI_PER_DIR and not system wild (that's what
servers options exist, or better appropriate there if one wants to
disable POST permanently and for all requests/URLs).
All in all, I don't think adding a set of new ini settings for very
specific and disputable features is not something good to do. If we
need something to process file uploads more efficiently, then let do
it in a good way (no, processing php://input manually is not what I
can consider as a good way :).
Cheers,
Pierre
@pierrejoye | http://blog.thepimp.net | http://www.libgd.org
On Thu, Dec 16, 2010 at 3:18 PM, Matthew Weier O'Phinney
weierophinney@php.net wrote:The only way I can see such an action being "sensible" is if it's also
runtime configurable (i.e., viaini_set()) - otherwise I foresee a ton
of issues between security-paranoid sysadmins and developers when code
is pushed to production and simply stops working...I understand the rationale behind Pierre's assertion, but it's
incredibly short-sighted when you consider a full application, where
some actions need to accept raw POST/PUT data (man, would I love a $_PUT
superglobal...), others need to accept file uploads, and most others
need neither.I never said it should be a php.ini option, or only a php.ini option.
But having 300 ways to do the same things, or to change options is
bad. You should also keep in mind that even if it is a php.ini option,
it should obviously be INI_PER_DIR and not system wild (that's what
servers options exist, or better appropriate there if one wants to
disable POST permanently and for all requests/URLs).
INI_PER_DIR doesn't work for, I would argue, the majority of modern PHP
applications.
If you look at most frameworks, CMS solutions, or standalone apps such
as WordPress, there's a single script acting as the gateway to all
functionality (basically, a front controller). Making the setting
INI_PER_DIR means that you have to move any functionality that may
potentially require access to raw POST/PUT data into separate scripts
and/or directories -- which splits functionality away from the front
controller and makes maintenance much more difficult.
All in all, I don't think adding a set of new ini settings for very
specific and disputable features is not something good to do. If we
need something to process file uploads more efficiently, then let do
it in a good way (no, processing php://input manually is not what I
can consider as a good way :).
I'm not talking about processing file uploads; I'm talking about normal
handling of raw POST/PUT body content -- something that's very, very
common when dealing with RESTful or RPC APIs, where the format isn't
normal URL encoding, but instead something like XML or JSON.
I'm all for better, more efficient processing of file uploads -- but not
at the expense of being able to build good APIs.
--
Matthew Weier O'Phinney
Project Lead | matthew@zend.com
Zend Framework | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc
On Thu, 16 Dec 2010 14:53:08 -0000, Matthew Weier O'Phinney
weierophinney@php.net wrote:
I never said it should be a php.ini option, or only a php.ini option.
But having 300 ways to do the same things, or to change options is
bad. You should also keep in mind that even if it is a php.ini option,
it should obviously be INI_PER_DIR and not system wild (that's what
servers options exist, or better appropriate there if one wants to
disable POST permanently and for all requests/URLs).INI_PER_DIR doesn't work for, I would argue, the majority of modern PHP
applications.If you look at most frameworks, CMS solutions, or standalone apps such
as WordPress, there's a single script acting as the gateway to all
functionality (basically, a front controller). Making the setting
INI_PER_DIR means that you have to move any functionality that may
potentially require access to raw POST/PUT data into separate scripts
and/or directories -- which splits functionality away from the front
controller and makes maintenance much more difficult.
I know you're responding to Pierre's proposed addition of a way to disable
POST data handling altogether possibly via an ini option, but since the
objection also applies to the ini option I've added to trunk, I'd like to
address it.
Yes, it sucks that the option cannot be changed with ini_set, but that's
an inevitable technical limitation that I don't see how it can be overcome
without major changes. By the time control is passed to the script, the
interesting stuff has already happened.
(By the way, I think it's unfortunate that most frameworks abandoned the
simplicity of a direct correspondence file <--> url -- whenever practical,
of course, but that's another matter)
All in all, I don't think adding a set of new ini settings for very
specific and disputable features is not something good to do. If we
need something to process file uploads more efficiently, then let do
it in a good way (no, processing php://input manually is not what I
can consider as a good way :).
As to multipart requests, we could of course build some mechanism to
provide, sequentially and in the order they are uploaded, streams for the
uploaded files; this would be easier than processing php://input manually.
It's also much more complicated to implement and doesn't cover all the
cases in which we want a way to handle multipart requests that differs
from the default behavior -- think, for instance, proxying requests.
In short, I don't think the possibility of a future implementation of more
high level mechanisms should preclude the existence of low level ones such
as the proposed and committed enable_post_data_processing, as the low
level ones, being less specific, have a larger domain of application,
despite being more cumbersome.
I'm not talking about processing file uploads; I'm talking about normal
handling of raw POST/PUT body content -- something that's very, very
common when dealing with RESTful or RPC APIs, where the format isn't
normal URL encoding, but instead something like XML or JSON.I'm all for better, more efficient processing of file uploads -- but not
at the expense of being able to build good APIs.
I'm not sure what you're proposing instead.
--
Gustavo Lopes
In short, I don't think the possibility of a future implementation of more
high level mechanisms should preclude the existence of low level ones such
as the proposed and committed enable_post_data_processing, as the low level
ones, being less specific, have a larger domain of application, despite
being more cumbersome.
I do, more than ever before. My reasoning is simple, php is full of
such small hacks we have to deal for years, both at the internal level
or from an application point of view. Hence too why I'm not sure it is
worth the gains.
Cheers,
Pierre
@pierrejoye | http://blog.thepimp.net | http://www.libgd.org
Gustavo Lopes wrote:
I know you're responding to Pierre's proposed addition of a way to
disable POST data handling altogether possibly via an ini option, but
since the objection also applies to the ini option I've added to
trunk, I'd like to address it.Yes, it sucks that the option cannot be changed with ini_set, but
that's an inevitable technical limitation that I don't see how it can
be overcome without major changes. By the time control is passed to
the script, the interesting stuff has already happened.(By the way, I think it's unfortunate that most frameworks abandoned
the simplicity of a direct correspondence file <--> url -- whenever
practical, of course, but that's another matter)
It could be a set-only option.
You can't disable it, but if it isn't set and you enable it, it
magically fills your $_POST.
Making the internal function available to the userland would be
preferable, though. That would be more flexible even though the common
usecase would be
$_POST = process_post("php://input");
Gustavo Lopes wrote:
I've committed to trunk the patch with the name of the ini option changed
from disable_post_data_processing to enable_post_data_reading.
Pierre Joye wrote:
hi,
The more I look at this option the more I think it is confusing. I'm
not sure the gain is worth this confusion either. However I would
prefer to bring back a proposal we had a couple of years ago, to
totally disable post data.
Would calling it enable_automatic_post_data_reading help with that
confusion?
Note that this is already possible by setting post_max_size to 0M. Was
useful prior to the APC upload hooks for writing progress bars.
2010/12/16 Ángel González keisial@gmail.com
Gustavo Lopes wrote:
I've committed to trunk the patch with the name of the ini option changed
from disable_post_data_processing to enable_post_data_reading.Pierre Joye wrote:
hi,
The more I look at this option the more I think it is confusing. I'm
not sure the gain is worth this confusion either. However I would
prefer to bring back a proposal we had a couple of years ago, to
totally disable post data.Would calling it enable_automatic_post_data_reading help with that
confusion?--
--
Matt Wilson
816.830.9429
Pierre Joye wrote:
The more I look at this option the more I think it is confusing. I'm
not sure the gain is worth this confusion either. However I would
prefer to bring back a proposal we had a couple of years ago, to
totally disable post data.Note that this is already possible by setting post_max_size to 0M. Was
useful prior to the APC upload hooks for writing progress bars.
I don't know how it used to be, but right now post_max_size=0 just
disables the checks on the maximum post size.
See e.g. this test:
http://svn.php.net/viewvc/php/php-src/trunk/tests/basic/bug53180.phpt?revision=304958&view=markup
--
Gustavo Lopes
Hi Pierre:
However I would
prefer to bring back a proposal we had a couple of years ago, to
totally disable post data.
Completely disabling POST is something that is probably best done
via web server configurations. Doing this at the
applicaiton/programming layer seems like a kludge.
--Dan
--
T H E A N A L Y S I S A N D S O L U T I O N S C O M P A N Y
data intensive web and database programming
http://www.AnalysisAndSolutions.com/
4015 7th Ave #4, Brooklyn NY 11232 v: 718-854-0335 f: 718-854-0409
hi,
That was one other thing related to this addition, not specifically
the main reason why I think this addition is not as good as expected.
Especially as as Gustavo said as well, it is possible to do it using a
nice and clean APIs instead of adding yet another cryptic ini setting.
I'm not saying that there is no need to improve what this feature does
but adding yet another option to deal with HTTP input is somehow very
bad.
On Sat, Dec 18, 2010 at 6:06 PM, Daniel Convissor
danielc@analysisandsolutions.com wrote:
Hi Pierre:
However I would
prefer to bring back a proposal we had a couple of years ago, to
totally disable post data.Completely disabling POST is something that is probably best done
via web server configurations. Doing this at the
applicaiton/programming layer seems like a kludge.--Dan
--
T H E A N A L Y S I S A N D S O L U T I O N S C O M P A N Y
data intensive web and database programming
http://www.AnalysisAndSolutions.com/
4015 7th Ave #4, Brooklyn NY 11232 v: 718-854-0335 f: 718-854-0409--
--
Pierre
@pierrejoye | http://blog.thepimp.net | http://www.libgd.org
December-18-10 12:07 PM Daniel Convissor wrote:
Completely disabling POST is something that is probably best done
via web server configurations. Doing this at the
applicaiton/programming layer seems like a kludge.
No offence, but I'm still waiting for someone with 2 breadsticks stuck up their
nose to jump out and yell "April Fools".
:)
Best Regards,
Mike Robinson
On Tue, Dec 7, 2010 at 8:08 AM, Gustavo Lopes glopes@nebm.ist.utl.ptwrote:
The very simple attached patch adds an option to disable POST data
processing, which implies the data can only be read in a stream fashion
through php://input.As far as I know, PHP offers no way to inhibit processing RFC 1867 data and
one has to use very hacky means to accomplish that. This is often required
(or at least convenient) in order to, e.g., proxy requests or handle file
uploads in memory.For other types of requests, the default processing of POST data may also
be a problem. Take a non-application/x-www-form-urlencoded POST requests
(say, some kind of RPC with a big XML payload) -- PHP is very memory
inefficient as it will hold the whole POST data into memory and duplicate it
twice (from SG(request_info).post_data to $HTTP_RAW_POST_DATA -- even if
always_populate_raw_post_data=0 -- and SG(request_info).raw_post_data).This introduces a new ini setting, disable_post_data_processing, but it's a
benign one. No incompatibilities between setups will arise because no one
will enable it globally (it would be insane), only selectively to the
scripts that require it. The reason for an ini setting is that it must be
set early in the request life.Thoughts?
--
Gustavo Lopes
Did we agree on something about this improvement/patch?
as I said, I like the idea, but if we cannot agree on the details(how to
control that which script needs this), could we somehow at least make it
possible to read the raw post through php://input for "multipart/form-data"
requests?
currently just no way to read the raw data for those requests which sucks
big time.
Tyrael
On Tue, Dec 7, 2010 at 8:08 AM, Gustavo Lopes
glopes@nebm.ist.utl.ptwrote:The very simple attached patch adds an option to disable POST data
processing, which implies the data can only be read in a stream fashion
through php://input.Did we agree on something about this improvement/patch?
as I said, I like the idea, but if we cannot agree on the details(how to
control that which script needs this), could we somehow at least make it
possible to read the raw post through php://input for
"multipart/form-data" requests?
currently just no way to read the raw data for those requests which sucks
big time.
As far as I remember there were no grave objections to it, only some
related proposals that didn't actually address the same problem, some
people who pointed shortcomings in this solution (e.g. cannot be set on
runtime) and some other that didn't understand it altogether.
In any case, if there are any other substantive proposals for the problems
at hand, we can revisit this topic.
For the record, these problems are:
- Excessive memory consumption on large, non multipart/form-data, POST
requests (3x the size of the request body). - No way to access the raw post data on multipart/form-data POST requests.
- No alternative way to handle files in multipart/form-data POST requests
other than dumping the files to disk.
--
Gustavo Lopes
hi Gustavo,
Could you apply your patch tomorrow please? So we have it for the
alpha3 on Thursday.
Thanks for your work!
Cheers,
The very simple attached patch adds an option to disable POST data
processing, which implies the data can only be read in a stream fashion
through php://input.As far as I know, PHP offers no way to inhibit processing RFC 1867 data and
one has to use very hacky means to accomplish that. This is often required
(or at least convenient) in order to, e.g., proxy requests or handle file
uploads in memory.For other types of requests, the default processing of POST data may also be
a problem. Take a non-application/x-www-form-urlencoded POST requests (say,
some kind of RPC with a big XML payload) -- PHP is very memory inefficient
as it will hold the whole POST data into memory and duplicate it twice (from
SG(request_info).post_data to $HTTP_RAW_POST_DATA -- even if
always_populate_raw_post_data=0 -- and SG(request_info).raw_post_data).This introduces a new ini setting, disable_post_data_processing, but it's a
benign one. No incompatibilities between setups will arise because no one
will enable it globally (it would be insane), only selectively to the
scripts that require it. The reason for an ini setting is that it must be
set early in the request life.Thoughts?
--
Gustavo Lopes
--
Pierre
@pierrejoye | http://blog.thepimp.net | http://www.libgd.org
hi Gustavo,
Could you apply your patch tomorrow please? So we have it for the
alpha3 on Thursday.
Hi Pierre,
I think you confuse dates here. Alpha 3 is planned for August 4. So it's tomorrow
in a week.
Thanks anyawy for moving forward with the patch
David