[PHP-DEV] PHP 7.0 and openssl 1.1

[Rasmus Lerdorf]

Jakub, what do you think about back-porting the openssl-1.1 supporting
changes to the PHP-7.0 branch? I think it is too early to have PHP-7.0 not
compile on new Linux versions and right now it doesn't compile on any Linux
that has openssl-1.1.

-Rasmus

Re: [PHP-DEV] pdo_sqlite fd leak

[Rasmus Lerdorf]

On Sat, Jan 21, 2017 at 4:47 PM, Christoph M. Becker 
wrote:
>
> Anyhow, the SQLite3 documentation states[1]:
>
> | The sqlite3_prepare_v2() and sqlite3_prepare16_v2() interfaces are
> | recommended for all new programs. The two older interfaces are
> | retained for backwards compatibility, but their use is discouraged.
>
> Isn't that reason enough to switch to sqlite3_prepare_v2() ASAP?  Note
> that this is documented at least for more than nine years[2]!
>
> [1] 
> [2]
>  sqlite.org/c3ref/prepare.html>


Yes, but it also says that the behaviour is slightly different on an error
condition which could potentially affect peoples' code. Although it seems
like a subtle difference and only in the case of an error, so it should be
ok to change for 7.2.

-Rasmus

Re: [PHP-DEV] pdo_sqlite fd leak

[Christoph M. Becker]

On 20.01.2017 at 22:41, Rasmus Lerdorf wrote:

> On Fri, Jan 20, 2017 at 1:08 PM, Rasmus Lerdorf  wrote:
> 
>> On Fri, Jan 20, 2017 at 12:58 Nikita Popov  wrote:
>>
>>> That sounds like it could be the source of the issue.
>>
>> Ah, that makes more sense than it never hitting that close call because I
>> couldn't find any scenario where we wouldn't get there eventually. So it
>> sounds like we should be calling sqlite3_close_v2() there instead.
>
> Of course, something must be causing the unfinalized prepared statement in
> the first place so moving to the v2 close likely wouldn't fix it, just move
> it from an unclosed db handle to an unclosed "unusable zombie" handle,
> whatever that means. I also noticed that ext/sqlite3 uses
> sqlite3_prepare_v2() while pdo_sqlite uses sqlite3_prepare(). The
> differences in those two don't seem like they would affect whether the
> prepare is finalized or not though. 

Anyhow, the SQLite3 documentation states[1]:

| The sqlite3_prepare_v2() and sqlite3_prepare16_v2() interfaces are
| recommended for all new programs. The two older interfaces are
| retained for backwards compatibility, but their use is discouraged.

Isn't that reason enough to switch to sqlite3_prepare_v2() ASAP?  Note
that this is documented at least for more than nine years[2]!

[1] 
[2]


-- 
Christoph M. Becker

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

[PHP-DEV] imap_fetchbody() leaks memory

[Leon Sorokin]

Hey guys,

I filed bug 73493 [1] a couple months ago and there's no triage or 
visible activity yet. Was hoping to get an update if possible.


I'm forced to download any large INBOX in batches with imap_close() & 
imap_open() gymnastics to avoid chewing through 2GB+ of RAM.


thanks!

[1] https://bugs.php.net/bug.php?id=73493

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

Re: [PHP-DEV] imagepolygon() – number of points

[Rowan Collins]

On 21/01/2017 17:42, Christoph M. Becker wrote:

In the long run (i.e. for PHP 8) I'd suggest to purge the $num_points
parameter altogether, relying solely on the number of $points given.


Since there's a 4th parameter ($color), what would "purge" mean in this 
case? Reducing the number of parameters to 3 would be very confusing, 
because existing code passing $num_points would be interpreted as 
passing that value as $color.


We could ignore the argument whatever value was provided; or we could 
document that it should be passed as null, and raise a warning if it's 
anything else; or we could ignore it if it matched the length of 
$points, and raise a warning or error if it didn't...


Regards,

--
Rowan Collins
[IMSoP]


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

[PHP-DEV] imagepolygon() – number of points

[Christoph M. Becker]

Hi!

This issue has been filed to our bugtracker years ago[1], but didn't
deserve much attention, so I'm forwarding to the list.

