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

Fix inconsistent error responses when using JSONP #2466

Closed

Conversation

kfitzgerald
Copy link

Greetings,

It appears that when using JSONP, only half the time will a response be wrapped and sent, depending on where the routing cycle ends during the request.

Inconsistent Responses

For example, if any one of these things occur during the request, the response will not be wrapped in JSONP:

  • State fails
  • Pre-auth hook points trigger a response
  • Authentication failure / missing
  • Post-auth hook points trigger a response
  • Payload fails to parse / authenticate
  • Headers validation fails
  • Params validation fails

Only when these things occur, do they actually get wrapped in a JSONP response:

  • Query validation fails
  • Payload validation fails
  • Pre-handler hook points trigger a response
  • Handler responses
  • Post-handler hook points trigger a response
  • Response validation fails

Motivation

In our situation, we have created a custom authentication scheme that will check and authenticate a query, header or cookie parameter (e.g. API key) and return the credentials for the route handler.

Since state and authentication occurs earlier in the routing cycle than JSONP does, those responses will not be wrapped up in a JSONP context, making it impossible for the client to understand what happened. Same thing goes if a route parameter was bogus. However, if they mess up a query parameter, they would actually get that error wrapped.

GET /some/stuff?callback=test (no key param, cookie, or Authorization header)

Expected
/**/test({"statusCode":401,"error":"Unauthorized","message":"Missing authentication"});
Actual
{
    "statusCode": 401,
    "error": "Unauthorized",
    "message": "Missing authentication"
}

TL.DR

This request moves JSONP handling to the top of the request cycle, as an attempt to make all responses consistent. Naturally, the only response that would not get wrapped is when the jsonp callback method name is invalid, then obviously it cannot be wrapped.

If the client is passing in a JSONP callback, they should expect that the response, no matter how it ends, should come back wrapped up. Error or not, here I come!

…ors will be wrapped in a callback, instead of just some.
@kfitzgerald
Copy link
Author

On a side note, we had to add a pre-response hook to fudge response codes for JSONP requests, since some browsers eat scripts that don't come back status 200.

For example:

server.ext('onPreResponse', function(request, reply) {
    if (request.jsonp) {
        if (request.response.isBoom) {
            request.response.output.statusCode = 200;
        } else {
            // This might need to change if the response is totally custom like a string or something
            request.response.statusCode = 200;
        }
    }

    return reply.continue();

});

@hueniverse hueniverse added the bug Bug or defect label Mar 12, 2015
@hueniverse
Copy link
Contributor

I agree that this is a problem, but this solution won't work because the jsonp parameter cannot be removed until auth is done. I'll solve it a bit differently.

@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Bug or defect
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants