DennisBirkholz
Repos
7

Userland EcryptFS library written in PHP

1
0

File IO abstraction library for PHP

0
0

Morgue: modular PHP archive file reader and writer

1
0

Replum: the PHP web widget framework

1
0

Events

issue comment
[HttpKernel] lock when writting profiles

I don't understand why there needs to be a 30+ lines change when a three line change is sufficient. Avoiding a lock if possible is always preferable over an unnecessary lock. The dump file is only ever written once and renaming a file on the same file system is atomic on all relevant file systems. This change touches two code points, which need to be kept in sync and the additional code increases the cognitive load when reasoning about this code in the future and will increase the barrier for new people to understand and contribute.

Created at 4 weeks ago
delete branch
DennisBirkholz delete branch 6.2
Created at 1 month ago

[Serializer] Fix caching context-aware encoders/decoders in ChainEncoder/ChainDecoder

[HttpFoundation] Fix tests on PHP 8.2

minor #47306 [HttpFoundation] Fix tests on PHP 8.2 (nicolas-grekas)

This PR was merged into the 4.4 branch.

Discussion

[HttpFoundation] Fix tests on PHP 8.2

| Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | -

Follows https://github.com/php/php-src/pull/9304

Commits

1c574bba7c [HttpFoundation] Fix tests on PHP 8.2

minor #47299 [Console] fix expected command name order with mixed integer and string namespaces (xabbuh)

This PR was merged into the 5.4 branch.

Discussion

[Console] fix expected command name order with mixed integer and string namespaces

| Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | | License | MIT | Doc PR |

Commits

5cd0eae5af fix expected command name order with mixed integer and string namespaces

[HttpFoundation] Fix tests on PHP 8.2 (bis)

Email image parts: regex for single closing quote

