Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Symfony\Component\Routing\Exception\MethodNotAllowedException: in Symfony\Component\Routing\Matcher\UrlMatcher->match() (line 97 of vendor/symfony/routing/Matcher/UrlMatcher.php).
Drupal\Core\Routing\UrlMatcher->finalMatch(Object, Object) (Line: 152)
Symfony\Cmf\Component\Routing\NestedMatcher\NestedMatcher->matchRequest(Object) (Line: 258)
Symfony\Cmf\Component\Routing\DynamicRouter->matchRequest(Object) (Line: 185)
Symfony\Cmf\Component\Routing\ChainRouter->doMatch('/node/212', Object) (Line: 155)
Symfony\Cmf\Component\Routing\ChainRouter->matchRequest(Object) (Line: 89)
Drupal\Core\Routing\AccessAwareRouter->matchRequest(Object) (Line: 138)
Drupal\Core\Routing\AccessAwareRouter->match('/node/212') (Line: 139)
Drupal\user\Plugin\LanguageNegotiation\LanguageNegotiationUserAdmin->isAdminPath(Object) (Line: 109)
Drupal\user\Plugin\LanguageNegotiation\LanguageNegotiationUserAdmin->getLangcode(Object) (Line: 191)
Drupal\language\LanguageNegotiator->negotiateLanguage('language_interface', 'language-user-admin') (Line: 136)
Drupal\language\LanguageNegotiator->initializeType('language_interface') (Line: 223)
Drupal\language\ConfigurableLanguageManager->getCurrentLanguage() (Line: 84)
Drupal\language\EventSubscriber\LanguageRequestSubscriber->onKernelRequestLanguage(Object, 'kernel.request', Object) (Line: 116)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('kernel.request', Object) (Line: 120)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 62)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 62)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 53)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 103)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 82)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 55)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 632)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
Steps to reproduce
Step to reproduce:
1. Install module rest.
2. Install module language.
3. Install module page_manager(Seams that this step can be excluded).
4. Create some language.
5. go to /admin/config/regional/language/detection and check Account administration pages.
6. Go to user edit page and set some language in "Administration pages language".
7. Go to node view.
Proposed resolution
In LanguageNegotiationUserAdmin: isAdminPath update catch with ExceptionInterface | HttpException
Remaining tasks
Merge
User interface changes
NA
API changes
NA
Data model changes
NA
Release notes snippet
NA
Comment | File | Size | Author |
---|---|---|---|
#79 | interdiff_74-79.txt | 2.28 KB | Spokje |
#79 | 2706241-79.patch | 5.79 KB | Spokje |
#78 | 2706241-test_only-should_fail-78.patch | 4.36 KB | Spokje |
Comments
Comment #2
george.karaivanov CreditAttribution: george.karaivanov at Peytz & Co commentedMethod Drupal\user\Plugin\LanguageNegotiation\LanguageNegotiationUserAdmin\isAdminPath
should catch MethodNotAllowedException as well.
I've create patch that fix it
Comment #3
george.karaivanov CreditAttribution: george.karaivanov at Peytz & Co commentedComment #4
dawehnerThis reads totally reasonable. Do you think its possible to write a test for that?
Comment #5
george.karaivanov CreditAttribution: george.karaivanov at Peytz & Co commentedSame story was on https://www.drupal.org/node/2619700
Was fixed same way
Comment #6
dawehnerFair, but some test coverage would be nice nevertheless.
Comment #8
lslinnet CreditAttribution: lslinnet as a volunteer commentedI am currently experiencing this issue on a project I am working on where we have a route similar to this:
When a logged in user tries to access it the Language negotiation fails because of the route matching creates an incomplete request object (it basically thinks a POST request can be matched like a GET request).
It do not seem farfetched to have a route where only POST is a valid method and not allowing for proper route matching due to the request method seems to be an wrong way to "solve" the issue.
I would aim for a solution that takes the current request into account when doing the route matching instead of mocking a new request.
Comment #9
lslinnet CreditAttribution: lslinnet as a volunteer commentedI tried with the following implementation which takes the request method into consideration, haven't verified if it passes any of the internal tests, but seems to solve the problem at hand.
Comment #10
lslinnet CreditAttribution: lslinnet as a volunteer and at Adapt commentedSecond though, here is a patch fil with a minor update checking if we are dealing with an instance of RequestMatcherInterface as the router.
Comment #11
lslinnet CreditAttribution: lslinnet as a volunteer commentedComment #13
dpiI cannot reproduce this issue using the steps in the issue summary.
Including with page_manager-alpha14
Is this still a problem?
Comment #14
lslinnet CreditAttribution: lslinnet as a volunteer commented@dpi, this is reproducible without page_manager module - so yes this is still an issue.
Comment #16
sathish.redcrackle CreditAttribution: sathish.redcrackle at Red Crackle commentedI checked in drupal 8.3 and couldn't reproduce this issue.
Steps i followed:
1. Enabled Rest, language, page_manager
2. Created languages
3. changed settings in /admin/config/regional/language/detection and checked Account administration pages
4. and in user edit page i have set language in "Administration pages language".
5. went to node/view pages and i found no error.
Comment #17
lslinnet CreditAttribution: lslinnet as a volunteer commented#16 the steps above might not reproduce the issue, but I could not at a quick glance at the 8.3.x codebase see changes that would prevent this from happening in the previous mentioned case I ran into.
Will take a look at this over the weekend and see where we are in regards to this functioning as needed.
Comment #18
Wim LeersComment #19
Wim LeersThis was reported again at #2878785: MethodNotAllowedException using POST or PATCH with REST on 8.3.2, which was marked as a duplicate.
Comment #20
Wim LeersThis is very similar to #2822190: PathValidator validates based on a RequestContext leaked from the current request, resulting in false negatives during CLI requests and POST submissions, which fixed
\Drupal\Core\Path\PathValidator::getPathAttributes()
. That method is very similar to\Drupal\user\Plugin\LanguageNegotiation\LanguageNegotiationUserAdmin::isAdminPath()
.Comment #21
Wim LeersComment #22
Wim LeersActually, I think this belongs in
routing system
, since that's where the root cause lies.Comment #23
nplowman CreditAttribution: nplowman at FFW commentedI'm still encountering issues when trying to create files over REST using the file_entity module as shown here:
https://www.drupal.org/node/2682341#comment-11965256
The issue in this case seems to be that some of the original headers are not getting passed along.
It seems that we might need to pass in an exact clone of the $request object, that only has the URI modified.
I'm not seeing a great way of doing that though.
As a work around for my current needs, I'm just adding an additional check so that isAdminPath() only gets checked on GET requests.
I'll add a patch in case that is helpful for anyone else.
Comment #27
mglamanIt seems like this issue more succinctly summarizes the problem at hand: #2857132: "Account administration pages" language negotiation ignores _format blocking REST resources. It shouldn't be caught rather than it should respect the _format parameter.
Comment #28
mglamanRetitling based on #22 and the work done in PathValidator. When
\Drupal\Core\Routing\AccessAwareRouter::match
recreates the request to match, it does not pass the proper HTTP method from the original request.This causes a POST request to be matched as a GET request, causing LanguageNegotiationUserAdmin to fail at MethodNotAllowed exception for valid routes.
I did some digging in https://www.drupal.org/project/drupal/issues/2857132#comment-12865440, where I thought it would be an appropriate issue. But it's more than just respecting _format. The problem is that AccessAwareRouter creates an invalid request to match against known routes for the path.
Incoming patch which updates the AccessAwareRouter to use its route context to pass the proper HTTP method when creating a request object to match on.
Comment #29
mglamanHere is the patch which updates \Drupal\Core\Routing\AccessAwareRouter::match to provide a correct request object.
Comment #30
mglamanWe're past the initial issue, however this is still insufficient. We need to ensure the _format parameter is passed so that MediaTypeMismatch is not thrown.
Comment #31
mglamanThis fixes POST/PATCH/DELETE requests for our rest.module plugins.
Comment #32
mglamanThis needs tests. This scenario seems to only happen when routes share similar paths and the missing HTTP method / _format cause the routing system to become confused.
The bug is exposed by \Drupal\user\Plugin\LanguageNegotiation\LanguageNegotiationUserAdmin. But that's not what needs to be tested.
It may be feasible to test in \Drupal\Tests\Core\Routing\AccessAwareRouterTest with \Drupal\Tests\Core\Routing\RoutingFixtures.
Comment #33
mglamanWe did some more testing with this. As soon as the admin language is configured things break, again.
The content type header filter causes the route to become ineligible: \Drupal\Core\Routing\ContentTypeHeaderMatcher::filter.
The request context parameters is an empty array, and the _format is not passed as the query string. Also, the content type is not set.
In our case JSON payloads were being rejected on a media mismatch due to this logic:
I'm wondering if the problem is
\Drupal\Core\Routing\AccessAwareRouter::match
or the fact\Drupal\user\Plugin\LanguageNegotiation\LanguageNegotiationUserAdmin::isAdminPath
invokes this method overmatchRequest
. Without a preferred administration language configured thematchRequest
is invoked.The other problem is that core cannot decide if
$this->route
should be type hinted against UrlMatcherInterface or RequestMatcherInterface. It choses either or based on the required methods.AccessAwareRouterInterface
implements both and is also the default interface for Drupal's default router service.Comment #34
mglamanI'm giving up on elegance here. I'm falling back to what was mentioned in #20: it works around the
MethodNotAllowedException
thrown by\Drupal\Core\Routing\AccessAwareRouter::match
. If we fix the method, we end up with a MediaMismatch exception. This fixes the problem in the negotiator and punts fixing the access router (which feels like a rabbit hole) and allows REST plugins to work as expected.This is essentially a reroll of #2 after a lot of failed attempts to fix the router.
Comment #35
johnpicozziThe patch in #34 seems to resolve the issue. RTBC +1
Comment #36
Wim Leers😭
I think this absolutely needs to be reviewed/RTBC'd by the routing system maintainer.
Comment #37
mglamanThe problem is that, inherently,
match
by path info alone cannot work for POST, PUT, PATCH. No matter how much request context we pass the_format
is lost and so is theCONTENT_TYPE
header. That means any matching of a path on those methods will instantly fail. Fixing theMethodNotAllowedException
exception uncovered this -- trading off for constantly having a mismatched media exception.The
match
method is fine for GET, HEAD, and DELETE when there is no request body and content type matching requirement.Comment #38
tim.plunkettCan we get tests to codify the steps to reproduce from the IS?
Step 3 involves contrib, so we'd need a test module to simulate that portion of the flow.
Comment #39
bklineI had to combine the fix from the most recent patch with the workaround in #23 to get our API calls to succeed.
@tim.plunkett - sorry, what does "IS" mean in #38?
Comment #40
MatroskeenJust for the record,
Due to this issue, ajax flag links in Flag module doesn't work (together with enabled "Administration pages language" of course).
They're expecting request method to be "POST" but it never happens in
AccessAwareRouter::match()
method.Comment #42
mdupontJust found a way to trigger this issue on Drupal 8.8.5 with the Media module:
MethodNotAllowedException
It turns out opening the Media editor dialog box calls the route
'/editor/dialog/media/{editor}'
which is restricted to the POST method. When it goes through theAccessAwareRouter
, the HTTP verb context is lost and aMethodNotAllowed
exception is triggered. Using the patch at #34 fixed the issue.Note: tested on a multilingual site, I don't know if it has an impact.
Comment #43
maacl CreditAttribution: maacl commented#3048288: MethodNotAllowedException thrown on POST requests if user has set admin language in their profile uses the same patch and is also core only.
Comment #45
Chi CreditAttribution: Chi commentedA test to prove the issue. Should fail.
Comment #46
Chi CreditAttribution: Chi commentedThe fix is basically the same as in #2.
Comment #47
andypostMaybe unit test here should be enough?
Comment #48
kunalkursija CreditAttribution: kunalkursija at Axelerant commentedHaven't tried the patches yet, But confirming that the steps suggested in #42 are valid and resulting in PHP-500 error on Drupal-8.9.2 as well.
Comment #49
tim.plunkettComment #50
Nchase CreditAttribution: Nchase as a volunteer commentedI can confirm that the findings in #42 can be reproduced and that the patch in #34 works with 8.9.3. The dialog box finally appears and the 500 error is gone. Thank you!
Comment #51
nimoatwoodwayPatch in #34 works for me. Thanks!
Had a problem with the contrib module Flag described in this ticket: https://www.drupal.org/project/flag/issues/3110134
Using Drupal 9.0.3, Multilanguage enabled, default language set to german and admin language set to english.
Comment #52
DamienMcKennaRerolled.
Comment #53
NWOM CreditAttribution: NWOM commented#46 worked great and fixed the issue for Commerce Cart Flyout's update quantity patch: #3144651: Add quantity to form. Thank you!
Comment #54
DamienMcKennaThis is #52 ported to 9.2.x, and also applies to 9.1.x.
Comment #55
quicksketchWe are experiencing this problem when editing media on a multilingual site. This is identical to the report @mdupont described in #42.
I have this problem on a Drupal 9.2 site. I have confirmed that the latest patch in #54 works to solve the immediate problem.
That said, I don't think the patch takes the right approach. When opening a media dialog, the route is
/editor/dialog/media/{editor}
and should be considered an administrative path, since it was called from an administrative path (/node/add/page
). By catching theMethodNotAllowedException
exception and returning FALSE, you could end up with the media dialog displaying in the front-end language rather than the admin language.I think the right solution here would be to preserve the current page request method (POST) when validating the route, rather than suppressing the exception and returning a potentially incorrect value.
Comment #56
extect CreditAttribution: extect commented#54 fixes the issue for me. Thanks!!
@quicksketch: I can confirm the issue regarding media dialog showing up in front-end rather than back-end language.
On the other hand, very similar problems have been solved by catching MethodNotAllowedException and returning FALSE as well:
#2619700: \Drupal\Core\Path\PathValidator::getPathAttributes() does not catch MethodNotAllowedException
#2822190: PathValidator validates based on a RequestContext leaked from the current request, resulting in false negatives during CLI requests and POST submissions
...
Shouldn't we take the same direction in here and than check in a separate issue for all places where catching MethodNotAllowedException and returning FALSE is causing trouble?
For the moment, media dialog showing up in the wrong language still is better than it not loading at all.
Comment #58
rkamepalli CreditAttribution: rkamepalli commented#52 Worked.
I experienced this issue with Acquia Site Studio/Cohesion when importing UI Kit YML file in a multilingual site studio powered site
Thanks
Rakesh
Comment #59
Rade CreditAttribution: Rade at Wunder commentedI found this issue via #2857132: "Account administration pages" language negotiation ignores _format blocking REST resources.
If we are going with the solution to catch different exceptions, we might as well add
\Symfony\Component\HttpKernel\Exception\NotAcceptableHttpException
, which would solve issue 2857132 as well.Attached a patch based on #54, with the addition. (But without tests for now, since they didn't apply cleanly from #54 on 9.3.x)
Comment #60
Rade CreditAttribution: Rade at Wunder commentedComment #62
FiNeX CreditAttribution: FiNeX as a volunteer commentedHi, patch #54 works on D 9.3. Thank you.
Comment #63
JeroenTBack to needs work since Patch #54 needs a reroll for Drupal 9.4.
Comment #64
ankithashettyRerolled the patch in #54 and updated the deprecated code (
$this->drupalPostForm('/user-language-test/form', [], 'Send');
) in core/modules/user/tests/src/Functional/UserAdminLanguageTest.php file, thanks!Comment #65
kevinquillen CreditAttribution: kevinquillen commentedIf you are using Site Studio with Content/Interface translation, you absolutely need the patch in #59 (Drupal 9.3.3) or the package export interface will be broken.
Comment #66
JeroenTCan we also re-add the exception added in #59?
Comment #67
JeroenTI re-added the exception that was added in #59.
Comment #69
anprok CreditAttribution: anprok at Drupal Ukraine Community for Drupal Ukraine Community commentedWith PATCH request to REST endpoint on my project with core 9.4.2 I got error with Symfony\Component\Routing\Exception\MethodNotAllowedException: in Symfony\Component\Routing\Matcher\UrlMatcher->match() for route with PATCH method. I applied the patch from #67 and the problem is gone. Nice to have this fix in core.
Comment #70
alexpottIf we include #67 then it would be great to add some test coverage of that case?
Another option...
How about catching \Symfony\Component\Routing\Exception\ExceptionInterface and \Symfony\Component\HttpKernel\Exception\HttpException exceptions... like if routing fails for whatever reason here I think we should return FALSE. I don't think any of the Http or Routing exceptions here should result in an exception reaching the user.
Comment #72
c_archer CreditAttribution: c_archer as a volunteer and commentedPatch #67 worked for me. Agree some test coverage for it would be good.
Comment #73
Chi CreditAttribution: Chi commentedTests exists in the patch #46, but weren't added to re-rolled patches.
One minor change here is that we can remove $exception variable for D10 patch as we don't need to support PHP 7.
Comment #74
rteijeiro CreditAttribution: rteijeiro at European Commission and European Union Institutions, Agencies and Bodies commentedRe-rolled #67 patch to work with Drupal 9.4.12
Comment #75
andypostComment #76
mvonfrie CreditAttribution: mvonfrie as a volunteer and at trinitec IT Solutions & Consulting GmbH commentedThis also effects Webprofiler #64 where the webprofiler toolbar reports data to the backend via
POST /admin/reports/profiler/frontend/{profile}/navigation
. The route only allows HTTP POST but as the admin path check of the "admin language from user preferences" language negotiator can't pass the HTTP method to the AccessAwareRouter it fails with the MethodNotAllowedException, which happens at least once per main request.Patch #74 seems to fix this. I can't 100% confirm yet as I have another issue with webprofiler to solve first.
Comment #78
SpokjeThis seems to be the root cause for #3351241: ckeditor5 upload image MethodNotAllowedException.
Let's first get a test-only should-fail patch on
11.x
based on #74 up.Comment #79
SpokjeAnd here's the full patch, based on #74, taking #70 on board.
Comment #80
SpokjeComment #81
sanket.addweb CreditAttribution: sanket.addweb commentedI tested the patch 2706241-79.patch and identified a syntax error in the catch statement. I made a minor syntax adjustment to resolve the issue.
Comment #82
SpokjeComment #83
dpi@sanket.addweb
There is no syntax problem, so long as you're on PHP8.
Make sure you upload an interdiff, you would have noticed you omitted a significant portion of the previous patch.
The current reviewable patch remains #79.
Comment #84
smustgrave CreditAttribution: smustgrave at Mobomo commentedUpdated the issue summary to include the solution from #79.
Majority of the patch is the added tests which #78 show are failing as expected.
Think this is good.
Comment #86
SpokjeKnown random test failure (#3380433: [random test failure] CronRunTest::testAutomatedCron), ordered retest.
Comment #88
SpokjeComment #91
catchCommitted/pushed to 11.x and cherry-picked to 10.1.x, thanks!