[PHP-DEV] Re: PHP7 Homework for everyone reading this list

2015-01-26 Thread Arjen Schol

On 01/22/2015 06:05 PM, Rasmus Lerdorf wrote:

By installing a couple of apps (Wordpress-4.1, Drupal8 and Moodle -
there are still some issues in Moodle I haven't figured out yet) on a
box we've tracked down some bugs over the last couple of days. It would
be really useful if we got more eyes on this. Install php7, then install
any random app and see how it goes. Sometimes it is useful to flip back
and forth between 5.6 and 7 to check if a problem is php7-specific, so I
have php56-fpm sitting alongside php7-fpm and it is trivial to switch
back and forth.

Hopefully everyone here knows how to compile from git and get things up
and running. But, if someone is good with building sharable container
images of some kind, that might be a nice contribution as well. An image
with clear instructions telling people to drop their app into
/var/www/whatever and add the appropriate nginx/apache vhost block and
they are set.

-Rasmus



Failed assertion in DirectoryIterator_getExtension on '.' entry.
https://bugs.php.net/bug.php?id=68915

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



[PHP-DEV] DateTime microseconds discussion

2016-11-08 Thread Arjen Schol

Hi,

Support for microseconds was added late in the 7.1 RC cycle, however is 
has some issues:


1. There is no easy way to set microseconds to 0, you have to call 
setTime see https://3v4l.org/YUhFF A setMicroseconds method would be 
handy and/or support to relative strings to set microseconds to 0 (just 
like midnight does for H:i:s). Or am I missing something?


2. Microsecond support is useful, by not by default I think. Why not 
introduce a 3rd parameter $with_microseconds/$set_microseconds which 
defaults to false? Or A second class DateTimeWithMicroseconds? Maybe not 
so fancy naming, but it's very clear what will happen.


3. The 4th parameter to setTime() should not cause BC breakage.

This discussion was started at https://github.com/php/php-src/pull/2186

Gr Arjen

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] DateTime microseconds discussion

2016-11-08 Thread Arjen Schol

Hi Dan,

I think you make some bad assumptions here. The examples provided by 
Sjon are scripts submitted to 3v4l.org They may have bad assumptions, 
but are real life examples of DateTime usage. And they will break.


We have two issues in our codebase. We ARE testing RC release, I think 
feedback should be taken seriously.


1. We overloaded DateTime::setTime, this needed an update because of the 
extra parameter (point 3). BC break.

2. We test the following:

A. $date = new DateTime;
B. Store in database.
C. Retrieve datetime.
D. $fromDatabase = new DateTime(storedDateTime);
E. $fromDatabase == $date (which fails now, because $date has 
microsecond precision while $fromDatabase doesn't).


Where does this have a bad assumption? We tested an assumption (DateTime 
has seconds precision), and now it fails. Is that a bad assumption? I 
think it's just backward breaking with no good way to fix the code.


We could set microseconds to 0, which is cumbersome because of a missing 
setMicroseconds method. One needs to call setTime with retrieved hours, 
minutes, seconds or setTimestamp($dt->format('U'))...


"Rely only on reliable things."...

Arjen



On 11/08/2016 12:03 PM, Dan Ackroyd wrote:

On 8 November 2016 at 09:32, Arjen Schol  wrote:

Hi,

Support for microseconds was added late in the 7.1 RC cycle, however is has
some issues:



My understanding is that if this affects your code, then your code
already has a bad assumption in it. The code is mistakenly assuming
that two DateTime's generated will have the same the same time for
both of them.*

I don't fully understand what problem you are want to solve through
this discussion. No-one is forcing you to update to PHP 7.1
immediately; you should have plenty of time to fix the code that has
bad assumptions about the time in a generated DateTime before you
upgrade to PHP 7.1.

SjonHortensius wrote:

if generating a DateTime takes roughly 4 μs (which it does for me), this 
happens ~ 250.000 times
more often


Having bugs happen more frequently is a good thing. It stops you from
ignoring edge cases and programming by coincidence.**

I realise that having a release make it more obvious that broken code
is broken is an annoying thing to have to fixbut that code is
already wrong. Delaying useful features, just to avoid having to fix
flaky code is not a good way for a project to proceed, imo.

dshafik wrote:

I think default microseconds to 0 when unspecific for relative strings is 
correct behavior


I don't think that sounds like a good plan at all. When creating a
DateTime through `new DateTime('first day of next month');` only the
date values are set from the input string, the rest of the time is set
to now().

Wouldn't having microseconds behave differently to the rest of the
time values would be instantly an extra piece of inconsistency for
people to regret?

cheers
Dan


* Example code that doesn't work correctly now. If the sleep period is
reduced, it becomes less obvious that the code is broken, but it is
still broken.

for ($i=0; $i<10; $i++) {
$dt1 = new DateTime('first day of next month');
usleep(rand(0, 50));
$dt2 = new DateTime('first day of next month');

if ($dt1 == $dt2) {
echo "Same\n";
}
else {
echo "Different\n";
}
}


** Programming by Coincidence -
https://pragprog.com/the-pragmatic-programmer/extracts/coincidence




--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] DateTime microseconds discussion

