php / php/php-src

Block premature access to session storage

Ouverte
#21,115 7 commentaires 1 réaction 1 personne assignée Voir sur GitHub

@Girgias y travaille déjà.

Depuis le 4/2/2026.

Extension: session Feature Status: Needs Triage
Langage dominant
C
Étoiles
40.4k
Forks
8.2k
Merge moyen
2 j 15 h
PR mergées (30 j)
103

Description

Description
  • session_start() unconditionally calls SessionHandler::read() - even in cases where the session id has been newly generated just a couple of lines earlier.
  • session_write_close() unconditionally calls SessionHandler::write() even if the session id has been newly generated for the current request and the session is empty.
  • session_regenerate_id(true) unconditionally calls SessionHandler::destroy() even if the session id has been newly generated for the current request.

I think there is some room for optimization if the session module could suppress calls into the session handler in certain situations. I guess its best expressed as phpt what I'm having in mind.


Incoming requests without a session (cookie) shouldn't be accessing storage at all:

--TEST--
Request without session
--EXTENSIONS--
session
--INI--
session.use_strict_mode=1
--FILE--
<?php
error_reporting(E_ALL);
ob_start();

class handler implements SessionHandlerInterface, SessionUpdateTimestampHandlerInterface {
    function open($save_path, $session_name): bool
    {
        print "OPEN: $session_name\n";
        return true;
    }

    function close(): bool
    {
        print "CLOSE\n";
        return true;
    }

    function read($key): string|false
    {
        print "READ: $key\n";
        return "";
    }

    function write($key, $val): bool
    {
        print "WRITE: $key, $val\n";
        return true;
    }

    function destroy($key): bool
    {
        print "DESTROY: $key\n";
        return true;
    }

    function updateTimestamp(string $id, string $data): bool {
        print "UPDATE_TIMESTAMP: $id, $data\n";
        return true;
    }

    function validateId(string $id): bool {
        print "VALIDATE_ID: $id\n";
        return false;
    }

    function gc($max_lifetime): int { return 0; }
}

session_set_save_handler(new handler());

session_start();
session_write_close();
?>
--EXPECT--
OPEN: PHPSESSID
CLOSE

This is failing with the following diff:

Running selected tests.
TEST 1/1 [ext/session/tests/user_session_module/request-without-session.phpt]
========DIFF========
     OPEN: PHPSESSID
002+ READ: 78d7fe4a7170fb5a60934db9784d7d63
003+ WRITE: 78d7fe4a7170fb5a60934db9784d7d63,
     CLOSE
========DONE========

When logging in the initial empty session record shouldn't attempted to be destroyed. Also there is no point in attempting to read a session record with the newly generated id:

--TEST--
Request with login
--EXTENSIONS--
session
--INI--
session.use_strict_mode=1
--FILE--
<?php
error_reporting(E_ALL);
ob_start();

class handler implements SessionHandlerInterface, SessionUpdateTimestampHandlerInterface {
    function open($save_path, $session_name): bool
    {
        print "OPEN: $session_name\n";
        return true;
    }

    function close(): bool
    {
        print "CLOSE\n";
        return true;
    }

    function read($key): string|false
    {
        print "READ: $key\n";
        return "user_id|i:42;";
    }

    function write($key, $val): bool
    {
        print "WRITE: $key, $val\n";
        return true;
    }

    function destroy($key): bool
    {
        print "DESTROY: $key\n";
        return true;
    }

    function updateTimestamp(string $id, string $data): bool {
        print "UPDATE_TIMESTAMP: $id, $data\n";
        return true;
    }

    function validateId(string $id): bool {
        print "VALIDATE_ID: $id\n";
        return false;
    }

    function gc($max_lifetime): int { return 0; }
}

session_set_save_handler(new handler());

session_start();
session_regenerate_id(TRUE);
$_SESSION['user_id']=42;
session_write_close();
?>
--EXPECTF--
OPEN: PHPSESSID
CLOSE
OPEN: PHPSESSID
VALIDATE_ID: %s
WRITE: %s %s
CLOSE

This is failing with the following diff:

Running selected tests.
TEST 1/1 [ext/session/tests/user_session_module/request-with-login.phpt]
========DIFF========
     OPEN: PHPSESSID
002+ READ: 9f2306bd1721271b20061dfe45d044a1
003+ DESTROY: 9f2306bd1721271b20061dfe45d044a1
     CLOSE
     OPEN: PHPSESSID
     VALIDATE_ID: a7efc5fb4eca6fd6e44799a6e50e7960
007+ READ: a7efc5fb4eca6fd6e44799a6e50e7960
     WRITE: %s %s
     CLOSE
========DONE========

When logging out the resulting empty session record shouldn't attempted to be read and written:

--TEST--
Request with logout
--EXTENSIONS--
session
--INI--
session.use_strict_mode=1
--FILE--
<?php
error_reporting(E_ALL);
ob_start();

class handler implements SessionHandlerInterface, SessionUpdateTimestampHandlerInterface {
    function open($save_path, $session_name): bool
    {
        print "OPEN: $session_name\n";
        return true;
    }

    function close(): bool
    {
        print "CLOSE\n";
        return true;
    }

    function read($key): string|false
    {
        print "READ: $key\n";
        return "user_id|i:42;";
    }

    function write($key, $val): bool
    {
        print "WRITE: $key, $val\n";
        return true;
    }

    function destroy($key): bool
    {
        print "DESTROY: $key\n";
        return true;
    }

    function updateTimestamp(string $id, string $data): bool {
        print "UPDATE_TIMESTAMP: $id, $data\n";
        return true;
    }

    function validateId(string $id): bool {
        print "VALIDATE_ID: $id\n";
        return $GLOBALS['validId'];
    }

    function gc($max_lifetime): int { return 0; }
}

session_set_save_handler(new handler());

$GLOBALS['validId'] = true;
session_id(session_create_id());
session_start();
$_SESSION = [];
$GLOBALS['validId'] = false;
session_regenerate_id(TRUE);
session_write_close();
?>
--EXPECTF--
OPEN: PHPSESSID
VALIDATE_ID: %s
READ: %s
DESTROY: %s
CLOSE
OPEN: PHPSESSID
VALIDATE_ID: %s
CLOSE

This is failing with the following diff:

Running selected tests.
TEST 1/1 [ext/session/tests/user_session_module/request-with-logout.phpt]
========DIFF========
--
     CLOSE
     OPEN: PHPSESSID
     VALIDATE_ID: %s
008+ READ: 483617bb2349fe2345fcb67b391e7b1d
009+ WRITE: 483617bb2349fe2345fcb67b391e7b1d,
     CLOSE
========DONE========

Guide de contribution

Ouvrir le guide de contribution

Par où commencer

  1. Lisez l'issue en entier, puis le guide de contribution du projet.
  2. Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
  3. Forkez le dépôt et travaillez sur une branche.
  4. Ouvrez une pull request qui référence le numéro de l'issue.

Évaluation

Cette issue n'a pas encore été évaluée.

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.