Accept header handling with accept-extensions

[jkennedy1980]

If you submit a request with an accept header like application/vnd.restify+json;ext=value you will get a 406 even if you add application/vnd.restify+json to the list of acceptable types. Additionally, customer formatters for application/vnd.restify will not be chosen and restify will throw a "500 no formatter" error. This appears to be due to the way that restify compares the accept header to the acceptable list and formatters map.

According to RFC2616 an accept header can contain 0 or more accept-parameters. Clients can use custom accept parameters with the exception of the 'q' parameter which is reserved. See spec here: https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html.

After digging into the code I see that the content types from the formatters map are normalized to the media-range part of the header and then merged into the acceptable list. e.g. application/vnd.restify+json;ext=value becomes application/vnd.restify+json.

I think it's safe to assume that a request with Accept: application/vnd.restify+json;ext=value should be considered acceptable when the server is configured with a formatter for application/vnd.restify. I'll be submitting failing tests and PR shortly.

[yunong]

@DonutEspresso mind taking a look at this? I know you were most recently in the formatters code base.

[DonutEspresso]

Thanks for filing the issue and PR! There's a lot to unpack here. Let's table the formatters for a second.

My reading of the RFC is that vnd.restify+json is the entire subtype. The + character itself is part of the subtype string, and not a special character that expands this into two separate subtypes. This snippet from W3C regarding application/html+xhtml appears to treat a subtype with a + as a string, as well. I couldn't find any documentation treating the + character in the way you are describing, please share any documentation as I may have missed it!

That said, application/vnd.restify+json;ext=value absolutely should fall back on application/vnd.restify+json, and that's a bug. From the RFC:

Media ranges can be overridden by more specific media ranges or specific media types. If more than one media range applies to a given type, the most specific reference has precedence. For example,

       Accept: text/*, text/html, text/html;level=1, */*

have the following precedence:

       1) text/html;level=1
       2) text/html
       3) text/*
       4) */*

I think this reads as, attempt to do a match with accept-extensions (other than q) before attempting to normalize the type. I think when we create the server, it'll have to be smarter and extrapolate accepted content-types automatically from the provided formatters when possible. Presumably that would trickle down through to req.accepts and the res formatter resolution. Thoughts?

[jkennedy1980]

Right. My issue is not with the handling of the '+' in the accept header. I only showed that because that is the format of the header I am using which lead to my discovery of this issue. I am not proposing any changes for this.

You are correct that the accept headers should be ordered by precedence according to each request. The problem is that there are 2 places in restify where the accept headers are used: acceptable and formatters. My PR would only adds simple support which basically allows extensions to function but without precedence. I think implementing precedence would require bigger changes like storing parsed accept headers which are then used when trying to choose a formatter.

I don't think the formatter definitions will trickle down to the acceptable list properly unless you start storing parsed accept headers or generate permutations into strings. People may also want to define complex acceptable types without formatters.

[DonutEspresso]

Ah, sorry - maybe I misread the initial issue, it sounded like you wanted automatic mapping to the subtype, i.e.,

application/vnd.restify+json => application/vnd.restify

If that's not the case, then I think we're on the same page. It looks like negotiator captured similar concerns in negotiator#35. My understanding is that that PR is primarily concerned with exposing the parsed parameters and extensions such that the end user can determine the correct value.

What's less clear to me is whether the desired behavior (application/vnd.restify+json;ext=value => application/vnd.restify+json) we are discussing is captured by the spec. i.e., is resolving extensions to no extension ok, or is any extension resolution application specific?

If it is in the spec, then I'd argue it's up to negotiator to do this resolution, rather than restify doing normalization prior to calling negotiator (as you have in the PR). If it is not in the spec, then I think we'd have to expose a negotiation hook in restify and pass the parsed accept parameters/extensions for the end user to decide.

[jkennedy1980]

Let's break the resolution of accept/content-type values into 2 types:

1. For the purpose of deeming a request acceptable or not.
2. For the purpose of identifying a proper response formatter.

I looked at negotiator#35 referenced above and agree that Negotiator is what is causing the first problem defined above and should be fixed. I'll put together a PR for negotiator with a fix for better extension handling.

The second problem defined above regarding formatters is a fix that will probably need to be made in Restify. When you define formatters it is already normalizing the accept values to exclude any parameters ( see mergeFormatters in server.js ). These normalized values are then merged into acceptable which will work if Negotiator is fixed. When a response formatter is being chosen the formatter needs to be selected based on the match priority including extensions. The formatters map will likely have to maintain the original parsed content types provided when the user is defining formatters.

To sum this all up:

1. Negotiator should properly support accept header extensions when sorting types by priority and stop returning null. This will fix the acceptable issue which is causing me to see 406s.
2. Restify should choose a formatter based on priority order using a provided content-type of best acceptable type. This will fix the 'no formatter' error that is being thrown. Perhaps this process should use Negotiator though the use of Negotiator inside Restify seems a bit hacky due to Negotiators interface. If some of the core utils were better exposed it may not seem so hacky.

Do you agree?

Thanks,

Josh

[DonutEspresso]

Hey @jkennedy1980, sorry for delayed reply. I'm with you on part 1.

Part 2 is where I'm less clear - my understanding based on reading the spec and the negotiator threads was that negotiation based on extensions is application specific - i.e., restify cannot safely determine the correct type for users when a match is not found. Instead, restify would return the parsed extensions (provided by negotiator) and a list of the formatters, allowing user land to make the correct determination. Likely, it'd have to be a new plugin that takes a user supplied function to do the resolution. Thoughts?

[anup4f82]

Is this issue fixed? I am talking about allowing Accept Headers of type -
application/vnd.restify;ext=value.

I see there is a PR - #1220 but when I look at the Recent codebase it does not have any of these changes. Was this change removed?

[retrohacker]

Hey @anup4f82,

These changes weren't removed, the PR was never merged. "Part 2" above is where the conversation left off during the review. According to the comment in #1220, it looks like the original author of the PR wont be finishing it up.

If we get a PR for part 1 above (which it sounds like @DonutEspresso 👍ed), I'll gladly do a review! I haven't looked too closely at the original PR yet, part 1 may be implemented there already and just needs to be pulled out.

Part 2 needs a bit more thought.

I'm going to close the PR, but leave this issue open.

[nateq314]

I need Restify to accept "application/vnd.api+json" (JsonAPI format) Accept headers and the below is not working:

const server = restify.createServer({
  name:     config.name,
  version:  config.version,
  log:      bunyanWinston.createAdapter(log),
  acceptable: ['application/vnd.api+json', 'application/json']
});

When I do a console.log('server.acceptable', server.acceptable);, the custom Accept header still doesn't appear in the list of valid Accept headers. Instead I always get the original

[ 'application/json',
  'text/plain',
  'application/octet-stream',
  'application/javascript' ]

Is there a correct way to do this that I'm missing? Or even a hacky way? Is the problem I'm running into related to this issue (#1219)?

[nateq314]

Please disregard, I solved the problem just now. Turns out I was using both the 'acceptable' property in the call to createServer() above, and the restify.acceptParser middleware with server.acceptable as the argument. Looks like the middleware was overriding the earlier acceptable: setting. Commented that out and it started working.

[biphobe]

I don't know which version did @nateq314 use, but I don't see "acceptable" option in current docs - my restify v4.3.0 ignored this option anyway.

const server = restify.createServer({
    formatters: {
        "application/json": restify.formatters["application/json; q=0.4"],
        "application/vnd.api+json": restify.formatters["application/json; q=0.4"]
    }
});

The above workaround worked for me.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically closed as stale because it has not had recent activity.