2016-11-08 Thread Arjen Schol

Hi Nikita,

The https://3v4l.org/YUhFF example was to demonstrate it is NOT easy to 
set microseconds to zero. NOT to demonstrate new DateTime("5 minutes 
ago") == new DateTime("5 minutes ago").


new DateTime("5 minutes ago, 0 microseconds") indeed sets the 
microseconds to zero (nice to know) but this does not work for 'now'.


See https://3v4l.org/8VE7W for new DateTime('now, 0 microseconds');
And this code fails < 7.1RC4.

Arjen

On 11/08/2016 01:24 PM, Nikita Nefedov wrote:

Hey Arjen,

On 8 November 2016 at 09:32, Arjen Schol  wrote:

There is no easy way to set microseconds to 0, you have to call setTime


There actually is an easy way, you can pass microseconds absolute value
in the constructor as well, it would like: `new DateTime("5 minutes ago,
0 microseconds")`

As Dan said it is probably a mistkae in the code, really, and it is a
very popular one,
to think that `new DateTime("5 minutes ago") == new DateTime("5 minutes
ago")`.
If you sample it enough times even on pre-7 versions of PHP you will get
inequality as well: https://3v4l.org/JV59e (sorry for overloading 3v4l a
bit here).
It is especially an undesirable mistake because it causes failures
randomly,
no one likes their test suite failing /sometimes/, it makes debugging
a very unpleasant experience.

When doing date manipulations, let's say the task is to generate a
report for the
previous month, you need to always have a base date as a point of
reference:

```
$startDate = new DateTimeImmutable("first day of previous month,
00:00:00.0");
$endDate = $startDate->add(new DateInterval("P1M"));
```



--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] DateTime microseconds discussion

2016-11-08 Thread Arjen Schol

On 11/08/2016 02:49 PM, Derick Rethans wrote:

Hi,

On Tue, 8 Nov 2016, Arjen Schol wrote:


I think you make some bad assumptions here.


Please don't top-reply on this list.


The examples provided by Sjon are
scripts submitted to 3v4l.org They may have bad assumptions, but are real life
examples of DateTime usage. And they will break.


They already could break. As Dan wrote better:

> Having bugs happen more frequently is a good thing. It stops
> you from ignoring edge cases and programming by coincidence.


We have two issues in our codebase. We ARE testing RC release, I think
feedback should be taken seriously.


Taking things seriously does not mean having to agree.


Taking things seriously means reading the complete message and not 
focussing on a detail of an example (https://3v4l.org/YUhFF faces the 
same problem without constructor argument; e.g. not easy to set 
microseconds to 0). I've never said we were facing new DateTime == new 
DateTime problems, but Dan gives a lesson on bad assumptions...


No comment on not being able to set microtime to zero.




1. We overloaded DateTime::setTime, this needed an update because of the extra
parameter (point 3). BC break.


PHP throws a warning here, but the expected result of the code
execusion is correct:

