Two issues:
1) The check happened before ZPP checks
2) The `return;` statement caused NULL to be returned while this
function can only return booleans. An exception seems not acceptable
in stable versions, but a warning may do.
Closes GH-16386.
Using `php_error_docref()` is preferable since it outputs additional
details (which function has been called and whether it is a startup or
shutdown error), uses HTML markup, and also provides a link to the
documentation, if configured.
Since these deprecation warnings have been introduced recently[1][2],
i.e. for PHP 8.4, there are no BC concerns.
[1] <e8ff7c70f9>
[2] <b36eac94d2>
Co-authored-by: Máté Kocsis <kocsismate90@gmail.com>
This partially reverts 0956267c08, which
introduced a type incompatibility where an `int` function is assigned
to a `zend_result` function. That yields a level 1 C4133 warning on
MSVC, and usually (e.g. in CI) level 1 warnings are elevated to errors,
so the build fails.[1]
The PHP-8.3 branch and up are uneffected by this, so the upward merges
should be empty.
[1] <0956267c08 (r144587696)>
This fixes -Winline errors where the functions are not ever inlined.
Also fixes some signature mismatches which were fixed previously but
for whatever reason were not ported to all maintained branches:
/usr/local/src/php/ext/session/session.c:1299:20:
warning:conflicting types for 'php_session_send_cookie' due to enum/integer mismatch;
have 'zend_result(void)' {aka 'ZEND_RESULT_CODE(void)'} [-Wenum-int-mismatch]
1299 | static zend_result php_session_send_cookie(void) /* {{{ */
| ^~~~~~~~~~~~~~~~~~~~~~~
/usr/local/src/php/ext/session/session.c💯12:
note: previous declaration of 'php_session_send_cookie' with type 'int(void)'
100 | static int php_session_send_cookie(void);
| ^~~~~~~~~~~~~~~~~~~~~~~
* Include from build dir first
This fixes out of tree builds by ensuring that configure artifacts are included
from the build dir.
Before, out of tree builds would preferably include files from the src dir, as
the include path was defined as follows (ignoring includes from ext/ and sapi/) :
-I$(top_builddir)/main
-I$(top_srcdir)
-I$(top_builddir)/TSRM
-I$(top_builddir)/Zend
-I$(top_srcdir)/main
-I$(top_srcdir)/Zend
-I$(top_srcdir)/TSRM
-I$(top_builddir)/
As a result, an out of tree build would include configure artifacts such as
`main/php_config.h` from the src dir.
After this change, the include path is defined as follows:
-I$(top_builddir)/main
-I$(top_builddir)
-I$(top_srcdir)/main
-I$(top_srcdir)
-I$(top_builddir)/TSRM
-I$(top_builddir)/Zend
-I$(top_srcdir)/Zend
-I$(top_srcdir)/TSRM
* Fix extension include path for out of tree builds
* Include config.h with the brackets form
`#include "config.h"` searches in the directory containing the including-file
before any other include path. This can include the wrong config.h when building
out of tree and a config.h exists in the source tree.
Using `#include <config.h>` uses exclusively the include path, and gives
priority to the build dir.
The spl dependency is configured so the spl_autoload_register is
available when session_start() is used, meaning the spl extension needs
to be loaded before the session. It is marked as optional to be more
explicit as spl is not directly used nor required in the session
extension.
The session extension once depended on the hash extension for having
hash functions available when using the `session.hash_function` INI
directive. This directive was removed in PHP-7.1 via
3467526a65. At the time it could be marked
as optional dependency, because it only needed to be loaded before the
session in that case.
The removed ext/hash/php_hash.h in the ext/session/php_session.h might
cause BC break for PHP extensions if they rely on transitive include and
use hash extension in the code without explicitly including the
ext/hash/php_hash.h header. Solution is to include the
ext/hash/php_hash.h separately.
This adds an optional dependency on the session extension and adds the
necessary APIs to make the functionality work with lazy binding.
This can be tested by configuring PHP with `--enable-session=shared` and
`--enable-soap=shared` and running the test suite, in particular the
buggy behaviour can be observed by the existing test `server009.phpt`.
It's illegal to return from a bailout because that doesn't restore the
original bailout data. Return outside of it.
Test by YuanchengJiang
Closes GH-13689.
The CombinedLCG is a terrible RNG with a questionable API and should ideally
not be used anymore. While in the case of ext/session it is only used for
probabilistic garbage collection where the quality of the RNG is not of
particular importance, there are better choices.
Replace the RNG used for garbage collection by an ext/session specific instance
of PcgOneseq128XslRr64. Its 16 Byte state nicely fits into the memory freed up
by the previous reordering of the session globals struct, even allowing to the
storage of the php_random_algo_with_state struct, making using the RNG a little
nicer.
Instead multiplying the float returned by the CombinedLCG by the GC Divisor to
obtain an integer between 0 and the divisor we can just use `php_random_range`
to directly generate an appropriate integer, completely avoiding the floating
point maths, making it easier to verify the code for correctness.
This allows consumers of just the CSPRNG to include a much smaller header. It
also allows to verify at a glance whether a source file might use non-secure
randomness.
This commit includes the new header wherever the CSPRNG is used, possibly
replacing the inclusion of php_random.h if nothing else is used, but also
includes it in the main php_random.h header for compatibility.
Somewhat related to 45f8cfaf10,
2b30f18708, and
b14dd85dca.
In an MPM worker scenario we have 1 module, N threads. Each thread must
have their globals initialised. If we only initialise the filename
fields in MINIT, then the threads have an uninitialized value. If the
uninitialized value is not NULL, this leads to segfaults upon access.
Closes GH-11530.
* Note where a session was already started
Duplicated session starts can be annoying to debug. The error that
occurs when a session is already active doesn't tell you where it
was initialized, so figuring out the callsite involves manual
debugging to find it out.
This keeps track of the call site of session_start as a request
global, and frees at the end of the request. It should make it
easier to find these instances for PHP users.
The resulting message can look like:
Notice: session_start(): Ignoring session_start() because a session is already active (started from /home/calvin/src/php-src/inc.php on line 4) in /home/calvin/src/php-src/index.php on line 9
Fixes GH-10721
* Convert to using zend_string for session start location
* Fix leak with session start callsite filename
If this was already initialized, we'd forget it. Have shared free
between session_start and RSHUTDOWN.
* For sessions that are automatically started, note that
Easy to forget that you have this set, in which case, session start
is done at RINIT outside of user code. Because this config option
can't change at runtime, we can check for it and make the error
more specific if that's the case.
* sid can never be NULL because it was NULL-checked earlier
* Change namelen to size_t because it is always unsigned and less in size than size_t
* Remove redundant check on ser
It can't be NULL, and even if it could, the ser++ would be UB.
This was introduced in 3467526a65 and the
corresponding RFC gives some reasoning. However the CSPRNG being “not secure
enough” is not a thing and reading these extra bytes is just security theater:
If the CSPRNG would hypothetically be broken, then PHP’s session IDs are the
least of one’s concerns, because we already trust it in `random_bytes()` and
might generate long-term secrets using that.
