The lack of such a check leads to false-passes of tests on Windows
which expect no output, but produce a segfault or similar issue. I
discovered this a while ago due to bad tests in an extension I maintain.
Dylan K. Taylor [Mon, 22 Feb 2021 23:56:11 +0000 (23:56 +0000)]
run-tests: fixed exit code not being set on BORKED tests
When no test paths are specified this shows up when 'make test' is used on a PECL extension without specifying tests to run (or in php-src too, I guess...)
Fix #75776: Flushing streams with compression filter is broken
First, the `bzip2.compress` filter has the same issue as `zlib.deflate`
so we port the respective fix[1] to ext/bz2.
Second, there is still an issue, if a stream with an attached
compression filter is flushed before it is closed, without any writes
in between. In that case, the compression is never finalized. We fix
this by enforcing a `_php_stream_flush()` with the `closing` flag set
in `_php_stream_free()`, whenever a write filter is attached. This
call is superfluous for most write filters, but does not hurt, even
when it is unnecessary.
Since we do no longer URL decode cookie names[1], we must not URL
encode the session name. We need to prevent broken Set-Cookie headers,
by rejecting names which contain invalid characters.
Nikita Popov [Mon, 22 Feb 2021 08:33:23 +0000 (09:33 +0100)]
Fixed bug #80781
zend_find_array_dim_slow() may throw, make sure to handle this.
This backports the code we already use for this on PHP-8.0,
and also backports an exception check that makes this easier to
catch.
Dylan T [Tue, 16 Feb 2021 18:40:26 +0000 (18:40 +0000)]
Remove generated zend_jit_x86.c on `make clean`
Not removing this causes build failure when reconfiguring and rebuilding after a `make clean`, e.g. enabling/disabling ZTS.
This makes https://bugs.php.net/bug.php?id=80561 more bearable. Ideally it would be rebuilt automatically on configuration change if necessary, but I have no idea how to implement this.
Nikita Popov [Tue, 16 Feb 2021 14:26:31 +0000 (15:26 +0100)]
Handle incomplete result set metadata more gracefully
Rather than segfaulting because sname is missing lateron, report
a FAIL here. As this indicates a server bug, the errors is reported
as an out of band warning, rather than a client error.
Nikita Popov [Tue, 16 Feb 2021 13:20:45 +0000 (14:20 +0100)]
Mark resource-like objects as non-comparable
As these hold on to some internal resource, there can't be two
"equal" objects with different identity. Make sure the lack of
public properties doesn't result in these being treated as always
equal.
Nikita Popov [Mon, 15 Feb 2021 14:54:49 +0000 (15:54 +0100)]
Suppress OpenSSL error on missing optional config
openssl_pkey_new() fetches various options from the config file --
most of these are optional, and not specifying them is not an error
condition from the perspective of the user. Unfortunately, the
CONF_get_string() API pushes an error when accessing a key that
doesn't exist (_CONF_get_string does not, but that is presumably a
private API). This commit adds a helper php_openssl_conf_get_string()
that automatically clears the error in this case. I've found that
OpenSSL occasionally does the same thing internally:
https://github.com/openssl/openssl/blob/22040fb790c854cefb04bed98ed38ea6357daf83/apps/req.c#L515-L517
libcurl 7.75.0 finally adds support for `gophers://`, i.e. gopher over
TLS. The protocol is neither standardized, nor is the protocol
registered with IANA, but well, it is there and the test case should
cater to that.
Nikita Popov [Mon, 15 Feb 2021 13:52:38 +0000 (14:52 +0100)]
Fix symtable cache being used while cleaning symtable
We need to first clean the symtable and then check whether a cache
slot is available for it. Otherwise, it may happen that a destructor
runs while cleaning the table and uses up all the remaining slots
in the cache.
This is particularly insidious because once we overflow the cache,
the first pointer we modify is symtable_cache_ptr, making it hard
to understand what happened after the fact.
Nikita Popov [Thu, 11 Feb 2021 09:21:31 +0000 (10:21 +0100)]
Don't return null from password_get_info()
The get_info() handler should never fail, but even if it does,
we should still return a proper info array -- it doesn't make
sense that a completely incorrect hash returns an info array,
but a hash that is recognized but for which the options can't
be extracted would return null.
Nikita Popov [Wed, 10 Feb 2021 11:18:42 +0000 (12:18 +0100)]
Make finfo_open() $magic_database nullable
Empty string was interpreted as a special value here, which indicates
that the default magic database should be used. It makes more sense
to use null for this purpose.
The documentation also explicitly mentions that null can be used.
Nikita Popov [Wed, 10 Feb 2021 11:00:20 +0000 (12:00 +0100)]
Don't pass null action to __doRequest
The parameter is not nullable, so it will be interpreted as
an empty string anyway.
The entire code here is pretty confusing though, and probably
deserves a second loop. The HTTP code only send SOAPAction/action
if soapaction is non-NULL -- but it always is, because it is
accepted through a non-nullable string parameter.
Regarding the SOAPAction header, it appears that always sending
it is actually a requirement of the standard:
> An HTTP client MUST use this header field when issuing a SOAP
> HTTP Request.
Although it does make a distinction between absence of value and
an empty string:
> The header field value of empty string ("") means that the intent
> of the SOAP message is provided by the HTTP Request-URI. No value
> means that there is no indication of the intent of the message.
The empty string interpretation appears to be the desired one.
However, for the action MIME tag the SOAP 1.2 Part 2 specification
says that
> The media type specifies an optional action parameter, which can
> be used to optimize dispatch or routing, among other things.
but also
> The SOAP Action feature defines a single property, which is
> described in Table 14. The value of this property MUST be an
> absolute URI[RFC 3986] and MUST NOT be empty.
which would indicate that we should not be sending an empty
action here.
As I'm not familiar with SOAP and this is long-standing behavior,
I'm just leaving this alone for now...
Nikita Popov [Wed, 10 Feb 2021 09:50:42 +0000 (10:50 +0100)]
Accept null $location in SoapClient::__setLocation()
Currently an empty string is used to unset the location. Once
again, it makes more sense to use a null value for this purpose
(though the special behavior of empty strings is retained).
The code comment above the function also explicitly indicates
that null should be accepted, and the function does return null
rather than an empty string for the old location value (if it
is missing).
Nikita Popov [Wed, 10 Feb 2021 09:36:32 +0000 (10:36 +0100)]
Make SoapVar arguments nullable
$typeName, $typeNamespace, $nodeName and $nodeNamespace all
special-case the empty string and don't set the property entirely
in that case. It makes more sense to use null to indicate absence
here (though of course the empty string behavior is retained).
Nikita Popov [Tue, 9 Feb 2021 14:15:20 +0000 (15:15 +0100)]
Make imagegd $file parameter nullable
It is explicitly documented to be nullable, and this matches other
functions like imagepng. It is also documented to accept a stream,
which it currently does not...
Nikita Popov [Tue, 9 Feb 2021 11:55:33 +0000 (12:55 +0100)]
Make Phar $fileNotFoundScript nullable
While "" is already treated the same way as absence, null is the
logically correct default here. Making this one argument non-nullable
is particularly pecular when considering that the preceding $alias
and $index arguments are both nullable.
Nikita Popov [Tue, 9 Feb 2021 11:28:28 +0000 (12:28 +0100)]
Make createDocument() $namespace nullable
According to the DOM specification, this argument should be
nullable. It's also supposed to be a required argument, but
not changing that at this point.
Fix #80706: mail(): Headers after Bcc headers may be ignored
We need to handle the case where a CRLF after a Bcc header is not the
beginning of a folding marker, because in that case the Bcc header was
not the last "thing".
Fix #74779: x() and y() truncating floats to integers
We must not use the locale dependent `atof()`, but instead use the
(hopefully) locale independent `zend_strtod()`, when converting string
representations of floating point numbers which are sent by the server.
Nikita Popov [Thu, 4 Feb 2021 14:18:45 +0000 (15:18 +0100)]
Fix CLI server worker support
If we create separate listening sockets in each worker using
SO_REUSEADDR, then an incoming connection may be load-balanced
to a process that is already busy, either due to a long-running
request, or because it is a recursive request (in which case we
would deadlock).
Instead, only create one listening socket, and only create worker
forks afterwards. This way the incoming request will be served
by one of the workers that is currently listening for an incoming
connection.