[PHP: 7.1.0-dev  ] derick@whisky:/tmp $ cat doo.php
setTime( 2, 5, 10 );

var_dump( $a, $b );
?>

[PHP: 7.1.0-dev  ] derick@whisky:/tmp $ php -n doo.php

Warning: Declaration of MyDate::setTime($h, $i, $s) should be 
compatible with DateTimeImmutable::setTime($hour, $minute, $second = NULL, 
$microseconds = NULL) in /tmp/doo.php on line 9
object(MyDate)#1 (3) {
  ["date"]=>
  string(26) "2016-11-08 13:30:06.371428"
  ["timezone_type"]=>
  int(3)
  ["timezone"]=>
  string(3) "UTC"
}
object(MyDate)#2 (3) {
  ["date"]=>
  string(26) "2016-11-08 02:10:10.00"
  ["timezone_type"]=>
  int(3)
  ["timezone"]=>
  string(3) "UTC"
}

This is at *most* a BC break because it shows a warning. The code itself isn't 
broken.

In PHP 5.6, this was a E_STRICT warning, which got converted to E_WARNING in 
PHP 7.0:

[PHP: 5.6.28-dev  ] derick@whisky:~/dev/php/php-src.git $ php -n 
-derror_reporting=-1 -ddate.timezone=UTC /tmp/doo.php
Strict Standards: Declaration of MyDate::setTime() should be compatible 
with DateTimeImmutable::setTime($hour, $minute, $second = NULL) in /tmp/doo.php 
on line 9
…

[PHP: 7.0.13-dev  ] derick@whisky:~/dev/php/php-src.git $ php -n 
-derror_reporting=-1 -ddate.timezone=UTC /tmp/doo.php
Warning: Declaration of MyDate::setTime($h, $i, $s) should be 
compatible with DateTimeImmutable::setTime($hour, $minute, $second = NULL) in 
/tmp/doo.php on line 9
…

As this specific overloading does *not* violate the Liskov Substitution
Principle, I would argue that it is this *warning* that is the BC break in PHP
7.0, and not the addition of the extra argument-with-default-value.


I think it DOES violate (and hence the warning) LSP. If you DO use the 
4th microseconds argument in your application, you cannot replace a 
DateTime object with CustomDate which does not have this 4th argument, 
even tough DateTime has a default value for it. If you do want to save a 
time with microseconds by setTime, you end up without microseconds if 
you replace DateTime with CustomDate.


"[..] if S is a subtype of T, then objects of type T may be replaced 
with objects of type S (i.e., an object of the type T may be substituted 
with its subtype object of the type S) without altering any of the 
desirable properties of that program (correctness, task performed, etc.)"







2. We test the following:

A. $date = new DateTime;
B. Store in database.
C. Retrieve datetime.
D. $fromDatabase = new DateTime(storedDateTime);
E. $fromDatabase == $date (which fails now, because $date has microsecond
precision while $fromDatabase doesn't).


If you don't store microseconds in the database, then I expect that not
to work. It's like if you wouldn't store seconds in the database, that
you wouldn't get the same result either.


This used to work for 7 years, and now it's not. If I hadn't stored 
seconds, it wouldn't have worked for 7 years.





Where does this have a bad assumption? We tested an assumption (DateTime has
seconds precision), and now it fails. Is that a bad assumption? I think it's
just backward breaking with no good way to fix the code.


The bad assumption is, that DateTime has always had microseconds
precision, and the bug that got fixes was, that they did not always get
correctly initialised:


First, it was virtually impossible t

Re: [PHP-DEV] DateTime microseconds discussion

2016-11-08 Thread Arjen Schol

On 11/08/2016 01:46 PM, Derick Rethans wrote:

On Tue, 8 Nov 2016, Arjen Schol wrote:


Hi,

Support for microseconds was added late in the 7.1 RC cycle, however is has
some issues:


Some *additional* support for microseconds was added in the PHP 7.1
cycle, mostly to support bug fixes that have been around for a long
time.



1. There is no easy way to set microseconds to 0, you have to call setTime see
https://3v4l.org/YUhFF A setMicroseconds method would be handy and/or support
to relative strings to set microseconds to 0 (just like midnight does for
H:i:s). Or am I missing something?


- You can't set just the seconds or minute portion alone either,
  through setTime(). Microseconds are just an extension of the time
  portion.
- Using "midnight" also sets the microseconds to 0:


new DateTime('now midnight') obviously won't work...



[PHP: 7.1.0-dev  ] derick@whisky:~ $ cat /tmp/midnight.php
modify( 'midnight' ) );
?>

