We need to avoid signed integer overflows which are undefined behavior.
We catch that, and set `offset` to `ZEND_LONG_MAX` (which is also the
largest value of `zend_off_t` on all platforms). Of course, that seek
may fail, but even if it succeeds, the stream is no longer readable,
but that matches the current behavior for offsets near `ZEND_LONG_MAX`.
Closes GH-15989.
This effectively inlines the behaviour of php_mkdir_ex() which is a deprecated API from at least 17 years ago, and also fixes some of the return values.
This also removes a dependency on ext/standard
We need to avoid signed integer overflows which are undefined behavior.
We catch that, and set `offset` to `ZEND_LONG_MAX` (which is also the
largest value of `zend_off_t` on all platforms). Of course, after such
a seek a stream is no longer readable, but that matches the current
behavior for offsets near `ZEND_LONG_MAX`.
Closes GH-15989.
This was first reported as a leak in GH-15026, but was mistakingly
believed to be a false positive. Then an assertion was added and it got
triggered in GH-15908. This fixes the leak. Upon merging into master the
assertion should be removed as well.
Closes GH-15924.
A common convention is to name internal C header files as `*_int.h`.
Since a couple of these are actually installed, we add comments that
this is not supposed to happen, (a) to avoid installing further
internal headers, and (b) to pave the way to fix this in the next major
PHP version.
Somewhat special is php_gmp_int.h, where "int" is meant as abbreviation
for "interface".
Another common convention is appending `_priv` or `_private`, but since
there have not been any issues regarding these headers so far, we
refrain from adding respective comments to these headers.
Anyhow, it might be a good idea to introduce some common naming
convention for such internal/private headers.
We're reasonably sure that appending the NUL is not an OOB write, since
the memory stream implementation uses `zend_string` APIs instead of
fiddling with the buffer.
We don't add a regression test because that would require to set up
something in the zend_test extension, and regressions are supposed
to be caught by external consumers of this API, such as mailparse.
Closes GH-15648.
These are either undefined or defined (to value 1):
- __DragonFly__
- __FreeBSD__
- HAS_MCAST_EXT
- HAVE_GETCWD
- HAVE_GETWD
- HAVE_GLIBC_ICONV
- HAVE_JIT
- HAVE_LCHOWN
- HAVE_NL_LANGINFO
- HAVE_RL_CALLBACK_READ_CHAR
- HAVE_RL_ON_NEW_LINE
- HAVE_SQL_EXTENDED_FETCH
- HAVE_UTIME
Follow up of GH-5526 (-Wundef)
Although the issue was demonstrated using Curl, the issue is purely in
the streams layer of PHP.
Full analysis is written in GH-11078 [1], but here is the brief version:
Here's what actually happens:
1) We're creating a FILE handle from a stream using the casting mechanism.
This will create a cookie-based FILE handle using funopen.
2) We're reading stream data using fread from the userspace stream. This will
temporarily set a buffer into a field _bf.base [2]. This buffer is now equal
to the upload buffer that Curl allocated and note that that buffer is owned
by Curl.
3) The fatal error occurs and we bail out from the fread function, notice how
the reset code is never executed and so the buffer will still point to
Curl's upload buffer instead of FILE's own buffer [3].
4) The resources are destroyed, this includes our opened stream and because the
FILE handle is cached, it gets destroyed as well.
In fact, the stream code calls through fclose on purpose in this case.
5) The fclose code frees the _bs.base buffer [4].
However, this is not the buffer that FILE owns but the one that Curl owns
because it isn't reset properly due to the bailout!
6) The objects are getting destroyed, and so the curl free logic is invoked.
When Curl tries to gracefully clean up, it tries to free the buffer.
But that buffer is actually already freed mistakingly by the C library!
This also explains why we can't reproduce it on Linux: this bizarre buffer
swapping only happens on macOS and BSD, not on Linux.
To solve this, we switch to an unbuffered mode for cookie-based FILEs.
This avoids any stateful problems related to buffers especially when the
bailout mechanism triggers. As streams have their own buffering
mechanism, I don't expect this to impact performance.
[1] https://github.com/php/php-src/issues/11078#issuecomment-2155616843
[2] https://github.com/apple-open-source-mirror/Libc/blob/5e566be7a7047360adfb35ffc44c6a019a854bea/stdio/FreeBSD/fread.c#L102-L103
[3] https://github.com/apple-open-source-mirror/Libc/blob/5e566be7a7047360adfb35ffc44c6a019a854bea/stdio/FreeBSD/fread.c#L117
[4] https://github.com/apple-open-source-mirror/Libc/blob/5e566be7a7047360adfb35ffc44c6a019a854bea/stdio/FreeBSD/fclose.c#L66-L67
Closes GH-14524.
These are either undefined or defined to value 1 in Autotools and
Windows:
- HAVE_COMMONCRYPTO_COMMONRANDOM_H
- HAVE_EXIF
- HAVE_FOPENCOOKIE
- HAVE_IF_NAMETOINDEX
- HAVE_LIBICONV
- HAVE_SOCKETS
- HAVE_STRUCT_STAT_ST_RDEV
- HAVE_STRUCT_TM_TM_GMTOFF
- HAVE_STRUCT_TM_TM_ZONE
Follow up of GH-5526 (-Wundef)
* Mark many functions as static
Multiple functions are missing the static qualifier.
* remove unused struct sigactions
struct sigaction act, old_term, old_quit, old_int;
all unused.
* optimizer: minXOR and maxXOR are unused
This call is only necessary if ret < 0.
Note that I also had to reoder the checks for EWOULDBLOCK, EMSGSIZE, EAGAIN
to avoid a false positive GCC warning about a duplicate condition
(EAGAIN == EWOULDBLOCK on my system).
Commit 5cbe5a538c disabled chunking for all writes to streams. However,
user streams have a callback where code is executed on data that is
subject to the memory limit. Therefore, when using large writes or
stream_copy_to_stream/copy the memory limit can easily be hit with large
enough data.
To solve this, we reintroduce chunking for userspace streams.
Users have control over the chunk size, which is neat because
they can improve the performance by setting the chunk size if
that turns out to be a bottleneck.
In an ideal world, we add an option so we can "ask" the stream whether
it "prefers" chunked writes, similar to how we have
php_stream_mmap_supported & friends. However, that cannot be done on
stable branches.
Closes GH-13136.
Christoph M. Becker