Skip to content

Conversation

@mintopia
Copy link
Contributor

Added a warning that is raised on session write if one of the session key variables contains the pipe character - as this will cause the session data to not be saved. Tests are included to cover this in both request shutdown and explicitly session_write_close().

Copy link
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that this should not fail silently, but adding a warning in a bugfix version is probably not okay (because folks turn them into Exceptions).

@mintopia
Copy link
Contributor Author

I agree that this should not fail silently, but adding a warning in a bugfix version is probably not okay (because folks turn them into Exceptions).

I can apply this change to master and submit a PR for that if that's OK?

@Girgias Girgias changed the base branch from PHP-8.3 to master May 25, 2025 17:36
@Girgias
Copy link
Member

Girgias commented May 25, 2025

I changed the base branch of the PR to master, can you rebase the PR on top of master and force push? Otherwise, the PR looks goods to me. :)

mintopia added 3 commits May 25, 2025 19:03
Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM if CI is green, please update UPGRADING and then this should be good to merge.

@mintopia
Copy link
Contributor Author

Thanks everyone for all the help and advice on my first contribution to PHP.

@Girgias Girgias merged commit 042a975 into php:master May 26, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Using pipe character in session variable key causes session data to be removed

4 participants