luckyframework / lucky

A full-featured Crystal web framework that catches bugs for you, runs incredibly fast, and helps you write code that lasts.
https://luckyframework.org
MIT License
2.57k stars 156 forks source link

Lucky does not handle Accept header properly #1766

Closed wezm closed 1 year ago

wezm commented 1 year ago

Describe the bug

Lucky incorrectly rejects a request as not acceptable, even thought the Accept header has a */* entry.

To Reproduce

  1. Create a lucky app with an API endpoint that inherits from the default ApiAction
  2. Make a request to the endpoint with an Accept header with this value: */*; q=0.5, application/xml

Expected behavior

The request is handled.

Actual behaviour

The request is rejected with status 406 Not Acceptable. The error indicates that the client wants XML so this should be added to the accepted formats. However, this is not true the */* entry means the client accepts anything, including the configured format, :json.

web          |  ▸  Lucky::NotAcceptableError
web          |
web          |      The request wants :xml, but Api::Payments::Webhook does not accept it.
web          |
web          |      Accepted formats: json
web          |
web          |      Try this...
web          |
web          |        ▸ Add :xml to 'accepted_formats' in Api::Payments::Webhook or its parent class.
web          |        ▸ Make your request using one of the accepted formats.

Versions (please complete the following information):

Additional context

  1. The client that is making this request is Stripe triggering a webhook.
  2. Adding :xml to accepted_formats allows working around the issue, but should not be necessary
  3. This seems related to the other issue I raised a few years ago #986
jwoertink commented 1 year ago

In your point 2, adding accepted_formats [:xml] to ApiAction is definitely necessary. Each action needs to know which mime types are allowed, and default is :json, so for an XML based API, you'd have to specify.

But I do think there may be some issues around using * in Accept headers. I had an issue before when just using cURL related to this.

wezm commented 1 year ago

In your point 2, adding accepted_formats [:xml] to ApiAction is definitely necessary. Each action needs to know which mime types are allowed, and default is :json, so for an XML based API, you'd have to specify.

Ahh yes, perhaps I wasn't clear. The Stripe API is completely JSON based, at least that's what is documented. There's no XML involved in what I'm doing. I'm not sure why their request indicates they accept XML.

jwoertink commented 1 year ago

I'd be curious to know... is that a bug in their postback API? That seems pretty strange if it's a JSON API, but their API sends an Accept header with "application/xml" in it :joy: maybe it's meant to be a fallback? :man_shrugging:

wezm commented 1 year ago

Weirdly enough it was reported and apparently fixed in July but appears to have returned... https://github.com/stripe/stripe-cli/issues/909