Skip to content

[HttpKernel] Ensure HttpCache::getTraceKey() does not throw exception #58921

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Nov 20, 2024
Merged

[HttpKernel] Ensure HttpCache::getTraceKey() does not throw exception #58921

merged 1 commit into from
Nov 20, 2024

Conversation

lyrixx
Copy link
Member

@lyrixx lyrixx commented Nov 19, 2024 •
edited
Loading

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Issues -
License MIT

We have such logs in our logs. It's in our raw PHP logs. They are not caught by monolog, it's too early

[11-Oct-2024 01:23:33 UTC] PHP Fatal error:  Uncaught Symfony\Component\HttpFoundation\Exception\SuspiciousOperationException: Invalid method override "__CONSTRUCT". in /var/www/redirection.io/backend/blue/vendor/symfony/http-foundation/Request.php:1234
Stack trace:
#0 /var/www/redirection.io/backend/blue/vendor/symfony/http-kernel/HttpCache/HttpCache.php(728): Symfony\Component\HttpFoundation\Request->getMethod()
#1 /var/www/redirection.io/backend/blue/vendor/symfony/http-kernel/HttpCache/HttpCache.php(207): Symfony\Component\HttpKernel\HttpCache\HttpCache->getTraceKey()
#2 /var/www/redirection.io/backend/blue/vendor/symfony/http-kernel/Kernel.php(188): Symfony\Component\HttpKernel\HttpCache\HttpCache->handle()
#3 /var/www/redirection.io/backend/blue/web/app.php(9): Symfony\Component\HttpKernel\Kernel->handle()
#4 {main}
  thrown in /var/www/redirection.io/backend/blue/vendor/symfony/http-foundation/Request.php on line 1234

I managed to reproduced locally.

  • Before the patch, without the http_cache, symfony returns a 405
  • After the patch, without the http_cache, symfony returns a 405
  • Before the patch, with the http_cache, symfony returns a 500, without any information (too early)
  • After the patch, with the http_cache, symfony returns a 405

@carsonbot carsonbot added this to the 5.4 milestone Nov 19, 2024
@lyrixx lyrixx changed the title [HttpKernel] Ensure HttpCache::getTraceKey() does not throw exception [HttpKernel] Ensure HttpCache::getTraceKey() does not throw exception Nov 19, 2024
@lyrixx lyrixx force-pushed the SuspiciousOperationException-and-cache branch from 7566856 to 3e931d8 Compare November 19, 2024 09:22
nicolas-grekas
nicolas-grekas reviewed Nov 20, 2024
View reviewed changes
src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php Outdated
try {
return $request->getMethod().' '.$path;
} catch (SuspiciousOperationException $e) {
return '_INVALID_ '.$path;
Copy link
Member

Choose a reason for hiding this comment

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

to make it clear it's the method?

Suggested change
return '_INVALID_ '.$path;
return '_BAD_METHOD_ '.$path;

@nicolas-grekas nicolas-grekas force-pushed the SuspiciousOperationException-and-cache branch from 3e931d8 to a2ebbe0 Compare November 20, 2024 10:47
@nicolas-grekas
Copy link
Member

Thank you @lyrixx.

@nicolas-grekas nicolas-grekas merged commit 32d55d7 into symfony:5.4 Nov 20, 2024
10 of 12 checks passed
@lyrixx lyrixx deleted the SuspiciousOperationException-and-cache branch November 20, 2024 12:41
This was referenced Nov 27, 2024
Merged
Merged
Closed
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas left review comments

Assignees
No one assigned
Labels
Bug HttpKernel Status: Needs Review
Projects
None yet
Milestone
5.4
Development

Successfully merging this pull request may close these issues.
3 participants
@lyrixx @nicolas-grekas @carsonbot