imagepolygon(), imagefilledpolygon() and imageopenpolygon() have the
following signature:

  image*polygon(resource $im, array $points, int $num_points, int $col)

Note the third parameter, $num_points, which doesn't make much sense for
PHP, and has most likely being directly ported from the underlying libgd
implementation of gdImage*Polygon[2].

Even worse, while the common implementation[3] checks that $points
(which would better have been called $coordinate, by the way) has at
least 6 elements, $num_points can be smaller than 3, what will cause
gdImage*Polygon() to be called with only so many points.  That is not an
issue per se, as gdImage*Polygon() is capable of handling monogons and
digons, but of course it doesn't make sense to force the programmer to
construct a larger array with unused values to do so.  Furthermore the
imagefilledpolygon() man page[4] states that $num_points must be greater
than or equal to 3, but neither of the other man pages document this.

In the long run (i.e. for PHP 8) I'd suggest to purge the $num_points
parameter altogether, relying solely on the number of $points given.

For PHP 7.2 I propose to weaken the constraint on count($points) so that
2 would be sufficient instead of the 6 required now.

Would the proposed change for PHP 7.2 require an RFC, or would a PR suffice?

[1] 
[2] 
[3] 
[4] 

-- 
Christoph M. Becker

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

Re: [PHP-DEV] Not autoloading functions

[Marcio Almada]

Hi, Rasmus!

2017-01-21 4:14 GMT-04:00 Rasmus Schultz :

> > How hard is it to write Foo::bar? You never have to go more
> than one level. I don't see a point in mixing internal function
> namespace with class methods for the sake of saving typing couple of
> characters.
>
> I'm not suggesting we mix namespaces - this of course would be file-local,
> same as use-statements in general.
>
> You likely have to write a use-statement either way, if you're going to
> call a static function, e.g.:
>
> use Foo\Bar\HtmlHelper;
> echo HtmlHelper::escape($text);
>
> Versus:
>
> use Foo\Bar\HtmlHelper::escape;
> echo escape($text);
>
> It's not about "saving characters", that's not what use-statements are for
> - it's to avoid qualifying the same references repeatedly, which (for one)
> is better for source-control, whether that's a namespace or a class-name
> being repeated.
>
> Anyhow, it sounds like most of you are positive about this idea, so I will
> ponder the details and post a small RFC :-)
>
>
> On Jan 20, 2017 19:55, "Stanislav Malyshev"  wrote:
>
> Hi!
>
> > Since the autoloading functions proposal is stalled, how about allowing
> for
> > import of static functions instead?
> >
> > use function Foo::bar;
> >
> > bar(); // calls Foo::bar()
>
> I'm not sure why it is good. This would certainly be confusing, if you
> call strlen and turns out it's completely different function from what
> you thought. One thing when it's the same namespace, at least you can be
> aware what this package does, but if it's just an arbitrary function
> from anywhere, it's really bad for understanding the code.
>
> How hard is it to write Foo::bar? You never have to go more
> than one level. I don't see a point in mixing internal function
> namespace with class methods for the sake of saving typing couple of
> characters.
>
> --
> Stas Malyshev
> smalys...@gmail.com
>

I think this idea looks like the easiest way to avoid fixing the underlying
problem. But it would irrevocably push
functions to a "second class citizenship" of the language, in favor of
static classes. It also would blurry
the lines between namespaces and classes in a way we haven't seen before.

Even though it feels pragmatic to do so, I'd prefer the harder path or at
least wait for the next major until commit to this.

Thanks,
Márcio.

Re: [PHP-DEV] Use decent entropy for uniqid($prefix, TRUE)

[Niklas Keller]

>
> I really don't see any pros for caring about failing CSPRNG and fallback
> to weak behavior.
>
> 1) BC is extremely unlikely. Basically, no BC on healthy hardware/OS.
> 2) Then things failed, programs should fail properly. i.e. Shouldn't
> fallback to weaker/problematic code.
>

Failing closed on a missing CSPRNG isn't really important for uniqid().
There's no guarantee that uniqid() produces ungessable output. It tries to
guarantee uniqueness and fails at the single one job it has for distributed
systems. I still think it's better to just leave it as is and deprecate it,
maybe while moving a UUID ext to core.

Regards, Niklas


