Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Match accept type exactly #1297

Closed
mstade opened this Issue · 22 comments

6 participants

@mstade

I'm probably not quite understanding how to use request.accepts() so any input here would be awesome. Basically, I'm trying to write some middleware that only deals with requests where the accept type matches something exactly. Looking at the docs, it seems request.accepts() isn't quite useful for this purpose, and I end up having to do this:

var acceptsType = (req.accepted || []).some(function(type) {
    return type.value === "my/type"
})

It's not a huge deal, but if I don't do that, things like */* seems to match, and browsers likes sending that it seems.

If request.accepts() already does this somehow, I'd love to know. If not, I suggest changing the logic of request.accepts() so that it returns the actual best match, meaning for a request that has Accept: */* it'd actually return */* if that is indeed the best match, rather than the input mime type. Thus, if you then want to do an exact match, the code would look something like:

if (req.accepts("application/json") == "application/json") {
    // Exact match
}

For cases when you just want to know if the client accepts the mime type, but isn't necessarily an exact match, just drop the quality check:

if (req.accepts("application/json")) {
    // Returns */* or whatever the match is, but truthy value anyhow
}

I'm probably not understanding the accepts function correctly, but it seems to me it's pretty much useless whenever a client sends Accept: */* and you have to resort to looking through the accepts array. Please do correct me if I'm wrong!

@tj
tj commented

yeah there is a debate going on about this in one of the other issues (or a commit, i cant remember). Technically if a browser or client gives you "/" then it's really willing to accept anything, in which case Express is doing the correct thing. One thing we could maybe do since there seems to be a consensus that people want Express to do the wrong thing here, maybe we can pass a flag in which case Express would ignore "/"

@mstade

What I'm suggesting isn't that express do the wrong thing, just that it be a little bit clearer about what the client actually said it accepts. I think it's incorrect in saying that application/json is the best match to */*, because in that case */* is the best match. It's also a more useful return value, since then I can look at the return value and make a clever choice as to what to do next (i.e. is it an exact match or a wildcard?)

It's not lying, it still returns a truthy value to the question. Think of it like a conversation:

me: Does the client accept application/json?
express: Yes, because the client said it's fine with */* which matches application/json.

Rather than:

me: Does the client accept application/json?
express: Yes, the client accepts application/json.

See, infinitely more useful! :)

@tj
tj commented

well partial matches are still matches, image/* etc

@tj
tj commented

it already has a return value though, which is used internally for res.format()

@mstade

I think I'm being confusing, my apologies. I'm saying (I think) that partial matches are definitely matches, but that the return value from accept() is currently not very interesting. This because all it tells me is that the input is matched by the request's list of accepted types, but it doesn't actually tell me why. If it would return the best match from the request (image/* or */* or whatever) then not only will I know it was a match, I can also determine if it was partial by the use of a simple equality check.

Granted, the need for checking for exact matches is probably less than the need for checking for any match, but I don't think it's unwarranted. Having middleware that deals with one set of media types but calls next() for anything else, for instance.

@tj
tj commented

gotcha, yeah we may be able to alter the return value a bit. currently it's returning the match because for example you can do req.accepts('html, json') in which case it might return "json". res.format() uses this to determine which callback to invoke. I think that's still a useful return value but perhaps we can return an object or maybe a different method to see which pattern matches

@dougwilson
Collaborator

It sounds like @mstade would be interested in a method that would return all the media types as sent in Accept but order them by their precedence, possibly with the one at [0] being the most wanted.

req.accepts(); // Perhaps no args returns an ordered list as-is?
// Accept: text/*, text/html, text/html;level=1, */*
// -> ['text/html;level=1', 'text/html', 'text/*', '*/*']

// Accept: text/*;q=0.3, text/html;q=0.7, text/html;level=1, text/html;level=2;q=0.4, */*;q=0.5
// -> ['text/html;level=1', 'text/html', '*/*', 'text/html;level=2', 'text/*']

Perhaps something else they are looking for is a mime => quality mapping.