[PHP: 7.1.0-dev  ] derick@whisky:~ $ php -n /tmp/midnight.php
object(DateTimeImmutable)#1 (3) {
  ["date"]=>
  string(26) "2016-11-08 12:01:20.023680"
  ["timezone_type"]=>
  int(3)
  ["timezone"]=>
  string(3) "UTC"
}
object(DateTimeImmutable)#2 (3) {
  ["date"]=>
  string(26) "2016-11-08 00:00:00.00"
  ["timezone_type"]=>
  int(3)
  ["timezone"]=>
  string(3) "UTC"
}


2. Microsecond support is useful, by not by default I think. Why not
introduce a 3rd parameter $with_microseconds/$set_microseconds which
defaults to false?


A 3rd parameter to which function?


With 3rd argument I meant the constructor:
public DateTime::__construct ([ string $time = "now" [, DateTimeZone 
$timezone = NULL, [ $initialize_microseconds = false ]]] )


When microseconds are NOT specified in $time, don't set them unless 
$initialize_microseconds is true.





Or A second class DateTimeWithMicroseconds? Maybe not so fancy naming,
but it's very clear what will happen.


But that would be a duplicate. The already existing DateTime and
DateTimeImmutable classes already support microseconds. Adding an extra
class for them, would just be confusing.


3. The 4th parameter to setTime() should not cause BC breakage.


Added additional new arguments with a default value, is not a BC break.

cheers,
Derick




--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



[PHP-DEV] Re: PHP 5.5.17RC1 is ready for testing

2014-09-05 Thread Arjen Schol

Hi Julien,

I've found a regression in stream_select with TLS.
Testscript and details at https://bugs.php.net/bug.php?id=67965

Thanks,
Arjen

On 09/04/2014 12:17 PM, Julien Pauli wrote:

Hello

PHP 5.5.17 RC1 is available for testing.

You can download it from

http://downloads.php.net/jpauli/

The Windows binaries are available at http://windows.php.net/qa/

This release contains a number of bugfixes.
For the list of bugfixes that you can target in your
testing, please refer to the NEWS file:

 https://github.com/php/php-src/blob/php-5.5.17RC1/NEWS

Please test it carefully, and report any bugs in the bug system.

The stable release is planned for September 18th, if no critical issues will
be discovered in the RC.

Thank you for your support.

Julien Pauli & David Soria Parra




--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



[PHP-DEV] Re: PHP 5.5.17RC1 is ready for testing

2014-09-18 Thread Arjen Schol
What's the benefit of doing a Release Candidate/QA cycle when fixes for 
regressions aren't merged to the final release?


See https://bugs.php.net/bug.php?id=67965
Regression reported and fixed, but fix not merged to 5.5.17 branch.

Commit 
https://github.com/php/php-src/commit/f86b2193a483f56b0bd056570a0cdb57ebe66e2f?diff=unified
File in 5.5.17: 
https://github.com/php/php-src/blob/PHP-5.5.17/ext/openssl/xp_ssl.c#L884


Not critical enough? Just missed it? RC releases just for the show? :?

Arjen

On 09/05/2014 02:04 PM, Arjen Schol wrote:

Hi Julien,

I've found a regression in stream_select with TLS.
Testscript and details at https://bugs.php.net/bug.php?id=67965

Thanks,
Arjen

On 09/04/2014 12:17 PM, Julien Pauli wrote:

Hello

PHP 5.5.17 RC1 is available for testing.

You can download it from

http://downloads.php.net/jpauli/

