Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

[Router] If the _method is not accepted, then the _format is always 'html' #6735

Closed
marcospassos opened this Issue · 8 comments

5 participants

@marcospassos

Hi all,

It seems like a Symfony limitation or bug (in our case a security case).

We use the _format for some validations and reports even when a Exception is thrown. But apparently it is not trusty all time. In one of the cases of use, we use the _format to generate the Exception redering according with requested format, but the format is ignored when the _method is not accepted.

product_option_create:
    pattern: /create.{_format}
    defaults:
        _format: ~
        _controller: controller.option:createAction
    requirements:
        _method: POST

The Symfony router cache system generates a cached UrlMatcher ( appdevUrlMatcher ) that has a method match($pathinfo) which verifies which cached URL matches with the current URL (including the _method) and returns the preg_match result to be merged with the context parameters. The preg_match result has all variables declared in the routes configurations populated with the URL data. The problem is when the parameter _method of route does not match with the request type, even then nothing is returned because a MethodNotAllowedHttpException is thrown.

#app/cache/dev/appdevUrlMatcher.php
public function match($pathinfo)
{
    if (0 === strpos($pathinfo, '/product/option/show') && preg_match('#^/product/option/show(?:\\.(?P<_format>json))?$#s', $pathinfo, $matches)) {
        if ($this->context->getMethod() != 'POST') {
            // print_r($matches);
            // Array ( [0] => /product/option/show.json [_format] => json [1] => json )
            $allow[] = 'POST';
            goto not_product_option_show;
        }

        return $this->mergeDefaults(array_replace($matches, array('_route' => 'product_option_show')), array (  '_format' => NULL,  '_controller' => 'product.controller.option:showAction',));
    }
    not_product_option_show:

    throw 0 < count($allow) ? new MethodNotAllowedException(array_unique($allow)) : new ResourceNotFoundException(); //
}

Thus, the method getRequestFormat returns html once the Request->format property is null.

#Symfony\Component\HttpFoundation\Request
public function getRequestFormat($default = 'html')
{
    #echo $this->format; // returns 'html'
    if (null === $this->format) {
        $this->format = $this->get('_format', $default);
    }

    return $this->format;
}

As the format is html, the method ExceptionController->showAction understands that is a case of templating format and renders a html page error instead of JSON serialization.

Maybe a possible solution is to pass the $matches embedded in the MethodNotAllowedException, but there is a discussion about it on FriendsOfSymfony/FOSRestBundle#358

@Tobion
Collaborator

I see the problem. Passing the matches to the MethodNotAllowedException might make sense to know what would have matched with the appropriate method. It would need to pass the matches for each method requirement individually because the matches can differ for each GET, POST, etc.

@Tobion
Collaborator

The real Request format can then be set depending on the theoretical matches.

@jfsimon

I think it would be great, but I see a caveat. Let take following routing:

test1:
    path: /test/{_format}
    requirements: { _method: post }
    # ...

test2:
    path: /test/{id}
    requirements: { _method: delete }
    # ...

If I request /test/something with GET method, should I feed _format with something?
@marcospassos @Tobion what do you think about that?

@stof
Collaborator

@jfsimon bad example as the router matches HEAD request as if they were GET requests (the spec say that a HEAD request should behave the same than a GET request except it returns an empty body), so test2 will not match for HEAD requests

@jfsimon

@stof wow, I didn't think about that... I replace head with delete.

@lsmith77 lsmith77 referenced this issue in FriendsOfSymfony/FOSRestBundle
Closed

Redirect view #500

@jrobeson

so how should we deal with this now? the same way @Tobion suggested?

@Tobion
Collaborator

Ok I looked into that and the origin cause in fos rest bundle in detail.
The issue in rest bundle is not valid anymore.
And in symfony itself, the only issue is see is that the format is lost when having a 405 or 404. But the only good solution is to just have content negotiation in symfony (similar to the one in rest bundle). It would just set the format based on the extension or Accept header and thus the correct format would also be available when dealing with a 404/405.
So I'm closing this as duplicate of #10538

The original idea to pass the theoritical matches for each request method seems more like a workaround.

@Tobion Tobion closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.