req.acceptsMap(); // ??
// Accept: text/*, text/html, text/html;level=1, */*
// -> {'text/html;level=1': 1, 'text/html': 1, 'text/*': 1, '*/*': 1}

// Accept: text/*;q=0.3, text/html;q=0.7, text/html;level=1, text/html;level=2;q=0.4, */*;q=0.5
// -> {'text/html;level=1': 1, 'text/html': 0.7, '*/*': 0.5, 'text/html;level=2': 0.4, 'text/*': 0.3]
@dougwilson
Collaborator

Ah, didn't know that. i.m.o req.accepted contains all the information possible, then, so that should solve the issues I've seen where people are talking about res.accepts(). If they wanted an ordered list from highest to lowest:

req.accepted.sort(function (a, b) { return b.quality - a.quality; }).map(function (a) { return a.value; });
@tj
tj commented

req.accepted is already sorted for you, but yeah it could be something more convenient built on top. To me */* is fine but I certainly can understand that since everything fires */* at you that it's maybe not ideal

@jonathanong

this issue seems to be solved, and express is doing the correct thing (hopefully). closing! you may also be interested in https://github.com/federomero/negotiator.

@kenperkins

EDIT: Lots of good information below. Take my comments with a grain of salt.

I'd really like to see this issue re-opened.

Here are the accepts headers for two requests in Chrome.

  • Ajax request via JQuery, expliciting setting Accept Header to application/json
    • Accept:application/json, text/javascript, */*; q=0.01
  • Fresh page request
    • Accept:text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8

Based on the current implementation of req.accepts, there's no way I can implement a smart route that correctly sends JSON when I want to, but HTML the rest of the time (assuming it's either */* or text/html in the HTML case).

I could see a couple ways to make this more flexible for devs:

  1. Allow an optional quality argument to req.accepts.
    • req.accepts('json', 0.5)
  2. Return the accepts header from req.accepts so that you can decide for yourself
if (req.accepts('json') && req.accepts('json').quality > 0.5 && req.accepts('json').type != '*') { 
  // handle for json case
}

Thoughts?

@kenperkins

I should have said:

There's no way I can implement a smart route without doing this logic myself. I.e. parse the req.accepted array and decide for myself. This seems like a pretty straightforward and basic use case though, so I'm inclined to believe it should behave as above for all parties without having to write accepted parsing logic.

@defunctzombie
Collaborator

I think what you want is the array form of the accepts api:

http://expressjs.com/api.html#req.accepts

req.accepts(['html', 'json']);

Which will return the one with the highest quality from an array you specify. This way you can say, I will be handling these two so tell me which one wins in quality.

@kenperkins

Thanks @defunctzombie, I see that this works, it still feels sloppy however:

if (req.accepts('html','json') === 'html') {
  // send html here
}
else if (req.accepts('html','json') === 'json') {
 // jsonify me
}
@defunctzombie
Collaborator

Depending on your default this could be simpler with less repeating

var accepted = req.accepts(['html', 'json']);
if (accepted === 'html') {
    ///
    return;
}

// default is json
@kenperkins

Yes, of course I could clean that up, it was more making the comment that I have to put in all of the possible header matches into the array. I guess this is just the tax of every fucking client on earth appearing to send */*.

@defunctzombie
Collaborator

Yea, everything sends */*. In reality most apps either respond with html or json so it isn't gonna be that much to handle. In my apps I actually split API servers/routes from HTML ones so don't even respect anything the browser requests and respond according to the function of the route/subsystem typically.

@jonathanong

Not sure how that's sloppy.accepts() is how content negotiation is supposed to work. Your method is much sloppier (unless I'm missing something) and does not honor the client's headers

@kenperkins

It's a function of the fact that basically every browser sends *.* with all of the requests. That's all I meant by sloppy.

@dougwilson
Collaborator

FYI @kenperkins jQuery is adding the */* when you use it (see https://github.com/jquery/jquery/blob/0d68b7877f761264bfe4950e4df156b854925a6b/src/ajax.js#L597). If you invoked XHR yourself, you can have it not include */* all you want.

@kenperkins

Really appreciate that legwork @dougwilson!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.