> Broken CSPRNG is like BUS error, i.e. hardware error, why should we care
> so much about it?
>
> Regards,
>
> --
> Yasuo Ohgaki
> yohg...@ohgaki.net
>
>

Re: [PHP-DEV] Re: Improving mt_rand() seed

[Niklas Keller]

>
> > Anyway, unusable CSPRNG is very unlikely to happen. I may just use
> > UNEXPECTED macro for the if branch.
> >
>
> I changed my mind due to comment for uniqid() CSPRNG usage.
>
> IMO, there is no benefit for CSPRNG failure fallback. We shouldn't add
> fackback for every CSPRNG usage.


Right, we absolutely should not. Most usages of a CSPRNG require a CSPRNG,
while mt_rand and uniqid do not, so it's a different case here.


> It's just does not make sense. Are we
> going to add poor fallbacks for every CSPRNG usage? I strongly against it.
>
> CSPRNG failure is like BUS error, i.e. hardware error. CSPRNG shouldn't
> fail with healthy hardware/OS. Therefore, we should not add poor fallback
> code for it.


A CSPRNG is not necessarily a hardware error. PHP might run on weird
platforms, no?

Anyway, the "issue" with mt_rand is not the seed being predictable but the
internal state being recoverable from the output. But mt_rand is
predictable by design, so why should we even seed it with a CSPRNG by
default?

Regards, Niklas

[PHP-DEV] Re: Improving mt_rand() seed

[Christoph M. Becker]

On 16.01.2017 at 08:04, Yasuo Ohgaki wrote:

> Since I was about to improve uniqid()'s entropy by replacing
> php_combined_lcg() to php_random_int(), I spent time to check other places
> that could be a problem.
> 
> mt_rand()'s is seeded as follows by default.
> 
> ext/standard/php_rand.h
> #ifdef PHP_WIN32
> #define GENERATE_SEED() (((zend_long) (time(0) * GetCurrentProcessId())) ^
> ((zend_long) (100.0 * php_combined_lcg(
> #else
> #define GENERATE_SEED() (((zend_long) (time(0) * getpid())) ^ ((zend_long)
> (100.0 * php_combined_lcg(
> #endif
> 
> We know this kind of seed is guessable. 

But where's the problem?  mt_rand() is not suitable for cryptographic
purposes anyway.

> i.e. Our session id is compromised
> by this kind of code.

Does the session ID rely on mt_rand() or GENERATE_SEED()?  If so, that
would of course be an issue, but that should be fixed by not using
mt_rand()/GENERATE_SEED() for the session ID at all, IMHO.

-- 
Christoph M. Becker

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

Re: [PHP-DEV] Not autoloading functions

[Rasmus Schultz]

> How hard is it to write Foo::bar? You never have to go more
than one level. I don't see a point in mixing internal function
namespace with class methods for the sake of saving typing couple of
characters.

I'm not suggesting we mix namespaces - this of course would be file-local,
same as use-statements in general.

You likely have to write a use-statement either way, if you're going to
call a static function, e.g.:

use Foo\Bar\HtmlHelper;
echo HtmlHelper::escape($text);

Versus:

use Foo\Bar\HtmlHelper::escape;
echo escape($text);

It's not about "saving characters", that's not what use-statements are for
- it's to avoid qualifying the same references repeatedly, which (for one)
is better for source-control, whether that's a namespace or a class-name
being repeated.

Anyhow, it sounds like most of you are positive about this idea, so I will
ponder the details and post a small RFC :-)


On Jan 20, 2017 19:55, "Stanislav Malyshev"  wrote:

Hi!

> Since the autoloading functions proposal is stalled, how about allowing
for
> import of static functions instead?
>
> use function Foo::bar;
>
> bar(); // calls Foo::bar()

I'm not sure why it is good. This would certainly be confusing, if you
call strlen and turns out it's completely different function from what
you thought. One thing when it's the same namespace, at least you can be
aware what this package does, but if it's just an arbitrary function
from anywhere, it's really bad for understanding the code.

How hard is it to write Foo::bar? You never have to go more
than one level. I don't see a point in mixing internal function
namespace with class methods for the sake of saving typing couple of
characters.

--
Stas Malyshev
smalys...@gmail.com