The regex for image src matches for single and double opening quotes: ([\'"])

The corresponding matching for non-closing characters is implemented for double quotes only: ([^"]+)

This change adds a non-greedy regex .+? which matches for as few characters as possbile before the "correspondingly matched opening quote" \\1 appears.

bug #47329 Email image parts: regex for single closing quote (rr-it)

This PR was merged into the 4.4 branch.

Discussion

Email image parts: regex for single closing quote

| Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | -

The regex for image src matches for single and double opening quotes: ([\'"])

The corresponding matching for non-closing characters is implemented for double quotes only: ([^"]+)

This change adds a non-greedy regex .+? which matches for as few characters as possbile before the "correspondingly matched opening quote" \\1 appears.

Commits

57c49b4e13 Email image parts: regex for single closing quote

bug #47304 [Serializer] Fix caching context-aware encoders/decoders in ChainEncoder/ChainDecoder (Guite)

This PR was merged into the 4.4 branch.

Discussion

[Serializer] Fix caching context-aware encoders/decoders in ChainEncoder/ChainDecoder

| Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #38270 | License | MIT | Doc PR | -

As proposed in https://github.com/symfony/symfony/pull/43231#issuecomment-1202259776 Will require #47150 on 6.1 until a proper solution is implemented, hopefully in 6.2.

Commits

59023805f3 [Serializer] Fix caching context-aware encoders/decoders in ChainEncoder/ChainDecoder

bug #47280: [HttpFoundation] prevent incomplete profile dumps

Created at 1 month ago
create branch
DennisBirkholz create branch 4.4
Created at 1 month ago

Do not send deleted session cookie twice in the response

bug #47273 [HttpFoundation] Do not send Set-Cookie header twice for deleted session cookie (X-Coder264)

This PR was merged into the 5.4 branch.

Discussion

[HttpFoundation] Do not send Set-Cookie header twice for deleted session cookie

| Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #47228 | License | MIT | Doc PR | -

In a user logout flow \Symfony\Component\Security\Http\EventListener\SessionLogoutListener gets triggered which invalidates the user session by calling session_regenerate_id(true) which calls \Symfony\Component\HttpFoundation\Session\Storage\Handler\AbstractSessionHandler::destroy which calls setcookie($this->sessionName, '', $params);. Calling setcookie with the second argument (the value) being an empty string signals to PHP that we want to send a Set-Cookie HTTP header to the client so that the client deletes the cookie (aka the expiry time of the cookie gets set to some time in the past) and PHP also sets the value of that cookie to deleted -> https://github.com/php/php-src/blob/php-8.1.9/ext/standard/head.c#L124

The \Symfony\Component\HttpKernel\EventListener\AbstractSessionListener class did not handle this case as \Symfony\Component\HttpKernel\EventListener\AbstractSessionListener::onKernelResponse method would call $response->headers->clearCookie() without calling the popSessionCookie method which would then add this same cookie again which would then result in two Set-Cookie headers being sent to the client for the same cookie.

Commits

b08025d2e0 Do not send deleted session cookie twice in the response

add senders to SendMessageToTransportsEvent

[Messenger] drop Redis*Proxy classes

minor #47293 [Messenger] drop Redis*Proxy classes (nicolas-grekas)

This PR was merged into the 6.2 branch.

Discussion

[Messenger] drop Redis*Proxy classes

| Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | -

We just don't need these internal classes.

Commits

d7936e58f4 [Messenger] drop Redis*Proxy classes

[Intl] Update EmojiTransliterator to translate emoji to github and slack short code

feature #47263 [Intl] Update EmojiTransliterator to translate emoji to github and slack short code (lyrixx)

This PR was merged into the 6.2 branch.

Discussion

[Intl] Update EmojiTransliterator to translate emoji to github and slack short code

| Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | yes | New feature? | yes (adding more "locale" to a new feature) | Deprecations? | no | Tickets | - | License | MIT | Doc PR |


Fabbot reports are not relevant

Commits

0a8185a014 [Intl] Update EmojiTransliterator to translate emoji to github and slack short code

add ability to mock the hrtime() function

[Intl] Fixed return type of EmojiTransliterator::create() and some others methods

minor #47297 [Intl] Fixed return type of EmojiTransliterator::create() and some others methods (lyrixx)

This PR was squashed before being merged into the 6.2 branch.

Discussion

[Intl] Fixed return type of EmojiTransliterator::create() and some others methods

| Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | https://github.com/symfony/symfony/pull/47264#issuecomment-1217929814 | License | MIT | Doc PR |

Commits

770d590f04 [Intl] Fixed return type of EmojiTransliterator::create() and some others methods

[String] Add support for emoji in AsciiSlugger

[Serializer] Add missing types to BackedEnumNormalizer

feature #47264 [String] Add support for emoji in AsciiSlugger (lyrixx)

This PR was merged into the 6.2 branch.

Discussion

[String] Add support for emoji in AsciiSlugger

| Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | | License | MIT | Doc PR |

Usage

use Symfony\Component\String\Slugger\AsciiSlugger;

$slugger = new AsciiSlugger();
$slugger = $slugger->withEmoji();

$slugger->slug('a 😺, 🐈‍⬛, and a 🦁 go to 🏞️... 😍 🎉 💛', '-', 'en');
// a-grinning-cat-black-cat-and-a-lion-go-to-national-park-smiling-face-with-heart-eyes-party-popper-yellow-heart

$slugger->slug('un 😺, 🐈‍⬛, et un 🦁 vont au 🏞️');
// un-chat-qui-sourit-chat-noir-et-un-tete-de-lion-vont-au-parc-national

Commits

ab20db7a45 [String] Add support for emoji in AsciiSlugger

fix expected command name order with mixed integer and string namespaces

minor #47298 Add missing types to BackedEnumNormalizer (nicolas-grekas)

This PR was merged into the 5.4 branch.

Discussion

Add missing types to BackedEnumNormalizer

| Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | -

Spotted while reviewing #47296 Not sure why we missed adding them in #40830 :shrug:

Commits

60d6325bca [Serializer] Add missing types to BackedEnumNormalizer

[Intl] Better DX when intl package is missing

minor #47300 [Intl] Better DX when intl package is missing (lyrixx)

This PR was merged into the 6.2 branch.

Discussion

[Intl] Better DX when intl package is missing

| Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | | License | MIT | Doc PR |

Commits

5ee9193c3c [Intl] Better DX when intl package is missing

minor #47299 [Console] fix expected command name order with mixed integer and string namespaces (xabbuh)

This PR was merged into the 5.4 branch.

Discussion

[Console] fix expected command name order with mixed integer and string namespaces

| Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | | License | MIT | Doc PR |

Commits

5cd0eae5af fix expected command name order with mixed integer and string namespaces

feature #47295 [PhpUnitBridge] add ability to mock the hrtime() function (xabbuh)

This PR was merged into the 6.2 branch.

Discussion

[PhpUnitBridge] add ability to mock the hrtime() function

| Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | | License | MIT | Doc PR |

Commits

68b428803f add ability to mock the hrtime() function

[HttpFoundation] Fix tests on PHP 8.2

Created at 1 month ago
issue comment
[HttpFoundation] bug #47280: prevent incomplete profile dumps

This should be for 4.4 as the referenced PR was merged in 4.4, right?

Right, this should be for all versions #46931 was merged into. We currently use 5.4 so I referenced only that version.

Created at 1 month ago
pull request opened
bug #47280: [HttpFoundation] prevent incomplete profile dumps

| Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #47280 | License | MIT

Write profile dump into tempfile and rename it after it is completely written to prevent loading incomplete profile dumps.

Created at 1 month ago
create branch
DennisBirkholz create branch fix-47280
Created at 1 month ago
opened issue
HttpFoundation: race condition introduced by #46931 breaks Web Profiler Bar

Symfony version(s) affected

5.4.11

Description

#46931 introduced a race condition between writing the profiler data and loading it as the browser now starts a XHR request to load the profiler data before the data is completely written.

The error produced when loading the incomplete profile dump is:

request.CRITICAL: Uncaught PHP Exception ErrorException: "Notice: unserialize(): Error at offset 3254883 of 3254908 bytes" at vendor/symfony/http-kernel/Profiler/FileProfilerStorage.php line 126 {"exception":"[object] (ErrorException(code: 0): Notice: unserialize(): Error at offset 3254883 of 3254908 bytes at vendor/symfony/http-kernel/Profiler/FileProfilerStorage.php:126)"

How to reproduce

Reproduction is difficult as it depends heavily on timing. In general, the issue may occur for very large profile dumps stored on slow disks.

Possible Solution

This race condition can be fixed in two ways:

  1. Ensure the stored profile data on disk is always complete by storing the data into a temporary file and moving that file to the final location after everything is written in FileProfilerStorage.php#L177
  2. Use the @ operator on the unserialize operation in FileProfilerStorage.php#L124

I prefer solution 1 as solution 2 would hide other sources of corruption in the stored profile. A pull request for solution 1 will be provided shortly.

Additional Context

No response

Created at 1 month ago
Created at 1 month ago