amberframework / amber

A Crystal web framework that makes building applications fast, simple, and enjoyable. Get started with quick prototyping, less bugs, and blazing fast performance.
https://amberframework.org
MIT License
2.58k stars 208 forks source link

Varying JSON response types in the Accept header can produce html for json requests #1244

Open damianham opened 3 years ago

damianham commented 3 years ago

Description

The HTML spec states that the JSON response type should be "application/json; charset=utf-8". However browsers often send just "application/json". Flutter/Dart Http client sends "application/json; charset=utf-8". For Dart clients the response type key in the @available_responses hash has to be "application/json; charset=utf-8". If a request action responds with html and json and the html response is defined first then the response will be html for json requests if the json request type does not match the available_responses json key. If the Accept header has multiple request types or includes "/" then the first available response type is added to the requested response types, thus, if defined first, the html response type is added to the requested response types for json requests.

Steps to Reproduce

  1. define a controller method with the following respond_with block
    respond_with do
    html -> { render("index.slang")
    json {result: "OK"}
    end
  2. Submit a request with an Accept header of "application/json; charset=utf-8, text/javascript, /; q=0.0"

Expected behavior: The response should be

{"result": "OK"}

Actual behavior: you get the html contents of index.slang

Reproduces how often: 100%

Versions

Amber CLI (amberframework.org) - v0.35.0

Crystal 0.35.0 [3c48f311f] (2020-06-09)

LLVM: 8.0.0 Default target: x86_64-unknown-linux-gnu Ubuntu buster/sid Linux 8d6c4a99af3b 5.4.0-65-generic #73-Ubuntu SMP Mon Jan 18 17:25:17 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

Additional Information

This respond_with block works as expected because the json response type is added to the requested response types

respond_with do
  json {result: "OK"}
  html -> { render("index.slang")
end
crimson-knight commented 2 years ago

@damianham is there any chance you'd be able to try this again using the latest Amber release and see if it's still an issue?

I'm trying to work my way through a fairly large backlog of things and this seems very important but help would be greatly appreciated!

damianham commented 2 years ago

@crimson-knight the relevant file lib/amber/src/amber/controller/helpers/responders.cr hasn't been updated in 2 years so I am sure the issue still exists without needing to test it.