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

Lucky does not handle Accept header properly #1766

Closed
wezm opened this issue Dec 28, 2022 · 4 comments · Fixed by #1769
Closed

Lucky does not handle Accept header properly #1766

wezm opened this issue Dec 28, 2022 · 4 comments · Fixed by #1769
Labels
Milestone

Comments

@wezm
Copy link
Contributor

wezm commented Dec 28, 2022

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):

  • Lucky version (check in shard.lock): 1.0.0.rc1
  • Crystal version (crystal --version): Crystal 1.6.2
  • OS: Linux

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 Lucky couldn't figure out what format the client accepts. The client's Accept header: 'image/*;q=0.8' #986
@wezm wezm added the bug label Dec 28, 2022
@jwoertink
Copy link
Member

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
Copy link
Contributor Author

wezm commented Dec 28, 2022

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
Copy link
Member

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 😂 maybe it's meant to be a fallback? 🤷‍♂️

@wezm
Copy link
Contributor Author

wezm commented Dec 28, 2022

Weirdly enough it was reported and apparently fixed in July but appears to have returned... stripe/stripe-cli#909

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

Successfully merging a pull request may close this issue.

2 participants