jshttp / accepts

Higher-level content negotiation
MIT License
252 stars 42 forks source link

Implementing "406: not acceptable" #16

Closed pietercolpaert closed 6 years ago

pietercolpaert commented 6 years ago

Context

In order to write a spec compliant content negotiation on the server, we also need to handle a request with e.g., the extreme accept: */*;q=0, which should return a 406 not acceptable.

Use cases are for example: a client that can only parse 1 serialization to do something like accept: application/json;q=1,*/*;q=0. If the server cannot provide an answer, the client will get the convenient 406.

Problem

In the accepts package, the server asks the client whether it accepts a certain list of values as follows (as copied from the README.md):

  switch (accept.type(['json', 'html'])) {
    case 'json':
      ...
      break
   case default:
      ...
      break
  }

The list ['json','html'] is the server-side list of preferences, yet the client also may have its preferences. It’s not because a client does not have application/json in its client-side list, that it cannot parse JSON. So, if the client is asking only for application/xml, but the server cannot offer that, the functionality should be that application/json is returned, as this is the preference from the server. Right now, the accepts library would return null, and leave the spec compliant implementation up to the user of the package. Yet probably, that user is not going to go through the pain of parsing the accept header all over again.

An extra note: it may be that the client cannot parse json at all, and wants to indicate that it can only parse XML.

Suggested solution

Instead of accept.type(['a','b','c']) is false or the format that is mentioned in the accept header of the request, I would only return false (and thus let the developer know it should return a 406) when the client indicated it does not accept any of the server-side representations. This function must return the first representation that is not not acceptable by the client.

When this is implemented, this would be a breaking change and would require a new major version number of this package.

RFC compliant explanation

https://developer.mozilla.org/en-US/docs/Web/HTTP/Content_negotiation

dougwilson commented 6 years ago

I'm sorry, but maybe we have fundamentally different understanding from each other. To me, you're describing exactly how this module already works, and it is not clear to me you're asking for anything to change. Maybe can you describe your request differently somehow? I have no idea what you're asking me to do or change currently.

dougwilson commented 6 years ago

You're always welcome to make a pull request with the change(s) you're looking to make as well, which may help my understanding of what you're asking to change (since you already made the change) :+1:

pietercolpaert commented 6 years ago

Here are 2 examples to replicate wrong behavior provided by the example in the README:

1 - Should give text/html

This one says it doesn’t want json, but all else is fine:

$ curl http://localhost:3000 -H 'accept: application/json;q=0' -I
HTTP/1.1 200 OK
Content-Type: text/plain

This one says it wants XML, but all else is fine as well:

curl http://localhost:3000 -H 'accept: application/xml' -I
HTTP/1.1 200 OK
Content-Type: text/plain

2 - Should give 406 not acceptable

This one says it does not want HTML and it does not want JSON either, but all else is fine

$ curl http://localhost:3000 -H 'accept: application/json;q=0,text/html;q=0' -I
HTTP/1.1 200 OK
Content-Type: text/plain
dougwilson commented 6 years ago

Ah, thanks for the follow up. I see the issue here.

Mainly, I think you're misunderstanding the Accept header itself. The spec is at https://tools.ietf.org/html/rfc7231#section-5.3.2

1 - No, text/plain for the example is correct. The header accept: application/json;q=0 just says "I accept nothing"; it does not say"doesn’t want json, but all else is fine" -- that would be the header accept: application/json;q=0, */*. The accept header is spec'd to be an exhaustive list -- anything not listed is not accepted.

2 - The example doesn't show how to do 406, so of course you won't get a 406 response. You need to change the example if that's what you actually want to do (which is not what the example is trying to demonstrate):

var accepts = require('accepts')
var http = require('http')

function app (req, res) {
  var accept = accepts(req)

  // the order of this list is significant; should be server preferred order
  switch (accept.type(['json', 'html'])) {
    case 'json':
      res.setHeader('Content-Type', 'application/json')
      res.write('{"hello":"world!"}')
      break
    case 'html':
      res.setHeader('Content-Type', 'text/html')
      res.write('<b>hello, world!</b>')
      break
    default:
      // client doesn't accept anything we can send
      res.statusCode = 406
      res.write('not acceptable')
      break
  }

  res.end()
}

http.createServer(app).listen(3000)