From 5c7cb4f3ee202bceb5efd17e7b2a95cc7218d8df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1t=C3=A9=20Kocsis?= Date: Tue, 18 May 2021 11:52:55 +0200 Subject: [PATCH 1/5] Fix return types in ext/session --- ext/session/session.stub.php | 8 ++++---- ext/session/session_arginfo.h | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ext/session/session.stub.php b/ext/session/session.stub.php index a824d6b3679ee..86e409e496835 100644 --- a/ext/session/session.stub.php +++ b/ext/session/session.stub.php @@ -80,13 +80,13 @@ public function write(string $id, string $data); /** @return bool */ public function destroy(string $id); - /** @return int|bool */ + /** @return int|false */ public function gc(int $max_lifetime); } interface SessionIdInterface { - /** @return string */ + /** @return string|false */ public function create_sid(); } @@ -116,9 +116,9 @@ public function write(string $id, string $data) {} /** @return bool */ public function destroy(string $id) {} - /** @return int|bool */ + /** @return int|false */ public function gc(int $max_lifetime) {} - /** @return string */ + /** @return string|false */ public function create_sid() {} } diff --git a/ext/session/session_arginfo.h b/ext/session/session_arginfo.h index 5f1c905f60fb9..29f62c4768969 100644 --- a/ext/session/session_arginfo.h +++ b/ext/session/session_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 88f428841bc3e12b949a3308d60eae80d87e563a */ + * Stub hash: 22a92834a572985faec8a8eedcd1cdd3e81d02f5 */ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_session_name, 0, 0, MAY_BE_STRING|MAY_BE_FALSE) ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, name, IS_STRING, 1, "null") From 9a9ea22357552f09f9f48a6a063558297d021de2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1t=C3=A9=20Kocsis?= Date: Tue, 18 May 2021 11:54:20 +0200 Subject: [PATCH 2/5] Fix session cleanup --- .../tests/session_set_save_handler_closures.phpt | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/ext/session/tests/session_set_save_handler_closures.phpt b/ext/session/tests/session_set_save_handler_closures.phpt index 535850ac3749a..adf49aa455a05 100644 --- a/ext/session/tests/session_set_save_handler_closures.phpt +++ b/ext/session/tests/session_set_save_handler_closures.phpt @@ -19,7 +19,8 @@ var_dump(session_module_name(FALSE)); var_dump(session_module_name("blah")); var_dump(session_module_name("foo")); -$path = __DIR__; +$path = __DIR__ . '/session_set_save_handler_closures'; +@mkdir($path); session_save_path($path); session_set_save_handler($open_closure, $close_closure, $read_closure, $write_closure, $destroy_closure, $gc_closure); @@ -41,7 +42,12 @@ $_SESSION['Bar'] = 'Foo'; var_dump($_SESSION); session_write_close(); +echo "Cleanup\n"; +session_start(); +session_destroy(); + ob_end_flush(); +@rmdir($path); ?> --EXPECTF-- *** Testing session_set_save_handler() : using closures as callbacks *** @@ -90,3 +96,8 @@ array(4) { } Write [%s,%s,Blah|s:12:"Hello World!";Foo|b:0;Guff|i:1234567890;Bar|s:3:"Foo";] Close [%s,PHPSESSID] +Cleanup +Open [%s,PHPSESSID] +Read [%s,%s] +Destroy [%s,%s] +Close [%s,PHPSESSID] From dadd47150038ed7b2fc8f13efb590ef1d7baf733 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1t=C3=A9=20Kocsis?= Date: Tue, 18 May 2021 14:09:24 +0200 Subject: [PATCH 3/5] Try to fix test expectation --- ext/session/tests/session_set_save_handler_variation4.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/session/tests/session_set_save_handler_variation4.phpt b/ext/session/tests/session_set_save_handler_variation4.phpt index e1b61f29a75c0..9f221c29337ca 100644 --- a/ext/session/tests/session_set_save_handler_variation4.phpt +++ b/ext/session/tests/session_set_save_handler_variation4.phpt @@ -48,7 +48,7 @@ ob_end_flush(); Open [%s,PHPSESSID] Read [%s,%s] GC [0] -2 deleted +1 deleted array(3) { ["Blah"]=> string(12) "Hello World!" From 0cb561351f9b6ba00a26e152734418f19341cc89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1t=C3=A9=20Kocsis?= Date: Thu, 27 May 2021 14:53:37 +0200 Subject: [PATCH 4/5] Promote "Session is not active" warning to error in case of SessionHandler::create_sid() --- ext/session/mod_user_class.c | 12 +++++++++++- ext/session/session.stub.php | 4 ++-- ext/session/session_arginfo.h | 2 +- ext/session/tests/bug67972.phpt | 11 ++++++++--- 4 files changed, 22 insertions(+), 7 deletions(-) diff --git a/ext/session/mod_user_class.c b/ext/session/mod_user_class.c index fb9e937dda660..72968f27e9032 100644 --- a/ext/session/mod_user_class.c +++ b/ext/session/mod_user_class.c @@ -27,6 +27,16 @@ RETURN_THROWS(); \ } +#define PS_SANITY_CHECK_THROW \ + if (PS(session_status) != php_session_active) { \ + zend_throw_error(NULL, "Session is not active"); \ + RETURN_THROWS(); \ + } \ + if (PS(default_mod) == NULL) { \ + zend_throw_error(NULL, "Cannot call default session handler"); \ + RETURN_THROWS(); \ + } + #define PS_SANITY_CHECK_IS_OPEN \ PS_SANITY_CHECK; \ if (!PS(mod_user_is_open)) { \ @@ -162,7 +172,7 @@ PHP_METHOD(SessionHandler, create_sid) RETURN_THROWS(); } - PS_SANITY_CHECK; + PS_SANITY_CHECK_THROW; id = PS(default_mod)->s_create_sid(&PS(mod_data)); diff --git a/ext/session/session.stub.php b/ext/session/session.stub.php index 86e409e496835..0300fec6566a1 100644 --- a/ext/session/session.stub.php +++ b/ext/session/session.stub.php @@ -86,7 +86,7 @@ public function gc(int $max_lifetime); interface SessionIdInterface { - /** @return string|false */ + /** @return string */ public function create_sid(); } @@ -119,6 +119,6 @@ public function destroy(string $id) {} /** @return int|false */ public function gc(int $max_lifetime) {} - /** @return string|false */ + /** @return string */ public function create_sid() {} } diff --git a/ext/session/session_arginfo.h b/ext/session/session_arginfo.h index 29f62c4768969..db5e56602e75e 100644 --- a/ext/session/session_arginfo.h +++ b/ext/session/session_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 22a92834a572985faec8a8eedcd1cdd3e81d02f5 */ + * Stub hash: 4221e895bdb0c3e903b7688f79e2863fc0788cee */ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_session_name, 0, 0, MAY_BE_STRING|MAY_BE_FALSE) ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, name, IS_STRING, 1, "null") diff --git a/ext/session/tests/bug67972.phpt b/ext/session/tests/bug67972.phpt index 64999b0087693..9ddfd148201f5 100644 --- a/ext/session/tests/bug67972.phpt +++ b/ext/session/tests/bug67972.phpt @@ -5,7 +5,12 @@ Bug #67972: SessionHandler Invalid memory read create_sid() --FILE-- create_sid(); +try { + (new SessionHandler)->create_sid(); +} catch (Error $exception) { + echo $exception->getMessage() . "\n"; +} + ?> ---EXPECTF-- -Warning: SessionHandler::create_sid(): Session is not active in %s on line %d +--EXPECT-- +Session is not active From ca5d168a0dd02e751d5679ebef49a41c37a6b3d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1t=C3=A9=20Kocsis?= Date: Thu, 27 May 2021 15:06:52 +0200 Subject: [PATCH 5/5] Promote more warnings to exception --- ext/session/mod_user_class.c | 12 +----- ext/session/tests/bug55688.phpt | 11 ++++-- ext/session/tests/bug69111.phpt | 30 +++++++++----- .../tests/sessionhandler_open_001.phpt | 39 +++++++++++++------ 4 files changed, 58 insertions(+), 34 deletions(-) diff --git a/ext/session/mod_user_class.c b/ext/session/mod_user_class.c index 72968f27e9032..b7ea926890400 100644 --- a/ext/session/mod_user_class.c +++ b/ext/session/mod_user_class.c @@ -18,16 +18,6 @@ #include "php_session.h" #define PS_SANITY_CHECK \ - if (PS(session_status) != php_session_active) { \ - php_error_docref(NULL, E_WARNING, "Session is not active"); \ - RETURN_FALSE; \ - } \ - if (PS(default_mod) == NULL) { \ - zend_throw_error(NULL, "Cannot call default session handler"); \ - RETURN_THROWS(); \ - } - -#define PS_SANITY_CHECK_THROW \ if (PS(session_status) != php_session_active) { \ zend_throw_error(NULL, "Session is not active"); \ RETURN_THROWS(); \ @@ -172,7 +162,7 @@ PHP_METHOD(SessionHandler, create_sid) RETURN_THROWS(); } - PS_SANITY_CHECK_THROW; + PS_SANITY_CHECK; id = PS(default_mod)->s_create_sid(&PS(mod_data)); diff --git a/ext/session/tests/bug55688.phpt b/ext/session/tests/bug55688.phpt index b073dc3c5c64d..dda9ccb0c0267 100644 --- a/ext/session/tests/bug55688.phpt +++ b/ext/session/tests/bug55688.phpt @@ -9,7 +9,12 @@ session.save_handler=files gc(1); + +try { + $x->gc(1); +} catch (Error $exception) { + echo $exception->getMessage() . "\n"; +} ?> ---EXPECTF-- -Warning: SessionHandler::gc(): Session is not active in %s on line %d +--EXPECT-- +Session is not active diff --git a/ext/session/tests/bug69111.phpt b/ext/session/tests/bug69111.phpt index c7a6cada65199..8fa16357723b8 100644 --- a/ext/session/tests/bug69111.phpt +++ b/ext/session/tests/bug69111.phpt @@ -12,14 +12,26 @@ $sessionName = ini_get('session.name'); // session_start(); // Uncommenting this makes it not crash when reading the session (see below), but it will not return any data. -$sh->open($savePath, $sessionName); -$sh->write("foo", "bar"); -var_dump($sh->read(@$id)); -?> ---EXPECTF-- -Warning: SessionHandler::open(): Session is not active in %s on line 10 +try { + $sh->open($savePath, $sessionName); +} catch (Error $exception) { + echo $exception->getMessage() . "\n"; +} + +try { + $sh->write("foo", "bar"); +} catch (Error $exception) { + echo $exception->getMessage() . "\n"; +} -Warning: SessionHandler::write(): Session is not active in %s on line 11 +try { + $sh->read(@$id); +} catch (Error $exception) { + echo $exception->getMessage() . "\n"; +} -Warning: SessionHandler::read(): Session is not active in %s on line 12 -bool(false) +?> +--EXPECT-- +Session is not active +Session is not active +Session is not active diff --git a/ext/session/tests/sessionhandler_open_001.phpt b/ext/session/tests/sessionhandler_open_001.phpt index 2b8c3a49fba3a..2ec7b8f4ca393 100644 --- a/ext/session/tests/sessionhandler_open_001.phpt +++ b/ext/session/tests/sessionhandler_open_001.phpt @@ -7,20 +7,37 @@ Testing repated SessionHandler::open() calls ini_set('session.save_handler', 'files'); $x = new SessionHandler; -$x->open('',''); -$x->open('',''); -$x->open('',''); -$x->open('',''); -print "Done!\n"; +try { + $x->open('',''); +} catch (Error $exception) { + echo $exception->getMessage() . "\n"; +} -?> ---EXPECTF-- -Warning: SessionHandler::open(): Session is not active in %s on line 5 +try { + $x->open('',''); +} catch (Error $exception) { + echo $exception->getMessage() . "\n"; +} -Warning: SessionHandler::open(): Session is not active in %s on line 6 +try { + $x->open('',''); +} catch (Error $exception) { + echo $exception->getMessage() . "\n"; +} -Warning: SessionHandler::open(): Session is not active in %s on line 7 +try { + $x->open('',''); +} catch (Error $exception) { + echo $exception->getMessage() . "\n"; +} -Warning: SessionHandler::open(): Session is not active in %s on line 8 +print "Done!\n"; + +?> +--EXPECTF-- +Session is not active +Session is not active +Session is not active +Session is not active Done!