The Windows binaries are available at http://windows.php.net/qa/

This release contains a number of bugfixes.
For the list of bugfixes that you can target in your
testing, please refer to the NEWS file:

 https://github.com/php/php-src/blob/php-5.5.17RC1/NEWS

Please test it carefully, and report any bugs in the bug system.

The stable release is planned for September 18th, if no critical
issues will
be discovered in the RC.

Thank you for your support.

Julien Pauli & David Soria Parra






--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



[PHP-DEV] Re: OpenSSL bug in 5.4.33 and 5.5.17

2014-09-22 Thread Arjen Schol
I noticed a regression in 5.5.17RC1, reported it (#67965) and it got 
fixed in f86b2193 on the 5.5 branch by Daniel Lowrey.


But this fix didn't make it in 5.5.17 final. I posted the following 
message on the 5.5.17RC1 release announcement:



What's the benefit of doing a Release Candidate/QA cycle when fixes for 
regressions aren't merged to the final release?

See https://bugs.php.net/bug.php?id=67965
Regression reported and fixed, but fix not merged to 5.5.17 branch.

Commit 
https://github.com/php/php-src/commit/f86b2193a483f56b0bd056570a0cdb57ebe66e2f?diff=unified
File in 5.5.17: 
https://github.com/php/php-src/blob/PHP-5.5.17/ext/openssl/xp_ssl.c#L884

Not critical enough? Just missed it? RC releases just for the show? :?


Who is resposible for cherrypicking fixes for regressions in release 
candidates? The release manager? The developer who commited the fix? Did 
the release manager ever considered including this fix in the final 
version? Where are regressions tracked?


The issues tracker didn't have a 'PHP5.5.15RC1' version, so bugs are 
filled against '5.5Git-2014-09-05' which makes it difficult to track 
issues specific on 5.5.17RC1. PHP 5.6.1.RC1 however, IS listed as a 
version on https://bugs.php.net/report.php


The releaseprocess RFC (https://wiki.php.net/rfc/releaseprocess) does 
not address these issues.


Arjen



On 09/19/2014 06:49 PM, Remi Collet wrote:

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Le 19/09/2014 18:25, Daniel Lowrey a écrit :

In an effort to fix a very old (seven years old) DoS
vulnerability involving encrypted streams I created a
regression where feof() notifications on encrypted sockets are
broken. This is present in both the most recent 5.4.33 and
5.5.17 releases.



Can you please point us to the related commit... (which one cause
the regression, which ones are useful)


In 5.4.33 and 5.5.17 an immediate fix is to revert these commits:

http://git.php.net/?p=php-src.git;a=commitdiff;h=6569db88081562f68a4f79e52cba83482bdf05fc


http://git.php.net/?p=php-src.git;a=commitdiff;h=372844918a318ad712e16f9ec636682424a65403


http://git.php.net/?p=php-src.git;a=commitdiff;h=32be79dcfa1bc5af8682d9f512da68c5b3e2cbf3

  The last of these (32be79d) has already been fixed upstream by
f86b2193a483f56b0bd056570a0cdb57ebe66e2f but this change did not go
into 5.4.33 and 5.5.17. Any reverts should also consider f86b2193.


Does a revert of the first enough to get back to previous
behavior?


Yes, reverting the above commits above will fix any issues. I'm
awaiting word from someone associated with Horde to verify that the
previously linked patch (
https://bugs.php.net/patch-display.php?bug=41631&patch=bug41631.patch&revision=1411139621)



resolves the issue. As long as that works as expected I can merge that and

everything should be resolved going forward.



After a quick check

6569db8 and 32be79d are in 5.4.33 / 5.5.17 / 5.6.1RC1
f86b219 and 3728449 are in 5.6.1RC1 only


-BEGIN PGP SIGNATURE-
Version: GnuPG v2
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlQcXrUACgkQYUppBSnxahgfigCfUYmoYXJJYC0JKmLi/tg+mo1r
mwwAoNXbDpPsdrVfzFWUy4tuOssqR256
=OUHp
-END PGP SIGNATURE-




--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php