Re: [PHP-DEV] Re: [RFC Discussion] Precise Session Management
Hi Mike, On Mon, Dec 21, 2015 at 1:24 PM, Mike Willbanks wrote: > > On Sun, Dec 20, 2015 at 7:01 PM, Yasuo Ohgaki wrote: >> >> Hi all, >> >> On Sat, Dec 19, 2015 at 7:33 AM, Yasuo Ohgaki wrote: >> > I would like to restart better session management for PHP 7.1. >> > >> > https://wiki.php.net/rfc/precise_session_management >> > >> > Although this RFC targets PHP 7.1, new session management >> > could be applied to older releases also if majority of us agree. >> > Please comment. >> >> I would like to write patch for this next week. >> If you have comment, please comment this week. > > > This week is hard due to several holidays, I would recommend postponing > discussion until after. Thanks. I'm working on this for years. I'm not in hurry. > > > However, I will comment on a few things that I dislike of the RFC: > > Exposing the internal state of the session via a key on the session > __SESSION_INTERNAL__ may be dangerous. How are you preventing writes to > this area? Is an exception or error thrown? I also do not feel that it is The session internal data structure (__SESSION_INTERNAL__) is removed when the data is unserialized. It is added back when session data is serialized. So user cannot see/touch private session data at all. If user add $_SESSION['__SESSION_INTERNAL__'] in user script, the module remove it and raise E_WARNING. > worth encoding this directly into the session value but would be of far > greater benefit to expose through functions and ensure it is not touched and > protected from user land. Anything that messes with the $_SESSION can cause > major issues (for instance upload progress did this and can cause session > corruption in certain cases as it manipulates the session state). Please file a bug report for this. As I wrote in previously, $_SESSION['__SESSION_INTERNAL__'] is completely hidden and untouchable from users > I fully agree that session_regenerate_id needs some additional work. > Although. I do not think that the implementation here seems like the correct > path as a general comment. >From user point of view, $_SESSION['__SESSION_INTERNAL__'] is a new reserved/ restricted session key. We had such restriction before "php_serialize" serialize option. e.g. $_SESSION['1'] , $_SESSION['1abc'], etc are invalid keys and raises confusing error messages. Key name must be valid as variable name for serialization method other than "php_serialize". I could choose to use completely new session serialization method like $__PSEUDO_SESSION_DATA__ = $SESSION_INTERNAL_DATA[] + $_SESSION[] where $SESSION_INTERNAL_DATA is $_SESSION['__SESSION_INTERNAL__'] and $_SESSION is the current session data array. No reserved key is required with this. However, this method breaks almost all 3rd party session data management libraries that access session data storage directly. Therefore, I chose to add and hide session internal data structure in $_SESSION. However, this could be vote option. Thank you for your comment! Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: [RFC Discussion] Precise Session Management
Hi all, On Sat, Dec 19, 2015 at 7:33 AM, Yasuo Ohgaki wrote: > I would like to restart better session management for PHP 7.1. > > https://wiki.php.net/rfc/precise_session_management > > Although this RFC targets PHP 7.1, new session management > could be applied to older releases also if majority of us agree. > Please comment. I would like to write patch for this next week. If you have comment, please comment this week. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [RFC Discussion] Precise Session Management
Hi all, I would like to restart better session management for PHP 7.1. https://wiki.php.net/rfc/precise_session_management Although this RFC targets PHP 7.1, new session management could be applied to older releases also if majority of us agree. Please comment. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Benchmark Results for PHP Master 2015-12-17
Hi Martin, On Thu, Dec 17, 2015 at 7:24 PM, Martin Keckeis wrote: > > The results should better be collected in a database and displayed with > helpful graphs or something like that. > A daily mail with no long term change does not really provide a good source > of information. The objective of this benchmark is to alert developers accidental performance drop. Even small change may affect performance a lot. This would be good enough for the purpose. IMO. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Travis CI is failing
Hi all, On Wed, Dec 16, 2015 at 12:32 AM, Xinchen Hui wrote: > most of them relates to session, Yasuo, could you please have a look into > them? > > thanks I suppose it's back to original state now. FPM test fails on PHP 5.6 ZTS build. = FAILED TEST SUMMARY - FPM: Test status page [sapi/fpm/tests/010.phpt] = FPM maintainer, please handle this. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Travis CI is failing
Hi Xinchen, On Wed, Dec 16, 2015 at 12:32 AM, Xinchen Hui wrote: > On Tue, Dec 15, 2015 at 7:58 PM, Lior Kaplan wrote: >> >> Hi, >> >> Travis CI reports show quite a lot of failures with recent commits to few >> of the branches. >> From a quick look it fails due to tests and not due to compilation issues. >> >> Please check the status of the build/tests after your commits: >> https://travis-ci.org/php/php-src/branches >> >> At the moment ext/session/tests/bug32330.phpt fails for 5.6, 7.0 and >> master. > > most of them relates to session, Yasuo, could you please have a look into > them? Thank you. I'll fix it now. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: PHP 5.6 life cycle
Hi all, On Tue, Dec 15, 2015 at 8:43 AM, Stanislav Malyshev wrote: > >> Furthermore, postponing EOL now means reduced pressure to upgrade to 7. > > Again, if you think people's decision to upgrade significantly depends > on that, I think you are deluding yourself. We have tons of "pressure" > to upgrade from EOLed 5.x versions. We still got majority of people > running them, and more sites running 5.2 than 5.6, judging by > http://w3techs.com/technologies/details/pl-php/5/all Upgrading extensions from PHP 5.X to PHP 5.Y is easy or even does not require any changes. Since upgrading extension modules to PHP 7 code base is not easy task for many people, giving more time for upgrading makes sense. We don't know how many private extensions are used in the wild. http://w3techs.com/technologies/details/pl-php/5/all This may be an evidence that there are many. +1 for extending PHP 5.6 support. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] HashDos protection
Hi Thomas, On Sat, Nov 28, 2015 at 2:07 AM, Thomas Hruska wrote: > I don't know if anyone has suggested this before, but why not have a > function that application developers can call to switch hash modes and > support multiple hash modes in the core? > > I've always viewed 'max_input_vars' as an emergency hack and I've run into > the default 1,000 limit many times. When I hit that limit, I inevitably > have to raise it to anywhere from 3,000 to 10,000 to get the target > application to function, which, of course, puts the whole server at risk. Because any hash functions have collisions. Even if we use stronger hash against collisions, computers are getting faster and faster, creating colliding key datatabease becomes easier and easier. Clever person may find algolithmic way to generate colliding keys in the future also. In practice, we wouldn't have problems with max number of collisions. Max number of collisions resolves the issue for good and we may execute code faster with faster hash. I forgot the number but SipHash is much slower than DJB. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] HashDos protection
Hi Nikita, On Fri, Nov 27, 2015 at 2:24 AM, Nikita Popov wrote: > What are your thoughts on this? Great! This is exactly what I was thinking. I prefer collision counting rather than slower hash function. Hardcoded collision max (1000) seems ok for me. Catchable fatal error, at least E_RECOVERABLE_ERROR, is preferred, IMHO. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] [DISCUSSION] More precise float value
Hi all and Ferenc, On Mon, Sep 28, 2015 at 4:51 AM, Anatol Belski wrote: >> -Original Message- >> From: jakub@gmail.com [mailto:jakub@gmail.com] On Behalf Of Jakub >> Zelenka >> > Exactly, so my question was - why it still needs to do "if (mode == 0) >> > ndigit = 17;" in snprintf at the place I've linked? It won't have any >> > effect as zend_dtoa will ignore it :) >> > >> >> As I said in the PR some time ago, it's not used by dtoa for mode 0 but it's >> still >> used by php_gcvt for checking if exponential or decimal notation should be >> used. See >> >> https://github.com/php/php- >> src/blob/250938e2d35fc54161a18167b7901c5e3b574371/main/snprintf.c#L163 >> > Ah, I see it now. The patch is quite sparse, so have to check the whole > functions. Thanks for the explanations. > > For 7.0 raising the precision INIs to max supported were probably OK, but no > new INI options should be added. And IMHO, there should be only one INI > option change, and preferably full patch with json_precision in 7.1 and not > touching other branches (the simplest solution). As it seems, one sees it > critical to start using 17 digits in 7.0 - then probably vote were eligible > (whereby I wouldn't see it critical). But then IMHO - switch to > serialize_precision in 7.0 and keep the name for 7.1 with full patch. Though > please be aware that times are going fast, if you decide to put something 7.0 > related to the vote, be sure the results stand till RC5. Thank you for explanation, Jakub. Ferenc, are you OK with the JSON serialize precision change? i.e. Use PG(serialize_precision) rather than EG(precision) when json_serialize() is called. If yes, I'll prepare the patch and start the vote immediately. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] new foo()->bar() syntax error : "new" associativity
Hi all, https://bugs.php.net/bug.php?id=70549 It seems this is better to be fixed in near future. Let "new" be associative? Currently, it's syntax error. Was there discussion for this with https://wiki.php.net/rfc/uniform_variable_syntax ? Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] [DISCUSSION] More precise float value
Hi all, On Fri, Sep 25, 2015 at 7:49 AM, Yasuo Ohgaki wrote: > No problem. I'll update so that 0 mode is for 7.1. > JSON's is better to use larger precision. So this change is targeted > to 5.6 and 7.0. > > Let me know if you have comments on this. The RFC is updated. https://wiki.php.net/rfc/precise_float_value Patches for 7.1/7.0/5.6 will be updated soon. 7.0/5.6's change will be one liner. i.e. Simply change to use PG(serialize_precision) instead of EG(precision). Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] [DISCUSSION] More precise float value
Hi Anatol, On Tue, Sep 22, 2015 at 4:35 AM, Anatol Belski wrote: >> -Original Message- >> From: yohg...@gmail.com [mailto:yohg...@gmail.com] On Behalf Of Yasuo >> Ohgaki >> Sent: Sunday, September 20, 2015 11:49 PM >> To: Kalle Sommer Nielsen >> Cc: Anatoliy Belsky ; Ferenc Kovacs ; >> internals@lists.php.net >> Subject: Re: [PHP-DEV] Re: [RFC] [DISCUSSION] More precise float value >> >> Hi Anatol and Ferenc, >> >> On Sat, Sep 19, 2015 at 7:15 AM, Kalle Sommer Nielsen wrote: >> > I'm sorry I have not caught this any earlier, but even without >> > consulting with Anatol, I'm not keen on adding this into 7.0, we are >> > in RC stage, and I want us to focus on a stable release instead of >> > adding more potential issues, I suggest you target 7.1 instead in the >> > RFC. >> > >> > (Ferenc and Anatol cc'd) >> >> I was expecting this kind of reply for this RFC. >> What do you think? >> >> This RFC contains JSON float value precision change for PHP 5.6 so that PHP >> would not loose floating number information, as well as more precise float >> handling option for PHP 7.0. >> >> FYI, var_dump()'s float precision was changed to use serialize_precision as >> bug >> fix before. >> > Thanks for the ping. IMHO this is the stuff for 7.1. I actually saw the > discussions previously, but was rather thinking you was targeting 7.1 as that > was already the time of the feature freeze. No problem. I'll update so that 0 mode is for 7.1. JSON's is better to use larger precision. So this change is targeted to 5.6 and 7.0. Let me know if you have comments on this. > > Please keep in mind that we effectively have two RCs to do the remaining > stabilizations. And we probably should as much as possible. Because this > patch it's rather an enhancement to the current functionality which is not > indeed broken - I wouldn't see it as critical for 7.0. It should be voted > (and discussed if necessary) for 7.1, so we don't loop away from our current > situation. > > BTW I also had a comment there in the PR about > https://github.com/php/php-src/pull/1455/files#diff-c84859666ff690a113d55ef1899d8bf4R149 > - so if mode 0 ignores ndigits, why set it to 17 (or anything else). Seems > like an unnecessary action, but snprintf is used quite frequently. But just > as a notice on the side. "-1" which is invalid precision is used to indicate 0 mode. 0 mode is supposed to use max precision for double (it seems many implementations use 16, but PHP uses 17), when 0 mode is used, it uses 17 as precision always. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] [DISCUSSION] More precise float value
Hi Anatol and Ferenc, On Sat, Sep 19, 2015 at 7:15 AM, Kalle Sommer Nielsen wrote: > I'm sorry I have not caught this any earlier, but even without > consulting with Anatol, I'm not keen on adding this into 7.0, we are > in RC stage, and I want us to focus on a stable release instead of > adding more potential issues, I suggest you target 7.1 instead in the > RFC. > > (Ferenc and Anatol cc'd) I was expecting this kind of reply for this RFC. What do you think? This RFC contains JSON float value precision change for PHP 5.6 so that PHP would not loose floating number information, as well as more precise float handling option for PHP 7.0. FYI, var_dump()'s float precision was changed to use serialize_precision as bug fix before. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: [RFC] [DISCUSSION] More precise float value
Hi all, On Fri, Sep 4, 2015 at 9:41 AM, Yasuo Ohgaki wrote: > IEEE 754 double cannot express exact float values. That said, > float values expressed by json/serialize/var_export/echo/print > are not precise enough in many cases. > > Issues: > - json_encode() uses EG(precision)=14 that truncates float values. > echo()/print() >does this as well. > - large EG(precision)/PG(serialize_precision) prints meaningless values. > > This RFC proposes zend_dtoa()'s 0 mode support which rounds float value > to nearest value. > > https://wiki.php.net/rfc/precise_float_value > https://github.com/php/php-src/pull/1455 > > This change is simple enough for PHP 7.0. IMO. > Comments/suggestions are appreciated! It's been 2 weeks and no comment for this. I'll start voting. Thanks. -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Make strict mode more strict?
Hi all, PHP 7 has strict_types mode for function parameters/return values and these are binded to certain type strictly. https://wiki.php.net/rfc/scalar_type_hints_v5 Why not make strict_types mode more strict? The idea is as follows: Assigning different type to already initialized variable is a bug most likely. There may be cases that variable should have several types, e.g. return INT for success and FALSE for failure, but programmers can use different variable or return value directly or make BOOL/NULL exception. This is useful with reference especially. For example, outputs string(6) "string" Just an idea. Any comments? -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] newbie question
Hi Andrei, On Wed, Sep 16, 2015 at 9:35 AM, Andrei Bernardo Simoni wrote: > HelloIm new in the PHP project and i would like to know where I can start.To > quench my curiosity I wonder which part of the code read the files with > extension .php If you are looking for some code execution path, you are better to use gdb (or any other debugger). script: http://www.php.net/unsub.php
[PHP-DEV] Re: Invalid date/time handling - strtotime()
Hi Derick,
On Wed, Sep 9, 2015 at 1:31 AM, Derick Rethans wrote:
> Currently, it works as follows by design:
>
> - the parser, allows for each unit (year, month, day, hour, minute,
> second) the full range of values. For a year that's just 4 digits, for
> a month that's 0-12, day is 0-31 and for hour and minute it's 0-59.
>
> - 60 is allowed for seconds, as sometimes date strings with that
> leapsecond do show up. But PHP implements Unix time where "60" is not
> a valid second number and hence it overflows. There is some more
> background information at http://drck.me/leapsec-6ye
>
> - strtotime() returns false if any number is outside of the ranges, and
> new DateTime() throws an exception.
>
> - the parser is dumb, and doesn't do any checks to make it faster (and
> more generic)
>
> - *but*, there is an additional check if you pass in an invalid date
> (like your suggested $strict):
>
> $ php -r '$res = date_parse("2015-09-31"); var_dump($res["warnings"]);'
>
> array(1) {
> [11] =>
> string(27) "The parsed date was invalid"
> }
>
> - It is already possible to handle the edge cases, but then you need to
> supply the right format. Otherwise there are too many possible
> confusing and unintuitive results coming out of the parser:
>
> php -r '$res = date_create_from_format("Y-m-d", "2015-09-34");
> var_dump($res);'
>
> class DateTime#1 (3) {
> public $date =>
> string(26) "2015-10-04 17:24:43.00"
> public $timezone_type =>
> int(3)
> public $timezone =>
> string(13) "Europe/London"
> }
>
> - We can't add a second argument to strtotime(), because it already
> exists.
>
> - Having a "strict" option to strtotime() does also not work, as that
> would mean two distinct parsing routines.
>
> With all those concerns, as well as functionality to handle edge cases
> in place, I do not think we should change anything.
Thank you for detailed explanation.
I'm OK with current design/API. How about add documentation for more precise
date/time parsing and handling to strtotime() manual page?
One concern is "-00-00" date. MySQL uses it as invalid date and strtotime()
produces non intuitive result. It should be added to the document also.
I made a bug report to track this.
https://bugs.php.net/bug.php?id=70463
Regards,
--
Yasuo Ohgaki
yohg...@ohgaki.net
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Invalid date/time handling - strtotime()
Hi Derick and all,
strtotime() may be improved.
$ php -r "echo date('Y-m-d',strtotime('00-00-00 00:00:00'));"
1999-11-30
because 00 is 2000, 00 month is -1(=12) and 00 date is -1(last date of
previous month).
This is unintuitive result at a glance.
strtotime() handles date/time edge cases and produces reasonable timestamp
in some cases.
$ php -r "echo date('Y-m-d H:i:s',strtotime('00-01-01 24:00:00'));"
2000-01-02 00:00:00
$ php -r "echo date('Y-m-d H:i:s',strtotime('03-02-29 00:00:00'));"
2003-03-01 00:00:00
$ php -r "echo date('Y-m-d H:i:s',strtotime('03-02-28 23:59:60'));"
2003-03-01 00:00:00
It returns FALSE for larger values exceeds edge cases. For example,
$ php -r "var_dump(strtotime('03-02-32 00:00:00'));"
bool(false)
Somehow it does not accept 13 month while it is edge case.
$ php -r "var_dump(strtotime('03-13-30 00:00:00'));"
bool(false)
It does not accept day 32 while it accepts 2/31.
$ php -r "var_dump(strtotime('03-01-32 00:00:00'));"
bool(false)
$ php -r "var_dump(strtotime('03-02-31 00:00:00'));"
int(1046617200)
25 hour/60 minutes is invalid also.
$ php -r "var_dump(strtotime('03-02-31 00:60:00'));"
bool(false)
$ php -r "var_dump(strtotime('03-02-31 25:00:00'));"
bool(false)
It seems edge case handling is inconsistent.
How about make strtotime()
- Accept all edge cases by default.
- Add $strict flag as 2nd parameter and reject any invalid date/time.
(Do not allow leap second also? Some system may create 60th second)
Alternatively, we may be strict
- Disallow all edge cases by default.
(There may be problem with leap second)
Any comments?
Reference:
https://bugs.php.net/bug.php?id=45647
--
Yasuo Ohgaki
yohg...@ohgaki.net
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [RFC] [DISCUSSION] More precise float value
Hi all, IEEE 754 double cannot express exact float values. That said, float values expressed by json/serialize/var_export/echo/print are not precise enough in many cases. Issues: - json_encode() uses EG(precision)=14 that truncates float values. echo()/print() does this as well. - large EG(precision)/PG(serialize_precision) prints meaningless values. This RFC proposes zend_dtoa()'s 0 mode support which rounds float value to nearest value. https://wiki.php.net/rfc/precise_float_value https://github.com/php/php-src/pull/1455 This change is simple enough for PHP 7.0. IMO. Comments/suggestions are appreciated! Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Heads up: merging security patches to 7
Hi Stas, On Thu, Sep 3, 2015 at 1:56 PM, Stanislav Malyshev wrote: >> I see number of var_push_dtor() to fix unserialization. >> var_push_dtor() or var_push_dtor_no_addref() is required always when >> php_var_unserialize() is failed. >> Am I correct? > > Not necessarily. Basically, what happens is that when you do > php_var_unserialize() the value you unserialize gets a slot in the > reference table. So if you are destroying it and some serialization part > later tries to access it - boom! That's where var_push_* comes in. The > first one adds refcount so even if everybody drops references to it the > value still survives. The second is used when we know no code uses this > value, so no need to add refcount - only thing that should keep it alive > is that it's still in the refs table (except for the case where it is > taken from the table by r: or R: - then its refcount is bumped). > > Now the failure is tricky case - generally after the failure we need to > bail out ASAP, but that failure may not be the final failure - > serializes can be nested. So we may want to keep the values around even > if our unserialize fails, because encompassing serialize may still > reference that variable, since we may have already put it in the refs > table. We could make serialize failure a fatal error, but that'd be > pretty big BC break (exception wouldn't work, we'd need full blown fatal > E_ERROR). > > In general, the idea is that anything that was put in reference table > (namely, anything that passed through php_var_unserialize()) should be > kept alive until the end of unserialize() call. Which means it should be > put to dtor list via var_push_dtor() or var_push_dtor_no_addref(). > > Now, that all was true for PHP 5. For PHP 7, the matter are more > complicated as the lists now keep zvals and not zval *, and I'm not 100% > sure yet what that means. I need to look into it and figure out. Thank you for the detailed explanation! It's very helpful and it seems I'm better to wait for unserialize() patch. I'll finish other parts first. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Heads up: merging security patches to 7
Hi Stas, On Wed, Sep 2, 2015 at 7:17 AM, Yasuo Ohgaki wrote: > There are many fixes regarding unserialize. > We also had many fixes regarding type mismatches. > I suppose many 3rd party modules have same issues. > > How about have a doc for secure PHP internal coding? I'm writing the draft. I see number of var_push_dtor() to fix unserialization. var_push_dtor() or var_push_dtor_no_addref() is required always when php_var_unserialize() is failed. Am I correct? It will cover - Pointers to general secure programming resources - Basic memory management and debugging (how to use run-tests.php) - Unserialization - Type confusion - Typical overflows If there is anything to add, please let me know. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Heads up: merging security patches to 7
Hi Stas, There are many fixes regarding unserialize. We also had many fixes regarding type mismatches. I suppose many 3rd party modules have same issues. How about have a doc for secure PHP internal coding? -- Yasuo Ohgaki yohg...@ohgaki.net On Wed, Sep 2, 2015 at 5:55 AM, Stanislav Malyshev wrote: > Hi! > > I've recently committed a number of fixes to 5.x branch. These fixes > mainly concern (un)serialization scenarios, you can see the full list in > 5.4/5.5 NEWS. These changes are not merged yet to master/7.0 since due > to extensive differences between 5.x and 7 in zval handling, they > basically must be rewritten for 7. I don't want to commit completely > broken code to master, so I'll work on at least getting it to a state > where there is no new breakage and then porting the fixes properly to 7, > but that can take a couple of days. In the meantime, please be aware > that 5.x and master may not be in full sync and exercise caution if you > merge stuff from 5 to 7. > -- > Stas Malyshev > smalys...@gmail.com > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Short Closures
Hi Anthony and Bob,
On Tue, Sep 1, 2015 at 2:53 PM, Anthony Ferrara wrote:
> Most programming languages today have a "short form" closure or lambda syntax
>
> HackLang: ($x) ==> $x + 1;
> C++: [](int x) -> int { return x + 1; }
> Java: (int x) -> x + 1;
> Python: lambda x: x+1
> Ruby: lambda |x| { x + 1 }
> Rust: |x| x + 1
> JavaScript (ES6): x => x + 1
> C#: x => x + 1
> Objective C: ^(int x) { return x + 1; }
Nice summary!
The syntax may look strange at first, but proposed syntax is close enough to
other languages. There will be no barrier for our users in the long run.
I thought scope variables enabled by default is destructive at first,
but Bob clarify
they are passed by value. Therefore, it would not be issue.
Bob, is there reason not to use the same syntax as Hack "==>"?
Regards,
--
Yasuo Ohgaki
yohg...@ohgaki.net
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack
Hi Matt, My friend made a product called sql_firewall for PostgreSQL. https://github.com/uptimejp/sql_firewall http://pgsnaga.blogspot.jp/2015/08/postgresql-sql-firewall.html It's not directly relevant to your proposal, but this kind of database extension could be alternative. PDO has parser for params. Parser may be extended to parse basic SQL syntax and store syntax tree hash like sql_firewall and compare it to reject SQL injection attacks. Just FYI. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net On Wed, Jul 29, 2015 at 2:33 AM, Matt Tait wrote: > Hi all, > > I've written an RFC (and PoC) about automatic detection and blocking of SQL > injection vulnerabilities directly from inside PHP via automated taint > analysis. > > https://wiki.php.net/rfc/sql_injection_protection > > In short, we make zend_strings track where their value originated. If it > originated as a T_STRING, from a primitive (like int) promotion, or as a > concatenation of such strings, it's query that can't have been SQL-injected > by an attacker controlled string. If we can't prove that the query is safe, > that means that the query is either certainly vulnerable to a SQL-injection > vulnerability, or sufficiently complex that it should be parameterized > just-to-be-sure. > > There's also a working proof of concept over here: > > http://phpoops.cloudapp.net/oops.php > > You'll notice that the page makes a large number of SQL statements, most of > which are not vulnerable to SQL injection, but one is. The proof of concept > is smart enough to block that one vulnerable request, and leave all of the > others unchanged. > > In terms of performance, the cost here is negligible. This is just basic > variable taint analysis under the hood, (not an up-front intraprocedurale > static analysis or anything complex) so there's basically no slow down. > > PHP SQL injections are the #1 way PHP applications get hacked - and all SQL > injections are the result of a developer either not understanding how to > prevent SQL injection, or taking a shortcut because it's fewer keystrokes > to do it a "feels safe" rather than "is safe" way. > > What do you all think? There's obviously a bit more work to do; the PoC > currently only covers mysqli_query, but I thought this stage is an > interesting point to throw it open to comments before working to complete > it. > > Matt -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] json_decode/encode should return full precision values by default
Hi all, Sorry, I was a bit busy during August. On Wed, Aug 5, 2015 at 5:37 PM, Yasuo Ohgaki wrote: > I sent work in progress PR for this and updated the RFC. > > https://github.com/php/php-src/pull/1455 > TODO: Add/modify tests. Add 0 mode support for PG(precision). Add > WDDX/XMLRPC PG(serialize_precision) support. > > https://wiki.php.net/rfc/precise_float_value The patch and wiki is updated. It's better to minimize the change at this time. I didn't look into WDDX/XMLRPC which uses EG(precision), if anyone insist, I'll look into it. I don't mind much if anyone suggests to drop PHP 5.6 part. I'll drop the part before RFC discussion to focus PHP7. It should only affects php_gcvt() and php.ini's precision setting. i.e. var_dump/var_export/ json_encode/echo/print. Tests passes, but if anyone notice misbehavior, please let me know. I would like to start RFC discussion in a few days. Thank you. -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] FYI: Linux Foundation CII Draft
Hi all, Some of us may know already. Linux Foundation CII project https://www.coreinfrastructure.org/ released draft badge program. https://github.com/linuxfoundation/cii-best-practices-badge We may consider badge program compliance in the future. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack
Hi all,
On Mon, Aug 10, 2015 at 6:57 PM, Craig Francis
wrote:
> I have been reading your conversation with great interest.
>
> But I would urge you to see Matts suggestion as a simple addition to the
> language (it's simpler than my suggestion), where his RFC seems to have
> already addressed your concerns (and he seems to have a working proof of
> concept).
>
> Personally I would like to see any one of these solutions being added to
> the core language, as there are far too many PHP programmers making
> ridiculous mistakes that the core language can be (and should be)
> identifying. That said, I am still biased to my suggestion, which also
> tries to consider other problems like XSS.
>
> But anyway...
>
> Take the code below, it is obviously wrong, but it executes without any
> complaints, and unless someone is checking every line of code that is
> written (note: PHP is doing so already), then the developer will just move
> on without thinking anything is wrong:
>
>
> http://php.net/manual/en/pdo.exec.php
> $dbh->exec("DELETE FROM fruit WHERE colour = '{$_GET['colour']}'");
>
This is awful... It seems someone already fixed the doc.
>
>
> Over the years I've heard many people say things like "use static
> analysis" or "prepared statements fix everything", but I don't think these
> are enough.
>
I fully agree "prepared statements fix everything" is simply wrong.
To be secure truly, secure API must split command and all others completely
_and_ command part must be programmer supplied constant/static/validated
string.
(i.e. Perfectly secure API must prohibit command injections at all)
Prepared statement does not satisfy both conditions.
Many APIs, that are considered secure API, are not perfectly secure
because "command part must be programmer supplied constant/static/validated
string" condition is not checked. It's left to developers.
e.g. execl/execv splits command and arguments, only single command is
allowed,
but if "command" is user parameter, it cannot be secure.
You only have to skim read things like the second comment (with 27 up
> votes) on the PDO prepare page to see that these problems are happening all
> the time:
>
>
> http://php.net/manual/en/pdo.prepare.php#111458
> SELECT * FROM users WHERE $search=:email
>
>
> So accept that education alone is not enough, and that the basic building
> blocks like prepared statements or ORMs are not enough, and look at these
> proposals and see how they will make the language better... because I can
> assure you, having a simple tainting system that can be switched on to show
> your mistakes, is considerably better than what we have today (i.e.
> nothing... or maybe using some half baked / expensive static analysis tool).
>
> Or are you scared that this will highlight the mistakes you have made in
> your own (probably over-complicated) code? as this is why I want this
> feature, because I know I have made mistakes, and with OOP, it is very easy
> todo so (abstractions like ORMs have a tendency to allow for these mistakes
> to creep in extremely easily).
>
I don't think the proposal is useless nor ineffective.
Taint system is nice to have, but the proposal does not seem preferable
resolution.
Don't get me wrong. I agree with your discussion overall.
I tend to dislike all or nothing choice for security related
issues especially, nonetheless
"nothing" is my choice this time...
Regards,
P.S. There are many outputs other than SQL. Context aware automatic
escaping/use of
secure API/validation for various outputs would be nice to have. This would
be new API
and wouldn't help users shooting their own foot, though. I don't have
concrete idea now,
but PHP may have features help it.
--
Yasuo Ohgaki
yohg...@ohgaki.net
Re: [PHP-DEV] libpcre version requirements
On Wed, Aug 12, 2015 at 4:59 AM, Yasuo Ohgaki wrote: > Distributions prefer to use system's libraries. I suppose question is if > PHP > supports older systems such as RHEL/CentOS 5 or not. They are used widely. > I prefer to support them if it is possible/feasible. Features that are not > supported > by libs could be documentation issues. IMO. > BTW, I don't think we should support very old libraries for new major release. -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] libpcre version requirements
Hi all, On Wed, Aug 12, 2015 at 2:55 AM, Christoph Becker wrote: > >> I'm asking because of bug #70232 which can easily be fixed, but that > >> requires PCRE 8.00 (2009-10-19). If we have to support older PCRE > >> versions, we'd probably need a fallback to the current behavior (which > >> would obviously keep the bug). > > > > I guess the question's really whether we should still support an > > _external_ libpcre that old, since we bundle much newer versions. I > > think the argument against changing it has always been that it works > > and we didn't need anything newer, but if we have a reason now then > > that doesn't really hold. > > > > One problem is that I don't think we can really change the minimum > > requirement on a stable branch, so for 5.5 and 5.6 we're going to have > > to implement something that works with older versions regardless, but > > I'd be for bumping the minimum version requirement for 7.0 if it makes > > the code cleaner (on that branch, at least) moving forward. > > I don't know if we can implement a fully working fallback for PCRE < 8 > in a viable way. Actually, PCRE_NOTEMPTY_ATSTART had been added to > libpcre to solve the issue with \K for /g[1]. Note the wording of the > commit message: > > | Added PCRE_NOTEMPTY_ATSTART in order to be able to correctly > | implement the /g option in pcretest when the pattern contains \K, > | [...] > > My best idea so far is to use PCRE_NOTEMPTY_ATSTART and to fallback to > PCRE_NOTEMPTY, if the former is not available. Of course the bug would > persist for old libpcre, but \K will not be effective for PCRE 6.6 anyway. Isn't it normal that PHP would not support features that underlying libs do not support? We have number of such functions. Isn't it the reason why phpinfo() shows underlying library versions? Distributions prefer to use system's libraries. I suppose question is if PHP supports older systems such as RHEL/CentOS 5 or not. They are used widely. I prefer to support them if it is possible/feasible. Features that are not supported by libs could be documentation issues. IMO. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
[PHP-DEV] Re: zend_string or not?
HI all, On Fri, Aug 7, 2015 at 4:25 AM, Yasuo Ohgaki wrote: > Is there zend_string usage guideline? > I'm wondering if zend_string is used where it is appropriate. > > Once we release PHP7, adopting zend_string for PHPAPI functions become > difficult. > (We have to keep legacy API or it will be 3rd party module author's > headache if we > change this with minor version up.) > > Evaluation for all PHPAPI functions that have "char *" parameter is > finished? > If not, we are better to do this now at least for core functions. > LXR output seems there are number of core functions that may use > zend_string. > > > http://lxr.php.net/search?q=PHPAPI&defs=&refs=&path=&hist=&project=PHP_TRUNK > I looked into the list quickly to see functions may be better to use zend_string. I picked up functions may have problem with null byte in char *. This is NOT exhaustive list, but partial list. I didn't check all function definitions as well as their usage. http://lxr.php.net/xref/PHP_TRUNK/ext/standard/php_var.h#57 http://lxr.php.net/xref/PHP_TRUNK/main/php_ini.h#33 http://lxr.php.net/xref/PHP_TRUNK/main/php_ini.h#34 http://lxr.php.net/xref/PHP_TRUNK/main/php_ini.h#35 http://lxr.php.net/xref/PHP_TRUNK/main/php_ini.h#36 http://lxr.php.net/xref/PHP_TRUNK/main/php_main.h#51 http://lxr.php.net/xref/PHP_TRUNK/main/php_network.h#233 http://lxr.php.net/xref/PHP_TRUNK/main/php_network.h#236 http://lxr.php.net/xref/PHP_TRUNK/main/php_network.h#252 http://lxr.php.net/xref/PHP_TRUNK/main/php_network.h#308 http://lxr.php.net/xref/PHP_TRUNK/main/fopen_wrappers.h#29 http://lxr.php.net/xref/PHP_TRUNK/main/fopen_wrappers.h#30 http://lxr.php.net/xref/PHP_TRUNK/main/fopen_wrappers.h#31 http://lxr.php.net/xref/PHP_TRUNK/main/fopen_wrappers.h#33 http://lxr.php.net/xref/PHP_TRUNK/main/fopen_wrappers.h#34 http://lxr.php.net/xref/PHP_TRUNK/main/fopen_wrappers.h#35 http://lxr.php.net/xref/PHP_TRUNK/main/fopen_wrappers.h#47 http://lxr.php.net/xref/PHP_TRUNK/main/fopen_wrappers.h#51 http://lxr.php.net/xref/PHP_TRUNK/main/fopen_wrappers.h#53 http://lxr.php.net/xref/PHP_TRUNK/main/php_streams.h#565 http://lxr.php.net/xref/PHP_TRUNK/main/php_streams.h#566 http://lxr.php.net/xref/PHP_TRUNK/main/php_streams.h#567 http://lxr.php.net/xref/PHP_TRUNK/main/php_streams.h#568 http://lxr.php.net/xref/PHP_TRUNK/main/php_streams.h#569 http://lxr.php.net/xref/PHP_TRUNK/main/php.h#283 http://lxr.php.net/xref/PHP_TRUNK/main/fopen_wrappers.h#29 http://lxr.php.net/xref/PHP_TRUNK/main/fopen_wrappers.h#30 http://lxr.php.net/xref/PHP_TRUNK/ext/standard/basic_functions.h#148 http://lxr.php.net/xref/PHP_TRUNK/ext/standard/basic_functions.h#149 We have input parameter validation for null bytes with ZPP. However, source of string may be database, file, network, etc. i.e. Strings may be passed without ZPP null byte check. There is potential risk that null byte injection attack may work. With zend_string, length/null byte could be checked if it is necessary. Function usage should be considered to decide if zend_string should be used or not, but it seems many of these functions are better to use zend_string. Except null byte injection, there are functions become more convenient with zend_string probably. Any comments? -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack
On Fri, Aug 7, 2015 at 10:29 AM, Yasuo Ohgaki wrote:
> Even if there is identifier placeholder, SQL keyword remains.
> So to be perfect, you'll need another place holder for SQL keywords.
> There is no escaping for SQL keywords and it has to be validation.
> e.g. ORDER BY {$_GET['order']}
>
Oops the last line should be
e.g. ORDER BY col {$_GET['order']}
BTW, instead of improving PHP, users are better to request "identifier
escape API"
to DB developers like PQescapeIdentifier() in PostgreSQL's client library.
IMO.
Regards,
--
Yasuo Ohgaki
yohg...@ohgaki.net
Re: [PHP-DEV] [RFC] Block requests to builtin SQL functions where PHP can prove the call is vulnerable to a potential SQL-injection attack
Hi Matt,
On Thu, Aug 6, 2015 at 12:46 PM, Matt Tait wrote:
> I'll take a few of your points in turn.
>
> With regards to the fact that not all SQL queries are directly
> parameterizable, this is true. Structural parts of a query, such as table
> names, column names and complex conditions are hard to parameterize with
> "vanilla" prepared statements, and many developers like to abstract some of
> these structural parts of a SQL query into config files, and append
> additional conditional constraints into the query at runtime based on user
> input.
>
> This feature addresses this head on. So long as the structural parts of the
> prepared statement -- that is, table names, database names and column names
> -- are not themselves attacker-controlled (I can't think of a valid reason
> whey they would be), this feature is happy for developers to concatenate
> them into a query string. For example, the following is not detected by the
> feature as dangerous, because the query (whilst dynamically constructed) is
> the linear concatenation of string literals, and so is a safeconst.
>
> $query = "SELECT * from {$config['tablename']} WHERE id=?";
> if(isset($_GET["filterbycolor"]))
> $query .= " AND color=?";
> do_prepared_statement($query, array("id" => $_GET["id"] "color" =>
> $_GET["color"]));
>
If you would like to prevent SQL injection via "Identifiers", how about
extend prepared query like
$query = "SELECT * from @ WHERE id=?";
if(isset($_GET["filterbycolor"]))
$query .= " AND color=?";
do_prepared_statement($query, array("id" => $_GET["id"] "color" =>
$_GET["color"]), array($config['tablename']));
where @ indicates identifier placeholder. The char could be anything that
will not violate SQL syntax. This could be implemented in user land as
there is no standard/native API for identifier placeholder.
Even if there is identifier placeholder, SQL keyword remains.
So to be perfect, you'll need another place holder for SQL keywords.
There is no escaping for SQL keywords and it has to be validation.
e.g. ORDER BY {$_GET['order']}
Regards,
--
Yasuo Ohgaki
yohg...@ohgaki.net
[PHP-DEV] zend_string or not?
Hi all, Is there zend_string usage guideline? I'm wondering if zend_string is used where it is appropriate. Once we release PHP7, adopting zend_string for PHPAPI functions become difficult. (We have to keep legacy API or it will be 3rd party module author's headache if we change this with minor version up.) Evaluation for all PHPAPI functions that have "char *" parameter is finished? If not, we are better to do this now at least for core functions. LXR output seems there are number of core functions that may use zend_string. http://lxr.php.net/search?q=PHPAPI&defs=&refs=&path=&hist=&project=PHP_TRUNK Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] json_decode/encode should return full precision values by default
Hi all,
On Fri, Jul 31, 2015 at 4:44 PM, Yasuo Ohgaki wrote:
> On Thu, Jul 30, 2015 at 6:06 PM, Nikita Popov
> wrote:
>
>> On Thu, Jul 30, 2015 at 1:25 AM, Yasuo Ohgaki wrote:
>>
>>> Hi all,
>>>
>>> On Thu, Jul 30, 2015 at 7:44 AM, Yasuo Ohgaki
>>> wrote:
>>>
>>>> On Thu, Jul 30, 2015 at 1:13 AM, Nikita Popov
>>>> wrote:
>>>>
>>>>> Instead of continuing to use serialize_precision, which will produce
>>>>> unnecessarily long outputs for many values, why don't we just switch to
>>>>> using the 0 mode of zend_dtoa, i.e. to return the shortest output that is
>>>>> still accurate if interpreted in round-to-nearest. I think this is what
>>>>> everybody else is using when they convert floating point numbers to
>>>>> strings. I guess we may not be able to change normal floating point
>>>>> printing to use this, but this seems like the best mode for anything using
>>>>> serialize_precision now and everything that should be using it (like JSON,
>>>>> and queries, etc).
>>>>
>>>>
>>>> I prefer your proposal!
>>>> Your proposal is a lot better than now.
>>>> Anyone has opinion for this?
>>>>
>>>> I'm writing the RFC and I would like to make this the first option. i.e.
>>>> serialize_precision=0 uses "zend_dtoa 0 mode" for all data exchange
>>>> functions (json/serialize/var_exrport. Anyone care about WDDX/XML_RPC?)
>>>>
>>>
>>> I wrote draft RFC.
>>>
>>> https://wiki.php.net/rfc/precise_float_value
>>>
>>> Please comment. I would like to start RFC discussion shortly.
>>> Thank you.
>>>
>>
>> Nice idea about using a special serialize_precision value for this. This
>> allows to keep BC for those that have tests for particular serialize output
>> or similar things.
>>
>> I would suggest to default serialize_precision to -1 in PHP 7 -- if
>> people want the previous behavior they can still have it, but I think -1 is
>> the more reasonable default as it matches what one would naturally expect.
>>
>> I don't see the need for having a separate setting for JSON. Having a
>> dozen different float precision settings will not help anyone.
>>
>
> Thank you for your feedback.
> I agree it's better if PHP7 uses 0 mode by default. I'll update the RFC.
> I have things to do on this weekend. I'll try to write patch this weekend
> hopefully.
>
I sent work in progress PR for this and updated the RFC.
https://github.com/php/php-src/pull/1455
TODO: Add/modify tests. Add 0 mode support for PG(precision). Add
WDDX/XMLRPC PG(serialize_precision) support.
https://wiki.php.net/rfc/precise_float_value
Old behavior
MacBook-Pro:github-php-src yohgaki$ php -r
'$a=["v"=>123450.1234567890123];
var_dump(json_encode($a), serialize($a), var_export($a, true));';
string(16) "{"v":1.2345e+29}"
string(39) "a:1:{s:1:"v";d:1.2344E+29;}"
string(42) "array (
'v' => 1.2344E+29,
)"
New behavior
MacBook-Pro:github-php-src yohgaki$ ./php-bin -r
'$a=["v"=>123450.1234567890123];
var_dump(json_encode($a), serialize($a), var_export($a, true));';
string(16) "{"v":1.2345e+29}"
string(27) "a:1:{s:1:"v";d:1.2345E+29;}"
string(30) "array (
'v' => 1.2345E+29,
)"
Before I start working on PG(precision) 0 mode support, I would like to hear
comments for this change if any. The default (PG(precision)=14) will not be
changed
and there will be no BC. I think it's more consistent if both PG(precision)
and
PG(serialize_precision) support 0 mode.
Any comments for adding PG(precision) 0 mode support?
Regards,
--
Yasuo Ohgaki
yohg...@ohgaki.net
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Niklas, On Sat, Aug 1, 2015 at 5:50 PM, Niklas Keller wrote: > You always assume the developer just wants to fallback to something > different. You can't > detect the environment btw. because it could just fail because of too many > open file descriptors. > This is not my assumption, but others. They argue that they would like to fallback to alternative method when error happened. (which I think it's not best way to do) If they don't, E_RECOVERABLE_ERROR is enough. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Niklas, On Sat, Aug 1, 2015 at 8:27 AM, Yasuo Ohgaki wrote: > They should totally be handled. You need to catch the error and throw a >> defined exception, otherwise your public API will break if you choose to >> use another internal implementation. >> Additionally, you seem to assume that the library doesn't have to do >> things like cleanups in such a case. >> > > My thought is based on Design by Contract (Contract programming). > When parameter or environment does not satisfy contract, contract error > should be > resulted in program/process termination. > > Fixing inappropriate parameter or environment is not library/framework > author's > responsibility, but the developer's. i.e. > Caller(function/programmer/system admin) > has the responsibility that satisfies parameter/environment requirement. > If > requirement is not met, it's perfectly OK for library/framework to raise > fatal > errors/exceptions. e.g. "You need PHP 5.6 or greater" error. > I'll be more specific for "CSPRNG not available" error. If a author would like to handle the error and fallback to non crypt safe RNG, he/she should detect environment and execute alternative code for the environment. Catching exception and fallback to non crypt safe RNG is not optimal way for handling unsatisfactory environment. IMHO. If we need function that checks environment, we are better to provide one rather than let users to use exception. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Niklas, On Sat, Aug 1, 2015 at 7:20 AM, Niklas Keller wrote: > 2015-07-31 23:36 GMT+02:00 Yasuo Ohgaki : > >> Hi Niklas, >> >> On Fri, Jul 31, 2015 at 7:20 PM, Niklas Keller wrote: >> >>> Using set_error_handler isn't handling errors gracefully. Well, it's >>> better than E_ERROR, but then libraries can't handle those errors >>> gracefully, because the user might override its error handler by setting an >>> own handler. >> >> >> Now I see what do you mean by "gracefully". >> >> TL;DR; >> It's app developer jobs to handle these fatal errors. >> > > Nope. > > >> Most fatal errors shouldn't be recovered by library. e.g. Fallback to non >> CSPRNG when CSPRNG is not >> available. >> > > They should totally be handled. You need to catch the error and throw a > defined exception, otherwise your public API will break if you choose to > use another internal implementation. > Additionally, you seem to assume that the library doesn't have to do > things like cleanups in such a case. > My thought is based on Design by Contract (Contract programming). When parameter or environment does not satisfy contract, contract error should be resulted in program/process termination. Fixing inappropriate parameter or environment is not library/framework author's responsibility, but the developer's. i.e. Caller(function/programmer/system admin) has the responsibility that satisfies parameter/environment requirement. If requirement is not met, it's perfectly OK for library/framework to raise fatal errors/exceptions. e.g. "You need PHP 5.6 or greater" error. Handling these fatal errors in a library/framework make code complex unnecessarily. More complex code has more chances to have bugs including security related bugs. Library/framework may simply raise error/exception telling users "It's impossible to work". PHP is general programing language, so we have to consider long life applications such as standalone apps. I fully agree that exception is far easier for handling errors properly and keep app running. However, making randon_*() a special function does not worth it. IMHO. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Niklas, On Fri, Jul 31, 2015 at 7:20 PM, Niklas Keller wrote: > Using set_error_handler isn't handling errors gracefully. Well, it's > better than E_ERROR, but then libraries can't handle those errors > gracefully, because the user might override its error handler by setting an > own handler. Now I see what do you mean by "gracefully". TL;DR; It's app developer jobs to handle these fatal errors. Most fatal errors shouldn't be recovered by library. e.g. Fallback to non CSPRNG when CSPRNG is not available. IMO, library authors do not have to worry about errors like session's E_RECOVERABLE_ERROR or CSPRNG not available. These are fatal errors by its nature and app developer or system administrator job to handle them. We cannot blindly assume such fatal errors never happen in production apps, so we are better to give users a chance to return "Nice error page/message" when it happened. Anyway, we are better to move errors to exceptions at once and force "all" modules including 3rd party module to raise exceptions. i.e. Make php_error_docref() raise exceptions. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Niklas, On Fri, Jul 31, 2015 at 5:12 PM, Niklas Keller wrote: > I think the question is more whether Exception or Error (class, not > E_RECOVERABLE_ERROR), to allow handling it gracefully. > E_WARNING is too weak for CSPRNG not available. It's fatal error. I agree fatal errors should be able to handle gracefully if it is possible. That's the reason why I prepared a patch that changes session E_ERROR to E_RECOVERABLE_ERROR. https://github.com/php/php-src/compare/master...yohgaki:master-e_recoverable_error?expand=1 It's inspired by this thread :) Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] json_decode/encode should return full precision values by default
Hi Nikita, On Thu, Jul 30, 2015 at 6:06 PM, Nikita Popov wrote: > On Thu, Jul 30, 2015 at 1:25 AM, Yasuo Ohgaki wrote: > >> Hi all, >> >> On Thu, Jul 30, 2015 at 7:44 AM, Yasuo Ohgaki wrote: >> >>> On Thu, Jul 30, 2015 at 1:13 AM, Nikita Popov >>> wrote: >>> >>>> Instead of continuing to use serialize_precision, which will produce >>>> unnecessarily long outputs for many values, why don't we just switch to >>>> using the 0 mode of zend_dtoa, i.e. to return the shortest output that is >>>> still accurate if interpreted in round-to-nearest. I think this is what >>>> everybody else is using when they convert floating point numbers to >>>> strings. I guess we may not be able to change normal floating point >>>> printing to use this, but this seems like the best mode for anything using >>>> serialize_precision now and everything that should be using it (like JSON, >>>> and queries, etc). >>> >>> >>> I prefer your proposal! >>> Your proposal is a lot better than now. >>> Anyone has opinion for this? >>> >>> I'm writing the RFC and I would like to make this the first option. i.e. >>> serialize_precision=0 uses "zend_dtoa 0 mode" for all data exchange >>> functions (json/serialize/var_exrport. Anyone care about WDDX/XML_RPC?) >>> >> >> I wrote draft RFC. >> >> https://wiki.php.net/rfc/precise_float_value >> >> Please comment. I would like to start RFC discussion shortly. >> Thank you. >> > > Nice idea about using a special serialize_precision value for this. This > allows to keep BC for those that have tests for particular serialize output > or similar things. > > I would suggest to default serialize_precision to -1 in PHP 7 -- if people > want the previous behavior they can still have it, but I think -1 is the > more reasonable default as it matches what one would naturally expect. > > I don't see the need for having a separate setting for JSON. Having a > dozen different float precision settings will not help anyone. > Thank you for your feedback. I agree it's better if PHP7 uses 0 mode by default. I'll update the RFC. I have things to do on this weekend. I'll try to write patch this weekend hopefully. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Scott, On Fri, Jul 31, 2015 at 6:33 AM, Scott Arciszewski wrote: > On Jul 30, 2015 2:27 PM, "Niklas Keller" wrote: > > I prefer Exception, too, because it's I/O related. > > > > @Scott: You can open votes on everything, doesn't matter, just create a > page with a vote. > > I just don't know where to put it in the wiki, because it's not a RFC. > > > > Regards, Niklas > > I'm not sure how to do that. > > I have a noncritical security patch I intend to write for random_bytes() > that I would like to submit as a PR but first I'd like to see what the > resolution will be re: Exceptions. > > Also, merge conflicts aren't fun. ;) > > If there's anything I can do to get those two merged faster, please let me > know. You can have RFC vote options like - Raise Error/Exception when CSPRNG is not available (Choice: Yes/No) - Use error(E_RECOVERABLE_ERROR) or exception(Whatever exception prefered) (Choice: Error/Exception) BTW, I prefer exception, but consistency is important also. My vote will be Yes and Error. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] json_decode/encode should return full precision values by default
Hi Ryan, On Thu, Jul 30, 2015 at 8:35 AM, Ryan Pallas wrote: > I wrote draft RFC. >> >> https://wiki.php.net/rfc/precise_float_value >> >> Please comment. I would like to start RFC discussion shortly. >> Thank you. >> > > My thought would be why not -1 to make it obvious it has special meaning? > When I see precision=0 I tend to think that means my numbers will be > returned as ints no matter what (there is zero fractional precision) > whereas this takes a valid value for precision and gives it special > meaning. I fully support your end game, but giving a valid value a special > meaning I do not like. > Thank you for your feedback. I'll update the RFC and use "-1" for it. When I make the patch for this, I'll post new RFC discussion mail. It will take a while. Comments are appreciated. Regards. -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] json_decode/encode should return full precision values by default
Hi all, On Thu, Jul 30, 2015 at 7:44 AM, Yasuo Ohgaki wrote: > On Thu, Jul 30, 2015 at 1:13 AM, Nikita Popov > wrote: > >> Instead of continuing to use serialize_precision, which will produce >> unnecessarily long outputs for many values, why don't we just switch to >> using the 0 mode of zend_dtoa, i.e. to return the shortest output that is >> still accurate if interpreted in round-to-nearest. I think this is what >> everybody else is using when they convert floating point numbers to >> strings. I guess we may not be able to change normal floating point >> printing to use this, but this seems like the best mode for anything using >> serialize_precision now and everything that should be using it (like JSON, >> and queries, etc). > > > I prefer your proposal! > Your proposal is a lot better than now. > Anyone has opinion for this? > > I'm writing the RFC and I would like to make this the first option. i.e. > serialize_precision=0 uses "zend_dtoa 0 mode" for all data exchange > functions (json/serialize/var_exrport. Anyone care about WDDX/XML_RPC?) > I wrote draft RFC. https://wiki.php.net/rfc/precise_float_value Please comment. I would like to start RFC discussion shortly. Thank you. -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] json_decode/encode should return full precision values by default
Hi Nikita, On Thu, Jul 30, 2015 at 1:13 AM, Nikita Popov wrote: > Instead of continuing to use serialize_precision, which will produce > unnecessarily long outputs for many values, why don't we just switch to > using the 0 mode of zend_dtoa, i.e. to return the shortest output that is > still accurate if interpreted in round-to-nearest. I think this is what > everybody else is using when they convert floating point numbers to > strings. I guess we may not be able to change normal floating point > printing to use this, but this seems like the best mode for anything using > serialize_precision now and everything that should be using it (like JSON, > and queries, etc). I prefer your proposal! Your proposal is a lot better than now. Anyone has opinion for this? I'm writing the RFC and I would like to make this the first option. i.e. serialize_precision=0 uses "zend_dtoa 0 mode" for all data exchange functions (json/serialize/var_exrport. Anyone care about WDDX/XML_RPC?) Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] json_decode/encode should return full precision values by default
Hi Jakub, For me, JSON is one of a data exchange format just like serialize/var_export. Anyway, we are about to reach an agreement. On Wed, Jul 29, 2015 at 5:32 PM, Jakub Zelenka wrote: > > >> >>> >>>> Question is "Is this the way it should be?". >>>> >>>> >>> I have already said that using precision ini wasn't the best idea. >>> However json_encode is not the same as serialize and we should not ever >>> change its output in a bug fixing release. Doing that could cause also >>> other issues as this is a BC break. Lets imagine that someone set low >>> precision on purpose just to limit precision and save some space >>> >> >> I've been fixed var_export precision issue as a bug. >> >> [yohgaki@dev php-src]$ git show 3cf2682083fc1c8635b02c4c >> commit 3cf2682083fc1c8635b02c4cf77bdf12c5e5da35 >> Merge: 5c89d5a 4c45e95 >> Author: Yasuo Ohgaki >> Date: Tue Oct 29 17:30:58 2013 +0900 >> >> Merge branch 'PHP-5.5' >> >> * PHP-5.5: >> Fixed Bug 64760 var_export() does not use full precision for >> floating-point numbers >> >> > Again it's not the same. json_encode is much more used the var_export IMHO > so the chance that you break someone's code is much bigger. Also it doesn't > mean that you did the right thing and it's not a precedence for such change > in json. I guess that there wasn't anyone who would disagree with you at > that time. > I agree that JSON is used more than var_export or even serialize. I guess almost all users do not care much about preciseness of JSON numeric. I was the one also until I had to deal with float values. Since almost nobody cares about preciseness, why not make it more precise? I may ask general list see if there are users who mind this change. > >> Although I don't strongly insist merging fix to 5.6 branch, I think data >> exchange function >> that is not trying to be precise is bad thing. >> >> >>> when transferring data . If you change it, then it's screwed up because >>> it will use different ini. We don't know what people do in their code and >>> we should not break it. As I said this is not a bug but we could consider >>> changing that if the RFC proposing such change passes. >>> >> >> Strong json numeric validator may have problem with the change. However, >> JSON RFC states >> >> 6. Numbers >> This specification allows implementations to set limits on the range >>and precision of numbers accepted. Since software that implements >>IEEE 754-2008 binary64 (double precision) numbers [IEEE754] is >>generally available and widely used, good interoperability can be >>achieved by implementations that expect no more precision or range >>than these provide, in the sense that implementations will >>approximate JSON numbers within the expected precision. A JSON >>number such as 1E400 or 3.141592653589793238462643383279 may indicate >>potential interoperability problems, since it suggests that the >>software that created it expects receiving software to have greater >>capabilities for numeric magnitude and precision than is widely >>available. >> >>Note that when such software is used, numbers that are integers and >>are in the range [-(2**53)+1, (2**53)-1] are interoperable in the >>sense that implementations will agree exactly on their numeric >>values. >> https://tools.ietf.org/html/rfc7159 >> >> In order PHP to be more compatible, we should use IEEE 754 range or >> our other data exchange standard, which is serialize_precision=17. >> >> > We are still compatible. This is recommendation, not requirement. However > I have to agree that it will be probably good idea to change default. But > just only if agreed in RFC as this is not a bug! Personally I am against > changing that to serialize_precision. I'd much prefer introducing new ini > (e.g. json_precision) with the same default as serialize_precision. It > would give more flexibility and allow changing value just for json. > I agree that it is implementation matter how "JSON number" is treated. Since PHP is general programming language, I think it's better to make it as generic as possible by default. It's perfectly OK for me to have json_precision. I prefer to have it indeed! My position is >> - PHP7.0: We must use serialize_precision for JSON >> - PHP5.6: I strongly suggest to use serialize_precision for JSON >> >> > I'd pre
Re: [PHP-DEV] json_decode/encode should return full precision values by default
Hi Jakub,
On Wed, Jul 29, 2015 at 3:15 AM, Jakub Zelenka wrote:
> On Mon, Jul 27, 2015 at 11:17 PM, Yasuo Ohgaki wrote:
>
>>
>> Get JSON data from Google maps and store the data using PHP, then
>> users lose last 2 digits of fraction part by default. The value is
>> changed
>> and wrong. This is definitely a bug.
>>
>>
> I don't really get why you use Google maps as an example. Do you actually
> know what's the difference between two distances that differs after
> rounding with precision 14?
>
> I just tried it on
> http://www.ig.utexas.edu/outreach/googleearth/latlong.html and set
>
> Lat 1: 51.602097123457 ; Long 1: -0.120667
> Lat 2: 51.602097123458 ; Long 2: -0.120667
>
> The difference is 1.114e-10 km which is 0.0001114 millimetres. Are you
> actually serious about that? :)
>
It's error margin for GPS. How about scientific values?
I'm not comfortable with that values aren't processed/stored correctly (as
much as possible.
You know float can't be exactly precise) Even if GPS values may ignore some
fractions,
I'm very uncomfortable with ignoring/destroying original values while it
could be stored
exactly as it is.
In addition, it's inconsistent with other data exchange functions. i.e.
serialize/var_export.
>
>
>
>> We can write
>>
>> $old = ini_set('precision', 17);
>> json_encode($var);
>> ini_set('precision', $old);
>>
>
> You can set it once in your ini file if you need such precision.
>
It's not a good idea. Most PHP codes depend on 14 precision and programmer
cannot assure other code works as expected.
>
>
>>
>> everywhere to workaround this problem.
>>
>> Question is "Is this the way it should be?".
>>
>>
> I have already said that using precision ini wasn't the best idea. However
> json_encode is not the same as serialize and we should not ever change its
> output in a bug fixing release. Doing that could cause also other issues as
> this is a BC break. Lets imagine that someone set low precision on purpose
> just to limit precision and save some space
>
I've been fixed var_export precision issue as a bug.
[yohgaki@dev php-src]$ git show 3cf2682083fc1c8635b02c4c
commit 3cf2682083fc1c8635b02c4cf77bdf12c5e5da35
Merge: 5c89d5a 4c45e95
Author: Yasuo Ohgaki
Date: Tue Oct 29 17:30:58 2013 +0900
Merge branch 'PHP-5.5'
* PHP-5.5:
Fixed Bug 64760 var_export() does not use full precision for
floating-point numbers
Although I don't strongly insist merging fix to 5.6 branch, I think data
exchange function
that is not trying to be precise is bad thing.
> when transferring data . If you change it, then it's screwed up because it
> will use different ini. We don't know what people do in their code and we
> should not break it. As I said this is not a bug but we could consider
> changing that if the RFC proposing such change passes.
>
Strong json numeric validator may have problem with the change. However,
JSON RFC states
6. Numbers
This specification allows implementations to set limits on the range
and precision of numbers accepted. Since software that implements
IEEE 754-2008 binary64 (double precision) numbers [IEEE754] is
generally available and widely used, good interoperability can be
achieved by implementations that expect no more precision or range
than these provide, in the sense that implementations will
approximate JSON numbers within the expected precision. A JSON
number such as 1E400 or 3.141592653589793238462643383279 may indicate
potential interoperability problems, since it suggests that the
software that created it expects receiving software to have greater
capabilities for numeric magnitude and precision than is widely
available.
Note that when such software is used, numbers that are integers and
are in the range [-(2**53)+1, (2**53)-1] are interoperable in the
sense that implementations will agree exactly on their numeric
values.
https://tools.ietf.org/html/rfc7159
In order PHP to be more compatible, we should use IEEE 754 range or
our other data exchange standard, which is serialize_precision=17.
My position is
- PHP7.0: We must use serialize_precision for JSON
- PHP5.6: I strongly suggest to use serialize_precision for JSON
Regards,
--
Yasuo Ohgaki
yohg...@ohgaki.net
Re: [PHP-DEV] json_decode/encode should return full precision values by default
Hi all,
On Tue, Jul 28, 2015 at 9:34 AM, Stanislav Malyshev
wrote:
> >
> > We can write
> >
> > $old = ini_set('precision', 17);
> > json_encode($var);
> > ini_set('precision', $old);
> >
> > everywhere to workaround this problem.
>
> If we're talking about encoding, that's what precision setting is - it
> says how much we want to preserve in decimal encoding of float numbers.
> Decoding, of course, should use whatever data available and not
> truncate, but I understand that is already happening?
>
> > Question is "Is this the way it should be?".
>
> I don't see why not. That's exactly what precision setting is for, as
> far as I can see.
JSON is var exporter/importer like serialize/var_export.
Both of serialize/var_export uses PG(serialize_precision), what's the point
of
_lose_ / _destroy_ original values while PHP could keep more precise values
than now?
Regards,
--
Yasuo Ohgaki
yohg...@ohgaki.net
Re: [PHP-DEV] json_decode/encode should return full precision values by default
On Tue, Jul 28, 2015 at 7:10 AM, Yasuo Ohgaki wrote:
> On Tue, Jul 28, 2015 at 6:54 AM, Anthony Ferrara
> wrote:
>
>> On Sun, Jul 26, 2015 at 4:20 PM, Yasuo Ohgaki wrote:
>> > Hi Jakub,
>> >
>> > On Mon, Jul 27, 2015 at 3:32 AM, Jakub Zelenka wrote:
>> >
>> >> I don't think that this is a bug. Your example is also completely
>> >> unrelated to json because the place when the value is rounded is
>> var_dump
>> >> where it's based on ini precision. You would get the same values with
>> >> json_encode but it's only because it uses the same ini ( precision ).
>> >> Basically it has nothing to do with json_decode.
>> >>
>> >
>> > OK. Better example.
>> >
>> > [yohgaki@dev PHP-master]$ ./php-bin
>> > > > $j = '{ "v": 0.1234567890123456789 }';
>> > var_dump(json_encode(json_decode($j)));
>> > ini_set('precision', 20);
>> > var_dump(json_encode(json_decode($j)));
>> > ?>
>> >
>> >
>> > string(22) "{"v":0.12345678901235}"
>> > string(28) "{"v":0.12345678901234567737}"
>>
>> I think you missed the point. Parsing isn't dependent upon precision
>> setting. Only dumping: http://3v4l.org/48VSt
>>
>> $j = '{ "v": 0.1234567890123456789 }';
>> $d1 = json_decode($j);
>>
>> ini_set('precision', 20);
>>
>> $d2 = json_decode($j);
>>
>> var_dump($d1, $d2);
>>
>> //object(stdClass)#1 (1) { ["v"]=> float(0.12345678901234567737) }
>> //object(stdClass)#2 (1) { ["v"]=> float(0.12345678901234567737) }
>>
>> Meaning that it's parsed correctly.
>>
>> There is no bug here.
>
>
> I understands PHP uses pure IEEE data and arithmetic and
> I disagree that this is not a bug.
>
> What I'm pointing it out is "simple JSON operation truncates
> _valid/precise_ value".
> IEEE double can store 17 digit fraction part precisely, but PHP truncates
> it.
>
> > > $j = '{ "v": 0.1234567890123456789 }';
> > var_dump(json_encode(json_decode($j)));
> > ini_set('precision', 20);
> > var_dump(json_encode(json_decode($j)));
> > ?>
> >
> >
> > string(22) "{"v":0.12345678901235}"
> > string(28) "{"v":0.12345678901234567737}"
>
> The same problem in serialize/unserialize was fixed as a bug.
> Encode/decode should be as precise as possible by _default_. IMO.
>
> What's the point of truncation and having broken value?
>
Get JSON data from Google maps and store the data using PHP, then
users lose last 2 digits of fraction part by default. The value is changed
and wrong. This is definitely a bug.
We can write
$old = ini_set('precision', 17);
json_encode($var);
ini_set('precision', $old);
everywhere to workaround this problem.
Question is "Is this the way it should be?".
--
Yasuo Ohgaki
yohg...@ohgaki.net
Re: [PHP-DEV] json_decode/encode should return full precision values by default
Hi Anthony,
On Tue, Jul 28, 2015 at 6:54 AM, Anthony Ferrara
wrote:
> On Sun, Jul 26, 2015 at 4:20 PM, Yasuo Ohgaki wrote:
> > Hi Jakub,
> >
> > On Mon, Jul 27, 2015 at 3:32 AM, Jakub Zelenka wrote:
> >
> >> I don't think that this is a bug. Your example is also completely
> >> unrelated to json because the place when the value is rounded is
> var_dump
> >> where it's based on ini precision. You would get the same values with
> >> json_encode but it's only because it uses the same ini ( precision ).
> >> Basically it has nothing to do with json_decode.
> >>
> >
> > OK. Better example.
> >
> > [yohgaki@dev PHP-master]$ ./php-bin
> > > $j = '{ "v": 0.1234567890123456789 }';
> > var_dump(json_encode(json_decode($j)));
> > ini_set('precision', 20);
> > var_dump(json_encode(json_decode($j)));
> > ?>
> >
> >
> > string(22) "{"v":0.12345678901235}"
> > string(28) "{"v":0.12345678901234567737}"
>
> I think you missed the point. Parsing isn't dependent upon precision
> setting. Only dumping: http://3v4l.org/48VSt
>
> $j = '{ "v": 0.1234567890123456789 }';
> $d1 = json_decode($j);
>
> ini_set('precision', 20);
>
> $d2 = json_decode($j);
>
> var_dump($d1, $d2);
>
> //object(stdClass)#1 (1) { ["v"]=> float(0.12345678901234567737) }
> //object(stdClass)#2 (1) { ["v"]=> float(0.12345678901234567737) }
>
> Meaning that it's parsed correctly.
>
> There is no bug here.
I understands PHP uses pure IEEE data and arithmetic and
I disagree that this is not a bug.
What I'm pointing it out is "simple JSON operation truncates
_valid/precise_ value".
IEEE double can store 17 digit fraction part precisely, but PHP truncates
it.
> $j = '{ "v": 0.1234567890123456789 }';
> var_dump(json_encode(json_decode($j)));
> ini_set('precision', 20);
> var_dump(json_encode(json_decode($j)));
> ?>
>
>
> string(22) "{"v":0.12345678901235}"
> string(28) "{"v":0.12345678901234567737}"
The same problem in serialize/unserialize was fixed as a bug.
Encode/decode should be as precise as possible by _default_. IMO.
What's the point of truncation and having broken value?
Regards,
--
Yasuo Ohgaki
yohg...@ohgaki.net
Re: [PHP-DEV] json_decode/encode should return full precision values by default
Hi Jakub,
On Mon, Jul 27, 2015 at 3:32 AM, Jakub Zelenka wrote:
> I don't think that this is a bug. Your example is also completely
> unrelated to json because the place when the value is rounded is var_dump
> where it's based on ini precision. You would get the same values with
> json_encode but it's only because it uses the same ini ( precision ).
> Basically it has nothing to do with json_decode.
>
OK. Better example.
[yohgaki@dev PHP-master]$ ./php-bin
string(22) "{"v":0.12345678901235}"
string(28) "{"v":0.12345678901234567737}"
I see that that the bug report you created (
> https://bugs.php.net/bug.php?id=70137 ) is actually about using
> serialize.precision instead of precision in json_encode . I agree that
> using 'precision' ini is not the best solution. However I don't think that
> start suddenly using serialize.precision is a good idea. First of all it's
> a big BC break that will be quite difficult to find (There is no way this
> should go to the point release because this is changing the generated
> json). Also the json encode usage is much wider than serialization. There
> might be use cases when you don't care about maximal precision and you
> prefer to save some space. Imagine that you are transferring lots of float
> numbers. Then it will result in increasing of the transfer size. It would
> be great if the new ini was used when it was introduced but I'm afraid that
> changing that now is not a good idea
>
> I think that due to BC break, this shouldn't be changed. If you really
> want a bigger precision, you can do ini_set('precision', 30) before
> json_encode and then set it back. It means that this not a bug because you
> can still change it if you want. That means that we can't change before PHP
> 8 or whatever comes after 7... :)
>
Same argument could be done for serialize, but it's changed to keep
more precise value. Manual recommend json_decode/encode for data
serialization also.
Warning
Do not pass untrusted user input to unserialize(). Unserialization can
result in code being loaded and executed due to object instantiation and
autoloading, and a malicious user may be able to exploit this. Use a safe,
standard data interchange format such as JSON (via json_decode() and
json_encode()) if you need to pass serialized data to the user.
http://jp2.php.net/manual/en/function.unserialize.php
It does not make sense having different serialization for float.
Losing effective precision is design bug. IMHO.
Currently users cannot handle even Google Map JSON data correctly by
default.
Fixing this is just a matter of using serialize.precision. If users need
larger setting
than 17 for whatever reason, they may do ini_set('serialize.precision', 30)
and the
rest of codes are unaffected.
BTW, larger precision setting does not mean precise, but current behavior
definitely lose effective values. Those who don't care for precise data
handling
wouldn't care for more precise data handling, do they?
Regards,
--
Yasuo Ohgaki
yohg...@ohgaki.net
[PHP-DEV] Re: json_decode/encode should return full precision values by default
Hi all,
Created bug report for this issue.
https://bugs.php.net/bug.php?id=70137
--
Yasuo Ohgaki
yohg...@ohgaki.net
On Sun, Jul 26, 2015 at 7:01 AM, Yasuo Ohgaki wrote:
> Hi all,
>
> I had to work with Google Map API and need to handle values precisely.
> Fortunately, it seems values are IEEE double, but I get rounded float
> values by default.
>
> For example,
> $json = '
> {
>"results" : [
> {
> "elevation" : 1608.637939453125,
> "location" : {
> "lat" : 39.73915360,
> "lng" : -104.98470340
> },
> "resolution" : 4.771975994110107
> },
> {
> "elevation" : -50.78903579711914,
> "location" : {
> "lat" : 36.460,
> "lng" : -116.870
> },
> "resolution" : 19.08790397644043
> }
>],
>"status" : "OK"
> }
> ';
>
> var_dump(json_decode($json));
> ?>
>
> object(stdClass)#5 (2) {
> ["results"]=>
> array(2) {
> [0]=>
> object(stdClass)#1 (3) {
> ["elevation"]=>
> float(1608.6379394531)
> ["location"]=>
> object(stdClass)#2 (2) {
> ["lat"]=>
> float(39.7391536)
> ["lng"]=>
> float(-104.9847034)
> }
> ["resolution"]=>
> float(4.7719759941101)
> }
> [1]=>
> object(stdClass)#3 (3) {
> ["elevation"]=>
> float(-50.789035797119)
> ["location"]=>
> object(stdClass)#4 (2) {
> ["lat"]=>
> float(36.46)
> ["lng"]=>
> float(-116.87)
> }
> ["resolution"]=>
> float(19.08790397644)
> }
> }
> ["status"]=>
> string(2) "OK"
> }
>
>
> json_decode()/json_encode() must be able to handle precise IEEE double
> value by _default_.
>
> serialize() is changed to use max precision. json_decode/encode should do
> the same at least.
>
> I think this change should be provided as bug fix.
> Any comments?
>
> --
> Yasuo Ohgaki
> yohg...@ohgaki.net
>
[PHP-DEV] json_decode/encode should return full precision values by default
Hi all,
I had to work with Google Map API and need to handle values precisely.
Fortunately, it seems values are IEEE double, but I get rounded float
values by default.
For example,
object(stdClass)#5 (2) {
["results"]=>
array(2) {
[0]=>
object(stdClass)#1 (3) {
["elevation"]=>
float(1608.6379394531)
["location"]=>
object(stdClass)#2 (2) {
["lat"]=>
float(39.7391536)
["lng"]=>
float(-104.9847034)
}
["resolution"]=>
float(4.7719759941101)
}
[1]=>
object(stdClass)#3 (3) {
["elevation"]=>
float(-50.789035797119)
["location"]=>
object(stdClass)#4 (2) {
["lat"]=>
float(36.46)
["lng"]=>
float(-116.87)
}
["resolution"]=>
float(19.08790397644)
}
}
["status"]=>
string(2) "OK"
}
json_decode()/json_encode() must be able to handle precise IEEE double
value by _default_.
serialize() is changed to use max precision. json_decode/encode should do
the same at least.
I think this change should be provided as bug fix.
Any comments?
--
Yasuo Ohgaki
yohg...@ohgaki.net
[PHP-DEV] Re: [PHP-CVS] com php-src: Change E_ERROR and some E_WARNING to E_RECOVERABLE_ERROR.: ext/session/mod_user.c ext/session/mod_user_class.c ext/session/session.c ext/session/tests/bug60860.php
Hi Anatol, On Tue, Jul 21, 2015 at 3:06 PM, Anatol Belski wrote: > Changing E_ERROR to E_RECOVERABLE_ERROR is probably ok where it is safe, > but changing E_WARNING to E_RECOVERABLE_ERROR is a BC break. I agree it's BC. E_ERROR does not give users a chance for graceful script termination and E_WARNING is used instead. This is the only reason why E_WARNING is used before. However, users may continue execution with the default setting. I think it's not good to continue with fatal setting errors, but I don't mind changing these errors back to E_WARNING. It's your call. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Rowan, On Wed, Jul 15, 2015 at 9:00 PM, Rowan Collins wrote: > If we are going to adopt exception for "functions", it would be better to >> have >> switch that convert all errors to exceptions. >> > > Strongly disagree. A runtime switch would be a horrible consistency > nightmare for userland, and many of the existing functions which > desperately need better error-handling (e.g. file I/O) don't issue an error > at the moment anyway. > > The only reason (and it's a massive reason, don't get me wrong) not to > start throwing exceptions all over the core is backwards compatibility. New > functions do not, by definition, have anything to be backwards compatible > with. > > So my suggestion is, to come up with some reasonable definition of "new > area of functionality" where we can do things in a way which everyone seems > to agree is basically better, i.e. throw Exceptions. > - The rule shouldn't apply for new variations or close cousins of existing > functions (e.g. the json_decode_to() that was briefly discussed should use > similar conventions to json_decode() if it were implemented), only in cases > like this where we're adding a new set of functions (the manual puts > random_bytes and random_int in their own "book": > http://php.net/manual/en/book.csprng.php). > - It also shouldn't be used for "routine" errors (like File Not Found), > only in place of conditions which can justify a fatal error - both for the > intuitive reason that an uncaught exception *is* a fatal error, and the > procedural reason that we may want to decide a policy for "routine" error > handling at a later date. > > At some point, we should revisit the general error-handling strategy of > both the language and the core library, but 7.0 having been somewhat > derailed (in my opinion) by the endless debates about scalar type hints, it > will have to wait until 8.0 before we can do anything radical to existing > functions. I prefer exception also. I agree with your discussion in general. The only difference is how PHP module functions should adopt exceptions. When PHP adopted naming standard, we didn't enforce new naming standard for all functions. As a result, we have "module_func_name" and "modulefuncname" still. The same thing will happen for exception/error. I would like to avoid such situation (at least for modules provided with PHP). i.e. One function raises exception, another raises error. I understand many of us dislike INI switch nor runtime switch that changes behavior. However, it's impossible to move to exception at once because we have no control for users' code including 3rd party modules. IMHO. Rather than adding exception adopted functions gradually, having the switch is better for in the long run. We may deprecate/remove the switch 10 years later or more. BTW, I'm thinking to have "ModulenameException"/"ModulenameFunctionException" or like rather than simply calling/converting errors to ErrorException when exception is enabled for functions. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
[PHP-DEV] Change session module's E_ERROR to E_RECOVERABLE_ERROR
Hi all, Session module's E_ERROR could be E_RECOVERABLE_ERROR. http://lxr.php.net/search?q=E_ERROR&defs=&refs=&path=ext%2Fsession&hist=&project=PHP_TRUNK Any comments changing the error? -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Rowan, On Wed, Jul 15, 2015 at 7:57 PM, Rowan Collins wrote: > Sammy Kaye Powers wrote on 14/07/2015 22:04: > >> Since the core functions in PHP don't throw Exceptions, there is debate on >> whether or not this change should be implemented. Some say the CSPRNG's >> should get a special pass since they will be relied on for cryptography. >> If >> we can't throw Exceptions, there were suggestions of raising a fatal error >> if the RNG fails. >> > > Given that the *engine itself* now throws exceptions, and we have bundled > extensions which throw exceptions, the blanket ban on exceptions "in core > functions" seems increasingly out-dated. This particular function is in > ext/standard, which is about as "core" as you can get, but it is a brand > new function with no BC considerations and an explicit mandate to be as > secure as possible. > > To me, using an exception here makes perfect sense, but I'm not sure how > to formulate a policy that allows that without opening the floodgates too > quickly. I'm also a big fan of proper exception hierarchies - I've seen so > many times that you should never *catch* the base Exception class, so > *throwing* a base Exception seems very wrong to me. As I wrote earlier, I prefer exceptions rather than errors. Engine exception is a little different. The main motivation for engine exception is to give a chance to users for graceful program termination. Functions can achieve the objective by E_RECOVERABLE_ERROR mostly. I don't want to see one function raise exception and the other raise error. It's "should be avoided inconsistency". IMHO. If we are going to adopt exception for "functions", it would be better to have switch that convert all errors to exceptions. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Scott,
On Wed, Jul 15, 2015 at 7:19 PM, Scott Arciszewski
wrote:
> On Wed, Jul 15, 2015 at 4:27 AM, Yasuo Ohgaki wrote:
> > Hi Sammy,
> >
> > On Wed, Jul 15, 2015 at 6:04 AM, Sammy Kaye Powers wrote:
> >
> >> There are two open PR's for PHP7 to modify the behavior of the CSPRNG's:
> >>
> >> https://github.com/php/php-src/pull/1397 (main discussion)
> >> https://github.com/php/php-src/pull/1398
> >>
> >> Currently the random_*() functions will issue a warning and return
> false if
> >> a good source of random cannot be found. This is a potential security
> hole
> >> in the event the RNG fails and returns false which gets evaluated as 0
> in a
> >> cryptographic context.
> >>
> >> To prevent this exploit the proposed behavior will throw an Exception
> when
> >> the RNG fails or certain argument validation fails. This also gives the
> >> developer a graceful way to fall back to an alternate CSPRNG.
> >>
> >> Since the core functions in PHP don't throw Exceptions, there is debate
> on
> >> whether or not this change should be implemented. Some say the CSPRNG's
> >> should get a special pass since they will be relied on for
> cryptography. If
> >> we can't throw Exceptions, there were suggestions of raising a fatal
> error
> >> if the RNG fails.
> >>
> >> I think the argument can be boiled down to consistency vs security. We'd
> >> love to hear your feedback to decide what we should do in this context.
> :)
> >>
> >
> > I prefer exception rather than error.
> >
> > However, I would not like to see exception in "some" functions.
> > It's whether we use exception for builtin functions or not.
> >
> > I understand the risk, but users should handle all errors properly
> > to be secure anyway.
> >
> > Regards,
> >
> > --
> > Yasuo Ohgaki
> > yohg...@ohgaki.net
>
> -1 I emphatically disagree.
>
> > I understand the risk, but users should handle all errors properly
> > to be secure anyway.
>
> Recommended reading:
> http://cr.yp.to/talks/2015.01.07/slides-djb-20150107-a4.pdf
>
> ---
>
> Instead of placing all the burden on the user to add code to their
> application to check for errors, lest their application fail open and
> operate with zero entropy, which is what you've proposed, I say we
> throw an exception if the random number generator fails. This means
> that, in the default case, their application will fail closed (i.e.
> not continue processing with zero entropy), but the developer can
> still intuitively handle the exception if they want to add code to
> fail gracefully. Try-catch blocks are far less messy than overriding
> the error handler.
>
> Failing open is a terrible idea. You're going to have people who write
> code like this:
>
> $max = strlen($alphabet) - 1;
> for ($i = 0; $i < 32; ++$i) {
> $password .= $alphabet[random_int(0, $max)];
> }
>
> How do I know this? Because people already write code like that with
> mt_rand().
>
> If a random number generator failure occurs, instead of generating a
> predictable password, we ought to abort. If the developer wants to
> handle this situation, they ought to do this.
>
> try {
> $max = strlen($alphabet) - 1;
> for ($i = 0; $i < 32; ++$i) {
>$password .= $alphabet[random_int(0, $max)];
> }
> } catch (Exception $e) {
> $this->template('error', 'Could not safely generate a passphrase!
> :(');
> }
>
> In the case of a negligent developer, the uncaught exception is
> indistinguishable from raising a fatal error. Either way prevents the
> script from proceeding with zero entropy.
>
> TL;DR we have three options:
>
> 1. E_WARNING - fail open, possibly cause security problems for the user
> 2. E_ERROR - fail closed, but make graceful handling a pain in the neck
> 3. Exception - fail closed by default, developer can write their own
> graceful failure code if they so choose, would currently be an
> exception to the rule
>
> 1 is bad, 2 is less bad, I think 3 is ideal. Security > continuity.
>
The basis of my thought is user must write code in a way that would
never raise errors under normal conditions and user must stop execution
by any errors because they are unexpected.
To do that, user should have custom error handler always to achieve
graceful exit just like handling uncaught exceptions.
i.e. User should have something like following always.
getMessage(), "\n";
}
set_exception_handler('exception_handler');
?>
Server side web app is simple request/response app basically.
There is no point to continue execution when unexpected occurs.
Users should have something similar to above code anyway.
By the way, I agree that random_*() errors are critical. So I don't
mind to raise E_RECOVERABLE_ERROR from them. IMO, E_RECOVERABLE_ERROR
is better. The issue with E_ERROR is graceful exit. E_RECOVERABLE_ERROR
resolves
the issue.
Regards,
--
Yasuo Ohgaki
yohg...@ohgaki.net
Re: [PHP-DEV] Core functions throwing exceptions in PHP7
Hi Sammy, On Wed, Jul 15, 2015 at 6:04 AM, Sammy Kaye Powers wrote: > There are two open PR's for PHP7 to modify the behavior of the CSPRNG's: > > https://github.com/php/php-src/pull/1397 (main discussion) > https://github.com/php/php-src/pull/1398 > > Currently the random_*() functions will issue a warning and return false if > a good source of random cannot be found. This is a potential security hole > in the event the RNG fails and returns false which gets evaluated as 0 in a > cryptographic context. > > To prevent this exploit the proposed behavior will throw an Exception when > the RNG fails or certain argument validation fails. This also gives the > developer a graceful way to fall back to an alternate CSPRNG. > > Since the core functions in PHP don't throw Exceptions, there is debate on > whether or not this change should be implemented. Some say the CSPRNG's > should get a special pass since they will be relied on for cryptography. If > we can't throw Exceptions, there were suggestions of raising a fatal error > if the RNG fails. > > I think the argument can be boiled down to consistency vs security. We'd > love to hear your feedback to decide what we should do in this context. :) > I prefer exception rather than error. However, I would not like to see exception in "some" functions. It's whether we use exception for builtin functions or not. I understand the risk, but users should handle all errors properly to be secure anyway. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
[PHP-DEV] Re: PHP_INT_MIN is not defined?
On Wed, Jul 8, 2015 at 12:02 PM, Yasuo Ohgaki wrote: > while it is described in the manual. > > http://php.net/manual/en/reserved.constants.php > > What is going on?? > Oops. It says it's from PHP 7.0.0. Please disregard the mail. -- Yasuo Ohgaki yohg...@ohgaki.net
[PHP-DEV] PHP_INT_MIN is not defined?
Hi all, I noticed PHP_INT_MIN is not defined http://3v4l.org/QBavX while it is described in the manual. http://php.net/manual/en/reserved.constants.php What is going on?? Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Headsup: PHP7 feature freeze
Hi Anatol, On Mon, Jun 29, 2015 at 10:19 PM, Anatol Belski wrote: > > -Original Message- > > From: yohg...@gmail.com [mailto:yohg...@gmail.com] On Behalf Of Yasuo > > Ohgaki > > Sent: Friday, June 26, 2015 1:58 PM > > To: Hannes Magnusson > > Cc: Kalle Sommer Nielsen; Internals; Anatoliy Belsky; Dmitry Stogov; > Nikita > > Popov; Ferenc Kovacs; Xinchen Hui > > Subject: Re: [PHP-DEV] Headsup: PHP7 feature freeze > > > > Hi all, > > > > On Fri, Jun 26, 2015 at 12:56 PM, Yasuo Ohgaki > wrote: > > > > > Hi Hannes, > > > > > > On Fri, Jun 26, 2015 at 12:51 PM, Yasuo Ohgaki > wrote: > > > > > >> On Fri, Jun 26, 2015 at 10:48 AM, Hannes Magnusson < > > >> hannes.magnus...@gmail.com> wrote: > > >> > > >>> Why do you think its undocumented? > > >>> http://php.net/manual/en/sessionhandler.create-sid.php > > >>> > > >> > > >> Rename discussion was there. And I explicitly discussed "it's > > >> undocumented and it violates CODING_STANDARDS", but it was added > > >> recently (after the discussion I suppose). > > >> > > >> [yohgaki@dev session]$ svn log -r 334814 > > >> - > > >> --- > > >> r334814 | aharvey | 2014-09-09 04:49:26 +0900 (2014年09月09日 (火)) | 2 > > >> lines > > >> > > >> Add documentation for SessionHandler::create_sid(). > > >> > > >> - > > >> --- > > >> > > >> 334814aharvey SessionHandler is a > special > > >> class that can be used > > >> 334814aharvey to expose the current internal PHP session save > > >> handler by inheritance. > > >> 334814aharvey There are seven methods which wrap the seven > > >> internal session save handler > > >> 334814aharvey callbacks (open, > > >> close, > > >> 334814aharvey read, > > >> write, > > >> 334814aharvey destroy, > > >> gc and > > >> 334814aharvey create_sid). By default, > > >> this class will wrap > > >> 334814aharvey whatever internal save handler is set as > defined by > > >> the > > >> 334814aharvey > >> linkend="ini.session.save-handler">session.save_handler > > >> 334814aharvey configuration directive which is usually > > >> files by > > >> 334814aharvey default. Other internal session save handlers > are > > >> provided by PHP > > >> 334814aharvey extensions such as SQLite (as > > >> sqlite), Memcache (as > > >> 334814aharvey memcache), and Memcached > > (as > > >> 334814aharvey memcached). > > >> > > >> I think this should be reverted. > > >> > > > > > > Or it may stay there. > > > It's just a matter of having a copy of create_sid(). > > > I'll add documentation. > > > > > > > I forgot that session_create_id() is needed createSid() method to be more > > useful. > > > > The code for session_create_id() is in the source, but it isn't enabled. > > I wouldn't like to have different naming session_create_id() and > createSid(). > > > > So I would like to have > > - session_create_id() function > > - createId() function > > because there is > > - session_id() > > since PHP4. > > > > I don't think session internal names do not have to be changed. > > i.e. Macros, etc. > > Any comments? > > > Changing internal or user space API is kind of too late, IMHO. Especially > the user space APIs that are documented. But also the internals, as a lot > of extensions are already ported. Also because sessions are a core > functionality where changes should be supported but a good migration path. > Please target later 7.x versions with this change. But probably would make > sense to create an RFC and start the discussion like already... yesterday, > so the topic is good discussed and accepted for the next. No problem. I'll write a RFC for this. For the record, please don't document questionable APIs... I'll add comment to source if there are similar cases. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Headsup: PHP7 feature freeze
On Fri, Jun 26, 2015 at 8:57 PM, Yasuo Ohgaki wrote: > I don't think session internal names do not have to be changed. I meant "I think session internal names do not have to be changed". I don't mind at all cleanup them. -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Headsup: PHP7 feature freeze
Hi all, On Fri, Jun 26, 2015 at 12:56 PM, Yasuo Ohgaki wrote: > Hi Hannes, > > On Fri, Jun 26, 2015 at 12:51 PM, Yasuo Ohgaki wrote: > >> On Fri, Jun 26, 2015 at 10:48 AM, Hannes Magnusson < >> hannes.magnus...@gmail.com> wrote: >> >>> Why do you think its undocumented? >>> http://php.net/manual/en/sessionhandler.create-sid.php >>> >> >> Rename discussion was there. And I explicitly discussed "it's undocumented >> and it violates CODING_STANDARDS", but it was added recently (after the >> discussion I suppose). >> >> [yohgaki@dev session]$ svn log -r 334814 >> >> r334814 | aharvey | 2014-09-09 04:49:26 +0900 (2014年09月09日 (火)) | 2 lines >> >> Add documentation for SessionHandler::create_sid(). >> >> >> >> 334814aharvey SessionHandler is a special >> class that can be used >> 334814aharvey to expose the current internal PHP session save >> handler by inheritance. >> 334814aharvey There are seven methods which wrap the seven >> internal session save handler >> 334814aharvey callbacks (open, >> close, >> 334814aharvey read, >> write, >> 334814aharvey destroy, >> gc and >> 334814aharvey create_sid). By default, >> this class will wrap >> 334814aharvey whatever internal save handler is set as defined by >> the >> 334814aharvey > linkend="ini.session.save-handler">session.save_handler >> 334814aharvey configuration directive which is usually >> files by >> 334814aharvey default. Other internal session save handlers are >> provided by PHP >> 334814aharvey extensions such as SQLite (as >> sqlite), Memcache (as >> 334814aharvey memcache), and Memcached (as >> 334814aharvey memcached). >> >> I think this should be reverted. >> > > Or it may stay there. > It's just a matter of having a copy of create_sid(). > I'll add documentation. > I forgot that session_create_id() is needed createSid() method to be more useful. The code for session_create_id() is in the source, but it isn't enabled. I wouldn't like to have different naming session_create_id() and createSid(). So I would like to have - session_create_id() function - createId() function because there is - session_id() since PHP4. I don't think session internal names do not have to be changed. i.e. Macros, etc. Any comments? Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Headsup: PHP7 feature freeze
Hi Hannes, On Fri, Jun 26, 2015 at 12:51 PM, Yasuo Ohgaki wrote: > On Fri, Jun 26, 2015 at 10:48 AM, Hannes Magnusson < > hannes.magnus...@gmail.com> wrote: > >> Why do you think its undocumented? >> http://php.net/manual/en/sessionhandler.create-sid.php >> > > Rename discussion was there. And I explicitly discussed "it's undocumented > and it violates CODING_STANDARDS", but it was added recently (after the > discussion I suppose). > > [yohgaki@dev session]$ svn log -r 334814 > > r334814 | aharvey | 2014-09-09 04:49:26 +0900 (2014年09月09日 (火)) | 2 lines > > Add documentation for SessionHandler::create_sid(). > > > > 334814aharvey SessionHandler is a special > class that can be used > 334814aharvey to expose the current internal PHP session save > handler by inheritance. > 334814aharvey There are seven methods which wrap the seven > internal session save handler > 334814aharvey callbacks (open, > close, > 334814aharvey read, > write, > 334814aharvey destroy, > gc and > 334814aharvey create_sid). By default, > this class will wrap > 334814aharvey whatever internal save handler is set as defined by > the > 334814aharvey linkend="ini.session.save-handler">session.save_handler > 334814aharvey configuration directive which is usually > files by > 334814aharvey default. Other internal session save handlers are > provided by PHP > 334814aharvey extensions such as SQLite (as > sqlite), Memcache (as > 334814aharvey memcache), and Memcached (as > 334814aharvey memcached). > > I think this should be reverted. > Or it may stay there. It's just a matter of having a copy of create_sid(). I'll add documentation. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Headsup: PHP7 feature freeze
Hi Hannes, On Fri, Jun 26, 2015 at 10:48 AM, Hannes Magnusson < hannes.magnus...@gmail.com> wrote: > Why do you think its undocumented? > http://php.net/manual/en/sessionhandler.create-sid.php > Rename discussion was there. And I explicitly discussed "it's undocumented and it violates CODING_STANDARDS", but it was added recently (after the discussion I suppose). [yohgaki@dev session]$ svn log -r 334814 r334814 | aharvey | 2014-09-09 04:49:26 +0900 (2014年09月09日 (火)) | 2 lines Add documentation for SessionHandler::create_sid(). 334814aharvey SessionHandler is a special class that can be used 334814aharvey to expose the current internal PHP session save handler by inheritance. 334814aharvey There are seven methods which wrap the seven internal session save handler 334814aharvey callbacks (open, close, 334814aharvey read, write, 334814aharvey destroy, gc and 334814aharvey create_sid). By default, this class will wrap 334814aharvey whatever internal save handler is set as defined by the 334814aharvey session.save_handler 334814aharvey configuration directive which is usually files by 334814aharvey default. Other internal session save handlers are provided by PHP 334814aharvey extensions such as SQLite (as sqlite), Memcache (as 334814aharvey memcache), and Memcached (as 334814aharvey memcached). I think this should be reverted. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Headsup: PHP7 feature freeze
Hi all, On Fri, Jun 26, 2015 at 12:03 AM, Kalle Sommer Nielsen wrote: > This is a quick heads up that we plan to have the next release of > 7.0.0 be Beta 1, this marks a feature freeze and from there on, we > will switch focus on to stabilization, regressions and other bug > fixes. > > Beta 1 is schedule to be tagged and packaged on July 7th and released > on July 9th which is a small 2 weeks from now to get any remaining > changes of such in. > > If you are in doubt about whether or not your change would be > considered a 'feature' or have any other questions, then feel free to > mail us RMs or reply here. > I would like to rename "_undocumented_ method name" in Session module. http://php.net/manual/en/book.session.php The line is this. https://github.com/php/php-src/blob/master/ext/session/php_session.h#L334 create_sid() method should be createSid(). If some users used it, all they have to do is define copy of create_sid() method and compatibility is kept. Any comments? Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Optimizing php_html_entities()
Hi Andrey, On Wed, Jun 24, 2015 at 6:20 PM, Andrey Andreev wrote: > On Wed, Jun 24, 2015 at 5:49 AM, Yasuo Ohgaki wrote: > > Hi Xinchen, > > > > On Wed, Jun 24, 2015 at 11:42 AM, Xinchen Hui wrote: > > > >> and for the "age" usage you replied in github, I think the author of > >> such codes should be aware, if it's only number, then instead of > >> htmlespcicalchars($age), he should use echo $age directly... which is > >> more faster. > >> > > > > To build secure apps, users MUST escape everything for the context by > > _default_. > > Selective escaping is the cause of injection vulnerability especially > with > > language like > > PHP. > > > > Principle is "Don't think, escape all (for the context)". > > > > The key word here is "context" ... you know that there's nothing to > escape for an integer, because the type is your context. > > Selective escaping isn't a problem by itself, but that many people use > a blacklist approach instead of a whitelist one; and you can only fix > that with education. Right and agree. Selective escaping isn't a problem by itself, but people do mistake, make wrong assumptions, use wrong blacklist approach. The same variable can be generated by different code path/source. It could be very hard to assure a variable is really a int/float without validation. If one would like to make sure what a variable is and skip escaping, they need something like It's much easier with unconditional escape everywhere like if htmlspecialchars() is fast enough for int/float. (I'm not sure which one is faster) One example is SQLite that making sure variable type could be difficult. SQLite can store string _regardless_ of type definition. If developers add SQLite support, in addition to MySQL/PostgreSQL, they may create attack vector if they don't escape unconditionally. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Move to Fast ZPP?
Hi Ferenc, On Wed, Jun 24, 2015 at 7:29 PM, Ferenc Kovacs wrote: > > > On Wed, Jun 24, 2015 at 12:21 PM, Yasuo Ohgaki wrote: > >> Hi Ferenc, >> >> On Wed, Jun 24, 2015 at 6:21 PM, Ferenc Kovacs wrote: >> >>> it was meant as a performance optimization for the most heavy/hot >>> codepaths. >>> What gave you the idea that the situation changed and we should use it >>> everywhere? >>> >> >> I'm curious about the status of Fast ZPP and it's usage. >> The RFC says it reduces function call overheads by 90% and >> I didn't see reason why we shouldn't move to Fast ZPP. >> >> "For some really simple function like is_string() or ord() the overhead >> of zend_parse_parameters() may be about 90%." >> https://wiki.php.net/rfc/fast_zpp >> >> I'll use it for functions called many times. >> >> > I wouldn't consider the likes of pg_fetch_* to be appropriate for for > using Fast ZPP, even db heavy applications are unlikely to call it more > than a houndred times per request, so the perf gain would be negligible > compared to the increased cost of maintenance. > I agree. It's painful to maintain #ifndef FAST_ZPP zend_parse_parameter(); #else Fast ZPP #endif I'll just leave them alone until #ifndef is not required. Thanks. -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Move to Fast ZPP?
Hi Ferenc, On Wed, Jun 24, 2015 at 6:21 PM, Ferenc Kovacs wrote: > it was meant as a performance optimization for the most heavy/hot > codepaths. > What gave you the idea that the situation changed and we should use it > everywhere? > I'm curious about the status of Fast ZPP and it's usage. The RFC says it reduces function call overheads by 90% and I didn't see reason why we shouldn't move to Fast ZPP. "For some really simple function like is_string() or ord() the overhead of zend_parse_parameters() may be about 90%." https://wiki.php.net/rfc/fast_zpp I'll use it for functions called many times. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Move to Fast ZPP?
Hi Dmitry, On Wed, Jun 24, 2015 at 7:04 PM, Dmitry Stogov wrote: > We should NOT use it everywhere. It'll lead to code bloat. OK. Thank you. I'll use it only for functions called many times. e.g. pg_fetch_*(). Should I use #ifndef FAST_ZPP? Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Optimizing php_html_entities()
Hi all,
On Wed, Jun 24, 2015 at 12:20 PM, Yasuo Ohgaki wrote:
> On Wed, Jun 24, 2015 at 12:05 PM, Juan Basso wrote:
>
>> Did you test the performance impact on strings? Since you changed how it
>> works the impact can be positive and maybe worth to make the method more
>> broad.
>
>
> It's a bit slower (~3%) for 'abc' as input string. If I remove redundant
> type
> check(convert_to_string), it's better though. ~3% seems too large for 2
> additional if statements - parameter type check. I'll look into.
>
> [yohgaki@dev github-php-src]$ cat b.php
> const LOOP=1;
>
> $start = microtime(true);
> for ($i = 0; $i < LOOP; $i++) {
> }
> $loop_time = microtime(true) - $start;
>
>
> $start = microtime(true);
> for ($i = 0; $i < LOOP; $i++) {
> htmlspecialchars('abc');
> }
> echo 'Time: '.(microtime(true) - $start - $loop_time)."\n";
>
> [yohgaki@dev github-php-src]$ ./php.old b.php
> Time: 4.4925589561462
> [yohgaki@dev github-php-src]$ ./php.old b.php
> Time: 4.4821939468384
> [yohgaki@dev github-php-src]$ ./php.new b.php
> Time: 4.5892491340637
> [yohgaki@dev github-php-src]$ ./php.new b.php
> Time: 4.6097931861877
>
Followings are better approximation. New code is a bit slower even if it's
in
benchmark error range. ~1% - ~3% slower for strings while it's much faster
(~70% or more. This case is more than 100%) for integers depending on
integer size. Since integer/float is much faster, only a few presence of
int/float
negates negative impact.
As you can see, loop only benchmark varies a lot.
I also counted total number of CPU instructions by perf. It varies too.
However, new code is a bit slower for sure even if there is only 2 "if"
addition
for strings. I was expecting use of raw zval negates this...
I would like to apply similar change to most string escape/conversion
functions.
(Please note that subject is "escape and conversion" function only)
Current patch allows "resource" type, but it should raise type error.
Comments are appreciated.
Sorry for flood of mails.
Regards,
[yohgaki@dev github-php-src]$ ./php.old b.php
Loop time
Loop: 0.08224892616272
Loop: 0.08589506149292
Loop: 0.087536096572876
Loop: 0.082170963287354
Loop: 0.082328081130981
Loop: 0.081686019897461
Loop: 0.087927103042603
Loop: 0.081233024597168
Loop: 0.082290887832642
Loop: 0.081492900848389
Loop: 0.081452131271362
Loop: 0.081291913986206
Loop Avg: 0.08220899105072
Escaping string abcdefgh
Time: 0.6288548707962
Time: 0.61239206790924
Time: 0.6169821023941
Time: 0.61680710315704
Time: 0.60974395275116
Time: 0.61229383945465
Time: 0.61705410480499
Time: 0.61985099315643
Time: 0.61954915523529
Time: 0.62286603450775
Time: 0.62001097202301
Time: 0.6216961145401
Time Avg: 0.6166380405426
Escaping integer 12345678
Time: 0.98103010654449
Time: 0.97324693202972
Time: 0.96535289287567
Time: 0.962033867836
Time: 0.95926511287689
Time: 0.9667671918869
Time: 0.96851003170013
Time: 0.96103513240814
Time: 0.96122300624847
Time: 0.95892608165741
Time: 0.96553599834442
Time: 0.96350586414337
Time Avg: 0.96321551799774
[yohgaki@dev github-php-src]$ ./php.new b.php
Loop time
Loop: 0.082480907440186
Loop: 0.081701993942261
Loop: 0.081080913543701
Loop: 0.080965042114258
Loop: 0.081230163574219
Loop: 0.08064603805542
Loop: 0.081595897674561
Loop: 0.081001043319702
Loop: 0.080890893936157
Loop: 0.084527969360352
Loop: 0.082308053970337
Loop: 0.081248998641968
Loop Avg: 0.081266903877258
Escaping string abcdefgh
Time: 0.62283012866974
Time: 0.62788126468658
Time: 0.63206312656403
Time: 0.62420103549957
Time: 0.62370917797089
Time: 0.62745521068573
Time: 0.626731133461
Time: 0.68132016658783
Time: 0.62430212497711
Time: 0.63127825260162
Time: 0.63222811222076
Time: 0.62956402301788
Time Avg: 0.62700154781342
Escaping integer 12345678
Time: 0.41526029109955
Time: 0.41403410434723
Time: 0.41262814998627
Time: 0.41230318546295
Time: 0.41555306911469
Time: 0.40698001384735
Time: 0.41639611721039
Time: 0.41320106983185
Time: 0.41369721889496
Time: 0.42157099246979
Time: 0.42368505001068
Time: 0.42184422016144
Time Avg: 0.4141624212265
[yohgaki@dev github-php-src]$ cat b.php
Re: [PHP-DEV] Optimizing php_html_entities()
Hi Anthony, On Wed, Jun 24, 2015 at 12:00 PM, Yasuo Ohgaki wrote: > On Wed, Jun 24, 2015 at 10:40 AM, Anthony Ferrara > wrote: > >> > >> > IMHO, escape/unescape/encode/decode/conversion function is better to >> accept >> > any types. >> > HTML template may be separated script, but database code etc may not. >> > >> > Writing code like >> > >> > > > declare(strict_types=1); >> > $sql = 'SELECT * FROM '. pg_escape_identifier((string)$table). ' WHERE >> id '. >> > pg_escpae_literal((string)$id).';'; >> > pg_query($sql); >> > ?> >> > >> > is better to be avoided. i.e. (string) cast before passing parameter. >> >> I agree 100%. Instead, the developer should get an error if the >> parameter is not a string. Because it is an error. If you're passing >> > an array to `pg_escape_identifier`, you have FAR WORSE problems. >> Having the function accept anything and return anything (as you're >> proposing) would eliminate any ability to detect this problem. >> > > I agree 100%. > > >> If people blind cast, that's their problem. We shouldn't make it >> harder for people to detect problems by blindly accepting anything >> under the sun. >> > > strict_types=1 creates issue for int/float which is valid and accepted > without strict_types. > > We will have mixed types due to type hint and it's problematic. > If escape functions accept string/int/float/object(only when > it has __toString), it's easier for users. Safety is guaranteed also. > > Other than escape/conversion functions that expect "string" type > should get type errors. > > > > Another example. JSON decode convert numeric to int/float >> > >> > > > declare(strict_types=1); >> > $data = json_decode($json); >> > $str = mb_convert_kana((string) $data['some_data'], 'AKHV'); >> > ?> >> > >> > Are we going to enforce users to use (string) casts for conversion >> functions >> > to switch >> > strict_types=1? >> >> No, the entire point is to have them actually validate the types. > > > I fully agree. > > But people will do this, unless we make conversion functions accept > safe/valid scalars/objects... Or worse, people make assumption that > variables are safe to output w/o escape... > > Since there weren't contracts before PHP7, I think we may adjust contract > for some functions before PHP7 release. > Another reason of this optimization is type affinity that I would like to add. https://wiki.php.net/rfc/introduce-type-affinity With type affinity, "integer string"/"float string" are converted to int/float type and PHP can execute script more efficiently with type hint. i.e. Type affinity eliminates countless "string" -> "int/float" conversions with weak type hint. Issue with converted values is int/float cannot escape as it is now because they expect "string" strictly with "strict_types=1", as well as it requires needless type conversions to "string" in weak mode. e.g. htmlspecialchars, pg_escape_string/identifier/literal, etc. Therefore, I would like to allow int/float for these functions. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Optimizing php_html_entities()
Hi Juan, On Wed, Jun 24, 2015 at 12:05 PM, Juan Basso wrote: > Did you test the performance impact on strings? Since you changed how it > works the impact can be positive and maybe worth to make the method more > broad. It's a bit slower (~3%) for 'abc' as input string. If I remove redundant type check(convert_to_string), it's better though. ~3% seems too large for 2 additional if statements - parameter type check. I'll look into. [yohgaki@dev github-php-src]$ cat b.php
Re: [PHP-DEV] Optimizing php_html_entities()
Hi Anthony, On Wed, Jun 24, 2015 at 10:40 AM, Anthony Ferrara wrote: > > > > IMHO, escape/unescape/encode/decode/conversion function is better to > accept > > any types. > > HTML template may be separated script, but database code etc may not. > > > > Writing code like > > > > > declare(strict_types=1); > > $sql = 'SELECT * FROM '. pg_escape_identifier((string)$table). ' WHERE > id '. > > pg_escpae_literal((string)$id).';'; > > pg_query($sql); > > ?> > > > > is better to be avoided. i.e. (string) cast before passing parameter. > > I agree 100%. Instead, the developer should get an error if the > parameter is not a string. Because it is an error. If you're passing > an array to `pg_escape_identifier`, you have FAR WORSE problems. > Having the function accept anything and return anything (as you're > proposing) would eliminate any ability to detect this problem. > I agree 100%. > If people blind cast, that's their problem. We shouldn't make it > harder for people to detect problems by blindly accepting anything > under the sun. > strict_types=1 creates issue for int/float which is valid and accepted without strict_types. We will have mixed types due to type hint and it's problematic. If escape functions accept string/int/float/object(only when it has __toString), it's easier for users. Safety is guaranteed also. Other than escape/conversion functions that expect "string" type should get type errors. > Another example. JSON decode convert numeric to int/float > > > > > declare(strict_types=1); > > $data = json_decode($json); > > $str = mb_convert_kana((string) $data['some_data'], 'AKHV'); > > ?> > > > > Are we going to enforce users to use (string) casts for conversion > functions > > to switch > > strict_types=1? > > No, the entire point is to have them actually validate the types. I fully agree. But people will do this, unless we make conversion functions accept safe/valid scalars/objects... Or worse, people make assumption that variables are safe to output w/o escape... Since there weren't contracts before PHP7, I think we may adjust contract for some functions before PHP7 release. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Optimizing php_html_entities()
Hi Xinchen, On Wed, Jun 24, 2015 at 11:42 AM, Xinchen Hui wrote: > and for the "age" usage you replied in github, I think the author of > such codes should be aware, if it's only number, then instead of > htmlespcicalchars($age), he should use echo $age directly... which is > more faster. > To build secure apps, users MUST escape everything for the context by _default_. Selective escaping is the cause of injection vulnerability especially with language like PHP. Principle is "Don't think, escape all (for the context)". Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Optimizing php_html_entities()
Hi Xinchen, On Wed, Jun 24, 2015 at 11:31 AM, Xinchen Hui wrote: > > > > On Wed, Jun 24, 2015 at 8:32 AM, Yasuo Ohgaki wrote: > > Hi Xinchen, > > > > On Tue, Jun 23, 2015 at 11:33 PM, Xinchen Hui wrote: > >> > >> But passing an non-string to htmlspecialchars are not common used > cases.. > >> > >> "optimize" not common used cases... will bring nothing to us.. > > > > > > The reason why I looked into this in the first place is one of my friend > > complained about slow htmlspecialchars for relatively large table data. > > > > He said more than half of execution time is took by htmlspecialchars > > because > > > > - it escapes numeric values as string > > - it does not accept array value for to be escaped values > > > > 1st is covered by this patch. > > 2nd issue is covered by this FR. > > https://bugs.php.net/bug.php?id=69908 > > > > So it brings something for us :) > Then what about strlen(number)? are you also going to change its ZPP > to accept zval instead of string? No. Only some kind of conversion functions are the subject. e.g. mb_convert_encoding(), mb_convert_kana(), etc. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
[PHP-DEV] Move to Fast ZPP?
Hi all, I'm wondering the state of Fast ZPP. https://wiki.php.net/rfc/fast_zpp Last year's Dmirty's commit message say, it should only applied to most used functions due to the state of Fast ZPP. commit 27f38798a1963de1c60aae4ef8a3675138255574 Author: Dmitry Stogov Date: Fri Jul 11 16:32:20 2014 +0400 Fast parameter parsing API This API is experimental. It may be changed or removed. It should be used only for really often used functions. (Keep the original parsing code and wrap usage with #ifndef FAST_ZPP) Are we supposed to convert anything to Fast ZPP now? What about the #ifndef FAST_ZPP? Is this required now? BTW, if we are supposed to use only Fast ZPP, https://github.com/php/php-src/blob/master/README.PARAMETER_PARSING_API should be updated. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Re: Typed Arrays in Scalar Types RFC
Hi all, On Wed, Jun 24, 2015 at 5:20 AM, Christoph Becker wrote: > > I would like to submit an RFC for supporting typed arrays as > function/method argument and return types. Please either throw stones at me > or give me enough karma to post it. > > > > My php.net <http://php.net/> username is tim-bezhashvyly. > > Note that there is already <https://wiki.php.net/rfc/arrayof>, which has > been declined. I would like to have something similar to JSON Schema Validation. This could be a function unlike arrayof. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Optimizing php_html_entities()
Hi Xinchen, On Tue, Jun 23, 2015 at 11:33 PM, Xinchen Hui wrote: > But passing an non-string to htmlspecialchars are not common used cases.. > > "optimize" not common used cases... will bring nothing to us.. > The reason why I looked into this in the first place is one of my friend complained about slow htmlspecialchars for relatively large table data. He said more than half of execution time is took by htmlspecialchars because - it escapes numeric values as string - it does not accept array value for to be escaped values 1st is covered by this patch. 2nd issue is covered by this FR. https://bugs.php.net/bug.php?id=69908 So it brings something for us :) Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Optimizing php_html_entities()
Hi all,
On Wed, Jun 24, 2015 at 6:51 AM, Yasuo Ohgaki wrote:
> I got it.
>
> On Wed, Jun 24, 2015 at 6:41 AM, Yasuo Ohgaki wrote:
>
>> On Wed, Jun 24, 2015 at 12:21 AM, Anthony Ferrara
>> wrote:
>>
>>> In addition, this breaks the contract, specifically when using scalar
>>> types. Because you're no longer going to error when the contract is
>>> broken (considering htmlspecialchars is documented as string:string).
>>>
>>
>> What do you mean by "break the contract".
>> "string" parameter is not a requirement/contract.
>> htmlspecialchars/htmlentities
>> just converts param to string. The patch does not change anything as you
>> can
>> see it from the phpt results.
>>
>
> [yohgaki@dev github-php-src]$ cat ../t.php
> declare(strict_types=1);
>
> var_dump( htmlspecialchars(123) );
>
> [yohgaki@dev github-php-src]$ ./php-bin ../t.php
>
> Fatal error: Uncaught TypeError: htmlspecialchars() expects parameter 1 to
> be string, integer given in /home/yohgaki/workspace/ext/git/oss/
> php.net/t.php:4
> Stack trace:
> #0 /home/yohgaki/workspace/ext/git/oss/php.net/t.php(4):
> htmlspecialchars(123)
> #1 {main}
> thrown in /home/yohgaki/workspace/ext/git/oss/php.net/t.php on line 4
>
>
> I think this is massive breakage. It only happens in strict mode, though.
>
IMHO, escape/unescape/encode/decode/conversion function is better to accept
any types.
HTML template may be separated script, but database code etc may not.
Writing code like
is better to be avoided. i.e. (string) cast before passing parameter.
Another example. JSON decode convert numeric to int/float
Are we going to enforce users to use (string) casts for conversion
functions to switch
strict_types=1?
Regards,
--
Yasuo Ohgaki
yohg...@ohgaki.net
Re: [PHP-DEV] Optimizing php_html_entities()
Hi Anthony, I got it. On Wed, Jun 24, 2015 at 6:41 AM, Yasuo Ohgaki wrote: > On Wed, Jun 24, 2015 at 12:21 AM, Anthony Ferrara > wrote: > >> In addition, this breaks the contract, specifically when using scalar >> types. Because you're no longer going to error when the contract is >> broken (considering htmlspecialchars is documented as string:string). >> > > What do you mean by "break the contract". > "string" parameter is not a requirement/contract. > htmlspecialchars/htmlentities > just converts param to string. The patch does not change anything as you > can > see it from the phpt results. > [yohgaki@dev github-php-src]$ cat ../t.php
Re: [PHP-DEV] Optimizing php_html_entities()
Hi Anthony, On Wed, Jun 24, 2015 at 12:21 AM, Anthony Ferrara wrote: > In addition, this breaks the contract, specifically when using scalar > types. Because you're no longer going to error when the contract is > broken (considering htmlspecialchars is documented as string:string). > What do you mean by "break the contract". "string" parameter is not a requirement/contract. htmlspecialchars/htmlentities just converts param to string. The patch does not change anything as you can see it from the phpt results. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Optimizing php_html_entities()
Hi Xinchen, On Tue, Jun 23, 2015 at 11:33 PM, Xinchen Hui wrote: > But passing an non-string to htmlspecialchars are not common used cases.. > > "optimize" not common used cases... will bring nothing to us.. > The reason why I brought up this now is scalar type hint. Before PHP7, people didn't not care if data sent from browser is actually a string. e.g. age, month, date, etc. However, this optimization have more effects because of PHP7's type hint that convert data type "always" and users must escape regardless of it's type. Wrong date type assumption is common source of JavaScript injections. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
[PHP-DEV] Optimizing php_html_entities()
Hi all, I'm trying to optimize php_html_entities(). Since htmlspecialchars()/htmlentities() are sensitive function, I would like to ask comments before merge. https://github.com/php/php-src/pull/1356 Current php_html_entities() convert int/float/etc to string, then convert it. int/float/etc is not required to be escaped. Optimize it by simply converting them to string. Simple benchmark shows about 60% execution time reduction. [yohgaki@dev github-php-src]$ ./php-bin b.php Time: 10.484607934952 [yohgaki@dev github-php-src]$ ./php-bin b.php Time: 10.867615222931 [yohgaki@dev github-php-src]$ ./php.new b.php Time: 3.9379420280457 [yohgaki@dev github-php-src]$ ./php.new b.php Time: 4.0694711208344 [yohgaki@dev github-php-src]$ cat b.php
Re: [PHP-DEV] Revisit trailing commas in function call parameters?
Hi Christian, On Wed, Jun 17, 2015 at 9:05 PM, Christian Schneider wrote: > Hi there, > now that syntax for variadic functions was added I quickly wanted to ask > whether allowing trailing commas for function calls could be reconsidered. > > It is a very small change to the language allowing something like > my_variadic_function( > "foo", > "bar", > "qux", > ); > which makes adding/removing additional parameters a little easier: No > comma has to be added/removed from the last line. > > Previously this was considered unnecessary but maybe things changed enough > to reconsider it? It doesn't involve any BC break and I attached the very > simple patch needed in the master branch. > > If people still consider it more harm- than useful then please don't flame > me and I'll shut up again :-) > PHP allows array( 1, 2, 3, ); therefore my_variadic_function( "foo", "bar", "qux", ); is consistent behavior to me. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Re: [VOTE] JSON number to string
Hi Jakub, On Wed, Jun 17, 2015 at 5:16 AM, Jakub Zelenka wrote: > On Tue, Jun 9, 2015 at 7:23 PM, Jakub Zelenka wrote: > > > > > The voting is now open: > > > > https://wiki.php.net/rfc/json_numeric_as_string#voting > > > > > The RFC has been rejected (all 4 proposed options). > > The bug https://bugs.php.net/bug.php?id=68456 (proposing such flag for > float decoding) will be closed as "Won't fix" because the result shows that > this is not the right way how to tackle the problem. The json shema has > already its feature request ( https://bugs.php.net/bug.php?id=69422 ) and > should be the way going forward in my opinion. It requires a lot more codes to make it work (both user and PHP), but it seems it's the only way to retrieve "correct raw value". Looking forward the patch. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Packed array is not fast?
Hi Dmitry, On Mon, Jun 15, 2015 at 6:06 PM, Dmitry Stogov wrote: > I see only the test for "packed" array. You may create similar "hash" > array, initializing it in reverse order. > It's better approach to see the difference because the array content is basically the same. > > In my experiments "packed" arrays are slightly (8%) faster. > > $ sapi/cli/php packed.php > Time: 0.022471189498901 > Time: 0.012310028076172 > > $ sapi/cli/php hash.php > Time: 0.024425029754639 > Time: 0.012874126434326 It's faster on my PC, too. 8% is good enough to have. I'm surprised that PHP's hash is super fast :) Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Packed array is not fast?
Hi Levi, On Mon, Jun 15, 2015 at 12:35 PM, Levi Morrison wrote: > How are you testing hash vs packed? As far as what you posted I cannot > tell a difference – it looks like you are running the same thing twice > (same binary and same input file). > I've modified START index and verified both packed and unpacked array is used by using gdb. I didn't analyze whole execution, but only zend_hash.c I didn't paste slightly different code because it will not give us much difference and it's useless services like 3v4l. I'm a little disappointed by the result because I was expecting much better result with packed array.. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Array dereferencing of scalars
Hi all, On Fri, Jun 12, 2015 at 9:36 AM, Chris Wright wrote: > On 12 June 2015 at 00:35, Rowan Collins wrote: > > > On 11/06/2015 21:22, Chris Wright wrote: > > > >> I'm inclined to agree that we should have consistency here, and that the > >> current behaviour in a function context is the correct one. A couple of > >> (IMO) good arguments for this: > >> > >> - The function behaviour gives a more explanatory and useful error > message > >> - This is definitely a programming error, there is no valid reason to > use > >> NULL as a variable name in this manner. It amounts to what many/most > other > >> languages would consider to be a "null pointer exception" or equivalent, > >> and the more useful error message may help track down the error quicker > >> (one might look for a case where the empty string was assigned, instead > of > >> looking for a case where it may be NULL). > >> > > > > Well, if you were to model it precisely on the function case, the > > *consistent* behaviour would be to error on any non-string value (e.g. > $v = > > 42; $$v;) but not a string which is an illegal variable name ($v = '42'; > > $v(); just complains that function 42 doesn't exist, not that it never > > could). > > > > A more useful approach would perhaps be to look at what should be a valid > > function name, and what should be a valid variable name, and throw an > error > > if dynamic access is used to by-pass either restriction. > > > > +1 > > If anything is to be done, then this should be it - properly abstract the > underlying structures so that the lexical/conventional limitations on > symbol naming are also imposed by mechanisms that let userland directly > manipulate the names of symbols (define(), class_alias() etc). +1 It sounds good change to spot possible bugs. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Array dereferencing of scalars
Hi Rowan,
On Thu, Jun 11, 2015 at 5:59 PM, Rowan Collins
wrote:
> Yasuo Ohgaki wrote on 11/06/2015 00:50:
>
>> If PHP should return NULL always against NULL variables, we may be better
>> to
>> reconsider these behavior.
>>
>> [yohgaki@dev Download]$ php
>> > $v = NULL;
>> $$v;
>>
>>
>> PHP Notice: Undefined variable: in - on line 3
>>
>
> This is not complaining that $v is NULL, it is warning you the same as if
> you wrote $v = 'some_name_you_never_assigned_to'; $$v;
>
> Somewhat surprisingly, you can actually assign to the variable whose name
> is NULL: http://3v4l.org/5pXJg
>
>
> [yohgaki@dev Download]$ php
>> > $v = NULL;
>> $v();
>>
>> PHP Fatal error: Uncaught EngineException: Function name must be a string
>> in -:3
>> Stack trace:
>> #0 {main}
>>thrown in - on line 3
>>
>
> This one has given a different message because of it being a non-string
> value, but would be equally fatal if you tried to call any undefined
> function. Would coercing NULL to an imaginary function which returns NULL
> actually be useful for anything?
For example, getting NULL handler as the default.
This is useful like $v=NULL;$v[1][2][3];
> I don't care much whether these yield NULL always or raise
>> error/exception,
>> but there should be consistency.
>>
>
> I see no reason why these cases need to be consistent, just because they
> both involve NULLs. It's all about the action you invoke, not just the
> value something's interpreted as.
I think those who are familiar with NULL in RDBMS might make sense
returning NULL for all.
NULL is special value and how it should be behaved is debatable.
Raising some kind of error for all would be nicer including
$v=NULL;$v[1][2][3]; IMO.
Less exceptions are better.
Regards,
--
Yasuo Ohgaki
yohg...@ohgaki.net
[PHP-DEV] Packed array is not fast?
Hi all, I've tried to see how packed array is faster with following code http://3v4l.org/TQd6c I confirmed when start index is non-zero, hash is used by zend_hash_index_find(). I got following result. (with much larger number of elements/loops) Fedora 22 + current master without --enable-debug 1st "Time" is total execution time. 2nd "Time" is the time spent by "for loop" Hash [yohgaki@dev php-src]$ ./php-bin ~/tmp/array_bench2.php Time: 1.7903809547424 Time: 1.1529920101166 [yohgaki@dev php-src]$ ./php-bin ~/tmp/array_bench2.php Time: 1.8049499988556 Time: 1.1719739437103 Packed [yohgaki@dev php-src]$ ./php-bin ~/tmp/array_bench2.php Time: 1.7407248020172 Time: 1.1594388484955 [yohgaki@dev php-src]$ ./php-bin ~/tmp/array_bench2.php Time: 1.7248120307922 Time: 1.1530420780182 Packed array is not so fast, even if zend_hash.c seems much faster with packed array. Just FYI. Regards, P.S. Am I doing something wrong? HHVM seems to have optimization margins for hash and loop. -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Migrating PHP classes to built in namespace
Hi Tony, On Thu, Jun 11, 2015 at 4:37 PM, Tony Marston wrote: > "Yasuo Ohgaki" wrote in message > news:CAGa2bXZvuqV4Kwru+wUL-bfTb9_tnv=l16reu-+w+uajx4h...@mail.gmail.com... > >> >> Hi Jakub, >> >> On Thu, Jun 11, 2015 at 7:43 AM, Jakub Kubícek >> >> wrote: >> >> This is weird and ugly what you're fabricating here. The PHP classes >>> should be all in global namespace. I am really against some >>> namespacing of them. >>> >>> >> It may seems weird if you aren't familiar with languages support >> this kind of feature. However, you'll find many languages support >> this or similar. >> >> JavaScript is one of them. JavaScript allows to override any functions, >> for example. It's works like importing other namespace function into >> root. i.e. Provide different feature with the same name. >> >> For long term PHP evolution, having ability to import some namespace >> into root namespace is great feature. i.e. API/Module version up. We >> can provide both old and new during migration. Use of namespace is >> better approach because it may give us super clean global namespace also. >> >> Regards, >> >> -- >> Yasuo Ohgaki >> yohg...@ohgaki.net >> > > The fact that other languages have this feature should not be a reason to > add it to PHP, especially as it would provide a huge BC break with > absolutely no practical, only theoretical, benefits. > It does not create huge BC. It's matter of adding a line for namespace or having a default namespace setting. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Migrating PHP classes to built in namespace
Hi Jakub, On Thu, Jun 11, 2015 at 7:43 AM, Jakub Kubíček wrote: > This is weird and ugly what you're fabricating here. The PHP classes > should be all in global namespace. I am really against some > namespacing of them. > It may seems weird if you aren't familiar with languages support this kind of feature. However, you'll find many languages support this or similar. JavaScript is one of them. JavaScript allows to override any functions, for example. It's works like importing other namespace function into root. i.e. Provide different feature with the same name. For long term PHP evolution, having ability to import some namespace into root namespace is great feature. i.e. API/Module version up. We can provide both old and new during migration. Use of namespace is better approach because it may give us super clean global namespace also. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Re: [VOTE] Throwable Interface
Hi Aaron, On Thu, Jun 11, 2015 at 2:50 AM, Aaron Piotrowski wrote: > > On Jun 10, 2015, at 12:29 PM, Christoph Becker > wrote: > > > > Aaron Piotrowski wrote: > > > >> Voting is now open on the Throwable Interface RFC: > >> > >> https://wiki.php.net/rfc/throwable-interface > > > > When will the voting end? It seems to be best to clearly state that in > > the RFC also. > > > > -- > > Christoph M. Becker > > > > Thank you, I’ve added this to the RFC. Voting will be open through June > 24th. You need new mail thread titled "[RFC][VOTE] Throwable Interface". Otherwise, people like me will miss it. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Array dereferencing of scalars
Hi all, On Thu, Jun 11, 2015 at 7:22 AM, Yasuo Ohgaki wrote: > I agree that NULL is debatable. In PHP, NULL is treated as 0/false by its > context. > It's simpler if we get rid of the behavior altogether. IMO. > If PHP should return NULL always against NULL variables, we may be better to reconsider these behavior. [yohgaki@dev Download]$ php
Re: [PHP-DEV] Array dereferencing of scalars
Hi Stas and Nikita, On Wed, Jun 10, 2015 at 5:30 PM, Nikita Popov wrote: > On Wed, Jun 10, 2015 at 8:53 AM, Stanislav Malyshev > wrote: > >> Hi! >> >> > > > $foo = 42; >> > $foo['bar']; // => NULL >> > $v = NULL; >> > $v[0][1][2][3][4][5][6][7][8][9]; // NULL >> > >> > this code is semantically wrong and I would like to have error/exception >> > for such >> > erroneous codes. It's inconsistent with array object, too. >> >> Why it's wrong? You try to get that's something not there, you get NULL. >> I don't see anything wrong. Adding fatal error about every little thing >> that isn't as expected never was how PHP worked. > > > I agree with what other people have said here: We should keep the behavior > for NULL, but drop the nonsense for other types - (42)[24] not throwing a > notice is quite ridiculous, that seems like a pretty obvious bug to me. > I agree that NULL is debatable. In PHP, NULL is treated as 0/false by its context. It's simpler if we get rid of the behavior altogether. IMO. Anyway, removing undefined behavior from other types may be good enough so I don't insist. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Re: PHP-7.0.0 branch
Hi Ferenc, On Wed, Jun 10, 2015 at 3:10 PM, Ferenc Kovacs wrote: > as mentioned in my original mail to the list: > "PHP-7.0.0 will be only used by the RMs to tag the alpha/beta/RC versions > from." > so, no, you aren't supposed to merge anything there. > It seems I'm better to check filter. I cannot find it in my local mailbox :( Anyway I got it. Thank you. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] [VOTE] JSON number to string
Hi Jakub, On Wed, Jun 10, 2015 at 8:09 PM, Jakub Zelenka wrote: > The idea would be to use JSON schema just for selecting the type for > deserialized item in decoded JSON. It means that you could for example > select one float item that you want to decode as a string but other floats > would be still decoded as floats. That gives a better flexibility and helps > the problem in the same way. Please see my email to Alexey when we > discussed this RFC (there are few examples): > > https://www.mail-archive.com/internals@lists.php.net/msg78698.html > Thank you for the reply. This could be done, but requires more work just to get raw value out of JSON. So I vote "yes" for all because your RFC is feasible resolution and useful enough. As I wrote in somewhere in discussion, it was better if all values are decoded as string just like DB values. Encoding is headache, though.. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] [VOTE] JSON number to string
On Wed, Jun 10, 2015 at 7:57 PM, Yasuo Ohgaki wrote: > On Wed, Jun 10, 2015 at 7:28 PM, Derick Rethans wrote: > >> > Hi, >> > >> > The voting is now open: >> > >> > https://wiki.php.net/rfc/json_numeric_as_string#voting >> >> FWIW, I voted no to all of them because (as you even say in the RFC), >> this is probably better be solved with a JSON schema thingy. > > > JSON Schema does not help to decoding/encoding numeric data without > loosing data. > > JSON Schema Core > http://json-schema.org/latest/json-schema-core.html#anchor8 > Note: JSON specification does not specify precision. So number could be > any number. > > JSON Schema Validation > http://json-schema.org/latest/json-schema-validation.html#anchor5 > > Those who do not care rounding errors/etc, they can use default behavior. > JSON Schema could be used to encode huge numbers correctly, so I would like to have it in the future also. -- Yasuo Ohgaki yohg...@ohgaki.net
