use sorted order of provided content-types in negotiation

[ahmadnassri]

been chasing this all the way from express to jshttp/accepts...

this "feature" (sorting by provided array index) was originally introduced by @dougwilson in 6fb679f4d836f579da5ac4b582b27e7ff57b6748 but doesn't seem to have ever worked, and no related tests for mimeType.js where language.js has one

[dougwilson]

Thank you!

[dougwilson]

So I looked at this and this "fix" is not related to the commit you referenced; the commit you references was to add support to keep it in the same order as the client specified; your change here changes to keep it in the same order the server specified. I need to look more into this.

[dougwilson]

I'm still investigating :)

[dougwilson]

Ok. So I added the media type tests that should have been included in the commit you referenced: 1d6d0b2e53fd749f670c8a4b0bc68b91ff14c182

It definitely looks like you're asking for a new feature, rather than a bug fix. I don't have anything against the feature request, but we just need to get that feature into all 4 of the negotiations :)

[dougwilson]

Besides from needing to get this feature in the other three, I don't see any reason not to have this feature :) I'll have to think about how this change will impact upstream modules and existing code using negotiations.

[ahmadnassri]

@dougwilson my commit was at 2AM so brain juices were running low, perhaps i didn't describe/read things properly :)

• you're correct, this is technically a feature, however looking at the real-world implementations of this, for example in Express it is not very clear to what the behavior is when client accepts multiple values of equal weight AND the server has no weight values:

  res.format(object) Performs content-negotiation on the Accept HTTP header on the request object, when present. It uses req.accepts() to select a handler for the request, based on the acceptable types ordered by their quality values. If the header is not specified, the first callback is invoked. When no match is found, the server responds with 406 "Not Acceptable", or invokes the default callback.

• so, in short: currently, there is no clear, traceable path of the decision tree to select a matching value in the case of no, or equal weights.

• my test shows this flaw, when negotiating with Accept: */* and

  var mediaTypes = [
  'text/javascript',
  'application/javascript',
  'application/x-javascript',
  'text/json',
  'text/x-json',
  'application/json',
  'application/x-json',
  'text/xml',
  'application/xml',
  'application/x-xml',
  'text/html',
  'application/xhtml+xml',
  'text/yaml',
  'text/x-yaml',
  'application/yaml',
  'application/x-yaml',
  'text/plain'
  ];

  I get the result application/xml as the preferred match, which seems random.

  in fact, the un-intended consequences of this in Express is changing the order of the object properties in res.format() will result in different priority matching based on that order!

• I actually read this test as the same use-case i'm describing, hence referencing that particular commit. that was wrong :)
• RFC7231 does not reference anything in regards to how the server should prioritize equal weighted values ... I'm actually considering this as a worthwhile use case to make a suggestion there as well ...

[dougwilson]

I'm not saying that I won't do this and I said above I think this is a good idea, so you don't need to justify it :)

[ahmadnassri]

@dougwilson no worries, I got that, I just felt like clarifying for future readers, especially since the premise of this being a bug I set first was wrong :) :+1:

[dougwilson]

Sorry it took so long for me to look into this one and for all :( This is indeed a bug. I originally mis-understood some of what you wrote, and I'm sorry. The behavior you desire works until the array gets too long and v8 changes from a stable sort to an unstable sort algorithm, is what's happening. We originally only fixed one side of the sort (the client side); this is a bug because we never fixed the server size of the sort to stay stable.

[ahmadnassri]

ah, the V8 behavior is something I didn't consider.

thank you for continuing to look into this.

[dougwilson]

Ok, so hopefully this is fixed on master now; if you want to give feedback before a release, please feel free to test and confirm :)!