Project

General

Profile

Actions

Feature #26709

closed

Use correct http status codes

Added by Pavel Rosický over 6 years ago. Updated about 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Category:
REST API
Target version:
-
Start date:
Due date:
% Done:

0%

Estimated time:
Resolution:
Duplicate

Description

1/ redmine ignores accept headers and jquery dataType attribute

The Accept request-header field can be used to specify certain media types which are acceptable for the response.

If no Accept header field is present, then it is assumed that the client accepts all media types. If an Accept header field is present, and if the server cannot send a response which is acceptable according to the combined Accept field value, then the server SHOULD send a 406 (not acceptable) response.

http://api.jquery.com/jQuery.ajax/

The type of data that you're expecting back from the server. If none is specified, jQuery will try to infer it based on the MIME type of the response (an XML MIME type will yield XML, in 1.4 JSON will yield a JavaScript object, in 1.4 script will execute the script, and anything else will be returned as a string). The available types (and the result passed as the first argument to your success callback)

examples:

curl -v -H http://demo.redmine.org/issues.json
-> 200 + json OK!

but
curl -v -H "Accept: application/json" http://demo.redmine.org/issues
-> 500 no builder for format

expected (with patch)
-> 200 + json output

in some cases html is returned even if json was requested

2/ 406 error raises exception

curl -v http://demo.redmine.org/issues.xxx
-> 406 + exception ActionController::UnknownFormat

expected (with patch)
-> 406 no exception

3/ csrf protection - useful to avoid exceptions because of site-scanner bots

curl -v -H "Accept: text/javascript" http://demo.redmine.org/issues/new
-> 422 + exception ActionController::InvalidCrossOriginRequest: Security warning: an embedded <script> tag on another site requested protected JavaScript.

expected
-> 400 - no exception


Files

builders.rb.patch (501 Bytes) builders.rb.patch no builder for format Pavel Rosický, 2017-08-17 01:26
application_controller.rb.patch (1.11 KB) application_controller.rb.patch ActionController::UnknownFormat & CSRF Pavel Rosický, 2017-08-17 01:26
api_test.rb.patch (1.22 KB) api_test.rb.patch + spec Pavel Rosický, 2017-09-15 22:43

Related issues

Is duplicate of Redmine - Feature #34766: Better error message when no API format is recognisedClosedMarius BĂLTEANU

Actions
Actions #1

Updated by Toshi MARUYAMA over 6 years ago

Could you add tests?
source:trunk/test/integration

Actions #2

Updated by Pavel Rosický over 6 years ago

builders.rb.patch without a patch fails:

Failure:
Redmine::ApiTest::ApiTest#test_accept_header_on_error:
Expected response to be a <422: Unprocessable Entity>, but was a <500: Internal Server Error>.
Expected: 422
  Actual: 500

Failure:
Redmine::ApiTest::ApiTest#test_accept_header_on_show:
Expected response to be a <200: ok>, but was a <500: Internal Server Error>.
Expected: 200
  Actual: 500

application_controller.rb.patch this isn't worth fixing, it affects only logs so I can't detect it in tests anyway

Actions #3

Updated by Toshi MARUYAMA over 6 years ago

  • Target version set to 4.1.0
Actions #4

Updated by Jean-Philippe Lang almost 5 years ago

  • Target version changed from 4.1.0 to Candidate for next major release

Fixing status codes and changing the behaviour of the API regarding Accept header are different topics. I think these patches need some deeper review.

Actions #5

Updated by Pavel Rosický almost 5 years ago

only http://www.redmine.org/attachments/19034/builders.rb.patch is relevant.

this is the redmine way how to access an api, the format is determined as an extension

curl -v -H http://demo.redmine.org/issues.json

but I think that Accept header should be also supported

curl -v -H "Accept: application/json" http://demo.redmine.org/issues

---
this is how rails determines the format

What that says is, “if the client wants HTML or JS in response to this action, just respond as we would have before, but if the client wants XML, return them the list of people in XML format.” (Rails determines the desired response format from the HTTP Accept header submitted by the client.)

source https://api.rubyonrails.org/v5.1/classes/ActionController/MimeResponds.html

right now

curl -v -H "Accept: application/json" http://demo.redmine.org/issues

raises an internal server error

if this patch isn't accepted I would propose to change the status to
https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/406
instead of 500, what do you think?

https://blog.bigbinary.com/2010/11/23/mime-type-resolution-in-rails.html

Actions #6

Updated by Go MAEDA about 2 years ago

  • Related to Feature #34766: Better error message when no API format is recognised added
Actions #7

Updated by Marius BĂLTEANU about 2 years ago

  • Tracker changed from Patch to Feature
  • Status changed from New to Closed
  • Target version deleted (Candidate for next major release)
  • Resolution set to Duplicate

I'm closing this in favour of #34766 which have a patch ready to be committed for the status code change.

For the Accept header, I've created #36544.

Actions #8

Updated by Marius BĂLTEANU about 2 years ago

  • Related to deleted (Feature #34766: Better error message when no API format is recognised)
Actions #9

Updated by Marius BĂLTEANU about 2 years ago

  • Is duplicate of Feature #34766: Better error message when no API format is recognised added
Actions

Also available in: Atom PDF