Skip to content
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.

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

Body/response parsing order is wrong #259

Open
timwhite opened this issue May 28, 2017 · 0 comments
Open

Body/response parsing order is wrong #259

timwhite opened this issue May 28, 2017 · 0 comments

Comments

@timwhite
Copy link

        // Decide how to parse the body of the response in the following order
        //  1. If provided, use the mime type specifically set as part of the `Request`
        //  2. If a MimeHandler is registered for the content type, use it
        //  3. If provided, use the "parent type" of the mime type from the response
        //  4. Default to the content-type provided in the response
        $parse_with = $this->request->expected_type;
        if (empty($this->request->expected_type)) {
            $parse_with = Httpful::hasParserRegistered($this->content_type)
                ? $this->content_type
                : $this->parent_type;
        }

Looking at #154 and #155 shows that I'm not the first to hit this issues. expectsJson() forces the parser to be JSON (1 in the selection order above).
Using something like expectsJson is really useful, as it priorities JSON in the Accepts header. BUT, we send an Accepts header of 'Accept: */*; q=0.5, text/plain; q=0.8, text/html;level=3;';q=0.9, application/json which specifically tells the server, we PREFER JSON, but we'll also accept anything you send to us.
Because our Accepts header is such, we should really be using the Content-Type response header as our first selection criteria.

There are numbers of API's out there, that when something goes wrong, fall back to HTML or XML. We have told the server we accept anything, so the server isn't doing the wrong thing, we are by trying to force it to parse as JSON.
Really, expectsJson() should be prefersJson(), because while we are expecting JSON, we can't guarantee it.

I'm working with an API that returns XML when something goes wrong, but I prefer to get the JSON response due to be serializing the response object (and the XML object doesn't serialize properly). My work around has been to stop using expectsJson(), and instead using addHeaders(['Accept' => 'application/xml, text/xml; q=0.5, application/json; q=0.9']), this allows the returned Content-Type header to be used to select the parser, and I don't get parsing exceptions when it tries to parse XML as JSON.

We should rewrite the parse selector so that expects*() doesn't force a parse, OR, we should change the Accepts header to only ask for the expected response type, and if the Content-Type header doesn't match what we expect, then throw an error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant