Hi Julien,
Please, remember that our release process forbidds BC breaks.
I suggest reintroducing the feature, together with
always_populate_raw_post_data (like the old behavior).What we need is vote for the change?
Mike, could you prepare RFC whether we keep the ini setting or just use
"input://".
I think it's time to vote.
Here you go, https://wiki.php.net/rfc/slim_post_data
Though, with only two supporters, chances are low.
--
Regards,
Mike
Hi Julien,
Please, remember that our release process forbidds BC breaks.
I suggest reintroducing the feature, together with
always_populate_raw_post_data (like the old behavior).What we need is vote for the change?
Mike, could you prepare RFC whether we keep the ini setting or just use
"input://".
I think it's time to vote.Here you go, https://wiki.php.net/rfc/slim_post_data
Though, with only two supporters, chances are low.
We can't break BC in 5.x, so the way to go is:
-
implement underlying changes
-
restore backwards compatibility (HTTP_RAW_POST_DATA should be
auto-populated in all cases when it is populated now) -
always_populate_raw_post_data should be introduced for BC purposes
-
Is it possible to detect usage of HTTP_RAW_POST_DATA and give
E_DEPRECATEDwhile doing so? -
remove HTTP_RAW_POST_DATA support in master (thinking about 6.x)
--
Alexey Zakhlestin
CTO at Grids.by/you
https://github.com/indeyets
PGP key: http://indeyets.ru/alexey.zakhlestin.pgp.asc
We can't break BC in 5.x, so the way to go is:
- implement underlying changes
It already is in master since a few weeks.
- restore backwards compatibility (HTTP_RAW_POST_DATA should be
auto-populated in all cases when it is populated now)
That defeats the entire change.
always_populate_raw_post_data should be introduced for BC purposes
Is it possible to detect usage of HTTP_RAW_POST_DATA and give
E_DEPRECATEDwhile doing so?
AFAICS no, it is a standard global variable.
- remove HTTP_RAW_POST_DATA support in master (thinking about 6.x)
It already is in master.
--
Regards,
Mike
We can't break BC in 5.x, so the way to go is:
- implement underlying changes
It already is in master since a few weeks.
- restore backwards compatibility (HTTP_RAW_POST_DATA should be
auto-populated in all cases when it is populated now)That defeats the entire change.
always_populate_raw_post_data should be introduced for BC purposes
Is it possible to detect usage of HTTP_RAW_POST_DATA and give
E_DEPRECATEDwhile doing so?AFAICS no, it is a standard global variable.
- remove HTTP_RAW_POST_DATA support in master (thinking about 6.x)
It already is in master.
I guess Alexey is trying to say that we could deprecate the usage of
$HTTP_RAW_POST_DATA in 5.6, but this feature should go(stay) to master(and
target the next major) as it breaks BC, which we can't do in a minor
version.
And while I agree with it, let's see what others think.
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
I guess Alexey is trying to say that we could deprecate the usage of
$HTTP_RAW_POST_DATA in 5.6, but this feature should go(stay) to master(and
target the next major) as it breaks BC, which we can't do in a minor
version.
Ah well, ok -- I was a bit confused by this list.
Anyway, we cannot E_DEPRECATE $HTTP_RAW_POST_DATA because it is a mean
global variable.
And while I agree with it, let's see what others think.
--
Regards,
Mike
- restore backwards compatibility (HTTP_RAW_POST_DATA should be
auto-populated in all cases when it is populated now)That defeats the entire change.
Not really.
We would still have a new implementation, which has a smaller diff to
master, so can be kept in sync.
I know the stance against new ini-settings, but I wonder if we could
introduce an ini-setting which would be equivalent to python's
"from future import …"
If this is an option, then HTTP_RAW_POST_DATA could be optimized away
when this option is turned on (default is obviously "Off").
always_populate_raw_post_data should be introduced for BC purposes
Is it possible to detect usage of HTTP_RAW_POST_DATA and give
E_DEPRECATEDwhile doing so?AFAICS no, it is a standard global variable.
that leaves us with deprecation via documentation, then
--
Alexey Zakhlestin
CTO at Grids.by/you
https://github.com/indeyets
PGP key: http://indeyets.ru/alexey.zakhlestin.pgp.asc
2013/11/19 Alexey Zakhlestin indeyets@gmail.com
- restore backwards compatibility (HTTP_RAW_POST_DATA should be
auto-populated in all cases when it is populated now)That defeats the entire change.
Not really.
We would still have a new implementation, which has a smaller diff to
master, so can be kept in sync.I know the stance against new ini-settings, but I wonder if we could
introduce an ini-setting which would be equivalent to python's
"from future import …"If this is an option, then HTTP_RAW_POST_DATA could be optimized away
when this option is turned on (default is obviously "Off").
always_populate_raw_post_data should be introduced for BC purposes
Is it possible to detect usage of HTTP_RAW_POST_DATA and give
E_DEPRECATEDwhile doing so?AFAICS no, it is a standard global variable.
that leaves us with deprecation via documentation, then
--
Alexey Zakhlestin
CTO at Grids.by/you
https://github.com/indeyets
PGP key: http://indeyets.ru/alexey.zakhlestin.pgp.asc
My take on it is just to leave the always_populate_raw_post_data in place,
because it is Off by default already.
Modify the docs to reflect that instead of activating the
HTTP_RAW_POST_DATA they should read the php://input stream. Make it clear
that it is deprecated as of 5.6 and with next major release it will be
removed.
Maybe a deprecation warning could be displayed as a startup error, just
like the timezone warnings? So that the scripts themselves are not
affected, but you see a clear warning when launching PHP (I've fixed my
timezones immediately when got the warning)
My 0.02$
Arvids.
My take on it is just to leave the always_populate_raw_post_data in place,
because it is Off by default already.
That's not enough. As far as I understand, even when this option is set
to "Off", HTTP_RAW_POST_DATA would be populated for unknown mime-types
see
http://docs.php.net/manual/en/ini.core.php#ini.always-populate-raw-post-data
Modify the docs to reflect that instead of activating the
HTTP_RAW_POST_DATA they should read the php://input stream. Make it clear
that it is deprecated as of 5.6 and with next major release it will be
removed.
+1
Maybe a deprecation warning could be displayed as a startup error, just
like the timezone warnings? So that the scripts themselves are not
affected, but you see a clear warning when launching PHP (I've fixed my
timezones immediately when got the warning)
+1
--
Alexey Zakhlestin
CTO at Grids.by/you
https://github.com/indeyets
PGP key: http://indeyets.ru/alexey.zakhlestin.pgp.asc
2013/11/19 Alexey Zakhlestin indeyets@gmail.com
My take on it is just to leave the always_populate_raw_post_data in
place,
because it is Off by default already.That's not enough. As far as I understand, even when this option is set
to "Off", HTTP_RAW_POST_DATA would be populated for unknown mime-typessee
http://docs.php.net/manual/en/ini.core.php#ini.always-populate-raw-post-data
--
Alexey Zakhlestin
CTO at Grids.by/you
https://github.com/indeyets
PGP key: http://indeyets.ru/alexey.zakhlestin.pgp.asc
Doh, not careful enough reading is to blame :)
Then I see 2 ways of doing it:
- Adding just the deprecation warnings.
- Besides the warnings, add a new valid value for
always-populate-raw-post-data - "fully_disabled" or just
"disabled"|"disable" to completely turn it off, so people can actually test
how their apps will behave.
Arvids.
Hi Julien,
Please, remember that our release process forbidds BC breaks.
I suggest reintroducing the feature, together with
always_populate_raw_post_data (like the old behavior).What we need is vote for the change?
Mike, could you prepare RFC whether we keep the ini setting or just use
"input://".
I think it's time to vote.Here you go, https://wiki.php.net/rfc/slim_post_data
Though, with only two supporters, chances are low.
This is not a new feature per-se.
It makes php://input reusable, right (in addition to the memory
improvements)? That sounds like a feature to me :)
Nikita
Hi Julien,
Please, remember that our release process forbidds BC breaks.
I suggest reintroducing the feature, together with
always_populate_raw_post_data (like the old behavior).What we need is vote for the change?
Mike, could you prepare RFC whether we keep the ini setting or just use
"input://".
I think it's time to vote.Here you go, https://wiki.php.net/rfc/slim_post_data
Though, with only two supporters, chances are low.--
Regards,
Mike
After thinking about this, I think I figured out a way to provide this
improvement in 5.6 without the BC break and marking the old behavior
deprecated so people using it can prepare for the removal of
$HTTP_RAW_POST_DATA.
before I describe it, here is some context, feel free to point out any
error/misunderstanding:
- $HTTP_RAW_POST_DATA is a global, but not a superglobal, which means we
can't really track and trigger anE_DEPRECATEDwhen accessed, because that
would add an overhead for any access for global variables. - $HTTP_RAW_POST_DATA (up until the introduction of this improvement)
will be populated if enable_post_data_reading is not set to Off(this ini is
present since php 5.4) and any of these conditions are met: - the always_populate_raw_post_data ini is set to true
- php is provided with an unrecognized MIME/content type
- the change in master(which was then branched to 5.6) removed
the always_populate_raw_post_data ini settings and the $HTTP_RAW_POST_DATA
global, so it will be never automatically populated.
- this breaks BC
Here is what I would propose:
- reintroduce always_populate_raw_post_data, but turn it into an integer
from a bool to an entry, which can take -1/0/1 as possible values, where 0
and 1 are the old On and Off behavior, and -1 would disable the population
of $HTTP_RAW_POST_DATA completely, this way people who want the performance
gain can opt-in. - reintroduce the population of the $HTTP_RAW_POST_DATA via reading the
data from the new stream which Mike introduced, this way we would keep BC
for those who doesn't explicitly opt-in for this new feature. - trigger an
E_DEPRECATEDerror when the always_populate_raw_post_data
ini setting is (explicitly or implicitly via not specifying) is set to
anything else but -1, with an error message, which tells people that
$HTTP_RAW_POST_DATA will go away in the future.
what do you think?
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
After thinking about this, I think I figured out a way to provide this
improvement in 5.6 without the BC break and marking the old behavior
deprecated so people using it can prepare for the removal of
$HTTP_RAW_POST_DATA.
before I describe it, here is some context, feel free to point out any
error/misunderstanding:$HTTP_RAW_POST_DATA is a global, but not a superglobal, which means we can't
really track and trigger anE_DEPRECATEDwhen accessed, because that would
add an overhead for any access for global variables.
$HTTP_RAW_POST_DATA (up until the introduction of this improvement) will be
populated if enable_post_data_reading is not set to Off(this ini is present
since php 5.4) and any of these conditions are met:the always_populate_raw_post_data ini is set to true
php is provided with an unrecognized MIME/content typethe change in master(which was then branched to 5.6) removed the
always_populate_raw_post_data ini settings and the $HTTP_RAW_POST_DATA
global, so it will be never automatically populated.
this breaks BCHere is what I would propose:
reintroduce always_populate_raw_post_data, but turn it into an integer from
a bool to an entry, which can take -1/0/1 as possible values, where 0 and 1
are the old On and Off behavior, and -1 would disable the population of
$HTTP_RAW_POST_DATA completely, this way people who want the performance
gain can opt-in.
reintroduce the population of the $HTTP_RAW_POST_DATA via reading the data
from the new stream which Mike introduced, this way we would keep BC for
those who doesn't explicitly opt-in for this new feature.
trigger anE_DEPRECATEDerror when the always_populate_raw_post_data ini
setting is (explicitly or implicitly via not specifying) is set to anything
else but -1, with an error message, which tells people that
$HTTP_RAW_POST_DATA will go away in the future.what do you think?
Sounds doable! I'd implement the INI with 1, "On", 0, "Off", and -1,
"Never" settings, if nobody objects (the first two for BC and "Never"
to complement).
--
Regards,
Mike
After thinking about this, I think I figured out a way to provide this
improvement in 5.6 without the BC break and marking the old behavior
deprecated so people using it can prepare for the removal of
$HTTP_RAW_POST_DATA.
before I describe it, here is some context, feel free to point out any
error/misunderstanding:$HTTP_RAW_POST_DATA is a global, but not a superglobal, which means we
can't
really track and trigger anE_DEPRECATEDwhen accessed, because that
would
add an overhead for any access for global variables.
$HTTP_RAW_POST_DATA (up until the introduction of this improvement) will
be
populated if enable_post_data_reading is not set to Off(this ini is
present
since php 5.4) and any of these conditions are met:the always_populate_raw_post_data ini is set to true
php is provided with an unrecognized MIME/content typethe change in master(which was then branched to 5.6) removed the
always_populate_raw_post_data ini settings and the $HTTP_RAW_POST_DATA
global, so it will be never automatically populated.
this breaks BCHere is what I would propose:
reintroduce always_populate_raw_post_data, but turn it into an integer
from
a bool to an entry, which can take -1/0/1 as possible values, where 0
and 1
are the old On and Off behavior, and -1 would disable the population of
$HTTP_RAW_POST_DATA completely, this way people who want the performance
gain can opt-in.
reintroduce the population of the $HTTP_RAW_POST_DATA via reading the
data
from the new stream which Mike introduced, this way we would keep BC for
those who doesn't explicitly opt-in for this new feature.
trigger anE_DEPRECATEDerror when the always_populate_raw_post_data ini
setting is (explicitly or implicitly via not specifying) is set to
anything
else but -1, with an error message, which tells people that
$HTTP_RAW_POST_DATA will go away in the future.what do you think?
Sounds doable! I'd implement the INI with 1, "On", 0, "Off", and -1,
"Never" settings, if nobody objects (the first two for BC and "Never"
to complement).
sorry, of course I meant 0 for Off and 1 for On.
thanks for your help!
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
Michael Wallner wrote:
what do you think?
Sounds doable! I'd implement the INI with 1, "On", 0, "Off", and -1,
"Never" settings, if nobody objects (the first two for BC and "Never"
to complement).
I'd probably go with 2 rather than -1, just in case we want to turn it
into a bitmask in the future (unsigned seems better than signed to me
anyway).
--
Ryan McCue
<http://ryanmccue.info/
Hi all,
Michael Wallner wrote:
what do you think?
Sounds doable! I'd implement the INI with 1, "On", 0, "Off", and -1,
"Never" settings, if nobody objects (the first two for BC and "Never"
to complement).I'd probably go with 2 rather than -1, just in case we want to turn it
into a bitmask in the future (unsigned seems better than signed to me
anyway).
Any positive number is max size of input, right?
Is this correct?
Then, 0 for unlimited and -1 for disabled sounds reasonable to me.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Any positive number is max size of input, right?
Is this correct?Then, 0 for unlimited and -1 for disabled sounds reasonable to me.
Wat? :)
If, then only -1==unlimited, 0==off; but nope, I don't plan any
additonal new behavior.
--
Regards,
Mike
Hi Mike,
Any positive number is max size of input, right?
Is this correct?Then, 0 for unlimited and -1 for disabled sounds reasonable to me.
Wat? :)
If, then only -1==unlimited, 0==off; but nope, I don't plan any
additonal new behavior.
No problem. Getting partial data does not make much sense either :)
Someone may find useful usage of getting partial data.
Perhaps, -1==off, 0==unlimited?
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi Mike,
Any positive number is max size of input, right?
Is this correct?Then, 0 for unlimited and -1 for disabled sounds reasonable to me.
Wat? :)
If, then only -1==unlimited, 0==off; but nope, I don't plan any
additonal new behavior.No problem. Getting partial data does not make much sense either :)
Someone may find useful usage of getting partial data.Perhaps, -1==off, 0==unlimited?
Yasuo, you're kidding me, aren't you? :)
Why should 0x0000 be On and 0xffff be Off?
--
Regards,
Mike
Hi Mike,
Yasuo, you're kidding me, aren't you? :)
Why should 0x0000 be On and 0xffff be Off?
Ok, I guess it's unsigned :)
There such implementations 0 to positive number enabling for that range,
and negative to disable.
Anyway, I would be too late, but we already have output_buffer INI setting
as long.
On/Off or size of buffer. (On = 4096) IIRC
It might be better to be consistent with existing parameter.
Just my .02c
It does not matter much as user should use On/Off anyway.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
2013.11.26. 1:03, "Ryan McCue" lists@rotorised.com ezt írta:
Michael Wallner wrote:
what do you think?
Sounds doable! I'd implement the INI with 1, "On", 0, "Off", and -1,
"Never" settings, if nobody objects (the first two for BC and "Never"
to complement).I'd probably go with 2 rather than -1, just in case we want to turn it
into a bitmask in the future (unsigned seems better than signed to me
anyway).--
Ryan McCue
I don't think there is any chance that we would want to turn it into a
bitmask, as the values are mutually exclusive.
And the ini should go away with the next major anyways.
And the ini should go away with the next major anyways.
Here you go:
https://github.com/m6w6/php-src/compare/php:PHP-5.6...always_populate_raw_post_data
--
Regards,
Mike
And the ini should go away with the next major anyways.
Here you go:
https://github.com/m6w6/php-src/compare/php:PHP-5.6...always_populate_raw_post_data
--
Regards,
Mike
looks good, could you add a new test for always_populate_raw_post_data=-1?
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
looks good, could you add a new test for always_populate_raw_post_data=-1?
Done.
--
Regards,
Mike
looks good, could you add a new test for always_populate_raw_post_data=-1?
Done.
The RFC has been updated as well:
https://wiki.php.net/rfc/slim_post_data
--
Regards,
Mike
2013.11.28. 9:42, "Mike" mike.php.net@gmail.com ezt írta:
looks good, could you add a new test for
always_populate_raw_post_data=-1?Done.
The RFC has been updated as well:
https://wiki.php.net/rfc/slim_post_data--
Regards,
Mike
Hi Mike,
I think it would be nice to either reconstruct or split the RFC.
Currently it is a bit hard to tell, that what is changed in which version.
I think it would make sense, to have an RFC for 5.6 about introducing the
new stream, the new ini option and and how does it keeps userland BC while
providing a way to opt-in for the performance gain, and that it introduces
an E_DEPRECATED so we can remove the ini and the raw post global variable
in the next major version.
Ps: posting from my phone, sorry for the typos and for not using the actual
ini/variable names.
Hi Mike,
I think it would be nice to either reconstruct or split the RFC.
Currently it is a bit hard to tell, that what is changed in which
version.
I changed to RFC to be easier comprehensible (hopefully).
I think it would make sense, to have an RFC for 5.6 about introducing
the new stream, the new ini option and and how does it keeps userland
BC while providing a way to opt-in for the performance gain, and that
it introduces anE_DEPRECATEDso we can remove the ini and the raw
post global variable in the next major version.
I think we cannot add E_DEPRECATED to the INI-setting unless we make its
default -1... which breaks BC :p
Also, I think two RFCs would be quite much, but I already gave up my
renitency.
--
Regards,
Mike
Hi Mike,
I think it would be nice to either reconstruct or split the RFC.
Currently it is a bit hard to tell, that what is changed in which
version.I changed to RFC to be easier comprehensible (hopefully).
thanks!
I think it would make sense, to have an RFC for 5.6 about introducing
the new stream, the new ini option and and how does it keeps userland
BC while providing a way to opt-in for the performance gain, and that
it introduces anE_DEPRECATEDso we can remove the ini and the raw
post global variable in the next major version.I think we cannot add
E_DEPRECATEDto the INI-setting unless we make its
default -1... which breaks BC :pAlso, I think two RFCs would be quite much, but I already gave up my
renitency.
we could emmit the error from populate_raw_post_data in
main/php_content_types.c, telling people that the auto population of
$HTTP_RAW_POST_DATA along with the use of always_populate_raw_post_data
will go away, and they should set always_populate_raw_post_data to -1 and
populate that variable for themself if their want to be forward compatible
(and silence the E_DEPRECATED messages).
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
I think it would make sense, to have an RFC for 5.6 about introducing
the new stream, the new ini option and and how does it keeps userland
BC while providing a way to opt-in for the performance gain, and that
it introduces anE_DEPRECATEDso we can remove the ini and the raw
post global variable in the next major version.I think we cannot add
E_DEPRECATEDto the INI-setting unless we make its
default -1... which breaks BC :pAlso, I think two RFCs would be quite much, but I already gave up my
renitency.we could emmit the error from populate_raw_post_data in
main/php_content_types.c, telling people that the auto population of
$HTTP_RAW_POST_DATA along with the use of always_populate_raw_post_data
will go away, and they should set always_populate_raw_post_data to -1 and
populate that variable for themself if their want to be forward compatible
(and silence theE_DEPRECATEDmessages).
Hi,
Any update on this?
I would be ok to merge with the PR in the current form, as it resolves the
BC issue, but if we want to remove the INI setting in the future, then
adding the E_DEPRECATED would be important step.
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
I think it would make sense, to have an RFC for 5.6 about introducing
the new stream, the new ini option and and how does it keeps userland
BC while providing a way to opt-in for the performance gain, and that
it introduces anE_DEPRECATEDso we can remove the ini and the raw
post global variable in the next major version.I think we cannot add
E_DEPRECATEDto the INI-setting unless we make its
default -1... which breaks BC :pAlso, I think two RFCs would be quite much, but I already gave up my
renitency.we could emmit the error from populate_raw_post_data in
main/php_content_types.c, telling people that the auto population of
$HTTP_RAW_POST_DATA along with the use of always_populate_raw_post_data
will go away, and they should set always_populate_raw_post_data to -1 and
populate that variable for themself if their want to be forward compatible
(and silence theE_DEPRECATEDmessages).Hi,
Any update on this?
I would be ok to merge with the PR in the current form, as it resolves the
BC issue, but if we want to remove the INI setting in the future, then
adding theE_DEPRECATEDwould be important step.--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
bump.
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
>
>
>
> On Sat, Dec 7, 2013 at 8:39 AM, Ferenc Kovacs
> wrote:
>
>
> > I think it would make sense, to have an RFC
> for 5.6 about introducing
> > the new stream, the new ini option and and
> how does it keeps userland
> > BC while providing a way to opt-in for the
> performance gain, and that
> > it introduces an `E_DEPRECATED` so we can
> remove the ini and the raw
> > post global variable in the next major
> version.
>
>
> I think we cannot add `E_DEPRECATED` to the
> INI-setting unless we make its
> default -1... which breaks BC :p
>
> Also, I think two RFCs would be quite much,
> but I already gave up my
> renitency.
>
>
> we could emmit the error from populate_raw_post_data
> in main/php_content_types.c, telling people that the
> auto population of $HTTP_RAW_POST_DATA along with the
> use of always_populate_raw_post_data will go away, and
> they should set always_populate_raw_post_data to -1
> and populate that variable for themself if their want
> to be forward compatible (and silence the `E_DEPRECATED`
> messages).
>
>
>
>
>
> Hi,
>
>
> Any update on this?
> I would be ok to merge with the PR in the current form, as it
> resolves the BC issue, but if we want to remove the INI
> setting in the future, then adding the `E_DEPRECATED` would be
> important step.
>
Ok, I'll change that and open for votes then!
Thanks,
Mike
And the ini should go away with the next major anyways.
Here you go:
https://github.com/m6w6/php-src/compare/php:PHP-5.6...always_populate_raw_post_data
Can you update php.ini-* in the patch too?
--
christopher.jones@oracle.com http://twitter.com/ghrd
Free PHP & Oracle book:
http://www.oracle.com/technetwork/topics/php/underground-php-oracle-manual-098250.html
On Mon, Dec 2, 2013 at 8:45 PM, Christopher Jones <
christopher.jones@oracle.com> wrote:
And the ini should go away with the next major anyways.
Here you go:
https://github.com/m6w6/php-src/compare/php:PHP-5.6...
always_populate_raw_post_dataCan you update php.ini-* in the patch too?
--
christopher.jones@oracle.com http://twitter.com/ghrd
Free PHP & Oracle book:
http://www.oracle.com/technetwork/topics/php/
underground-php-oracle-manual-098250.html--
you mean to mention the deprecation right?
because the default/suggested values shouldn't be changed imo (as some
distros are following those, and it could break code in the userland if we
change it).
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu