Bug 1037753 - [GSS] (6.3.0) Chosen variant is not always the best match
[GSS] (6.3.0) Chosen variant is not always the best match
Status: CLOSED CURRENTRELEASE
Product: JBoss Enterprise Application Platform 6
Classification: JBoss
Component: RESTEasy (Show other bugs)
6.1.1
Unspecified Unspecified
unspecified Severity unspecified
: ER1
: EAP 6.3.0
Assigned To: Weinan Li
nobody nobody
Scott Mumford
:
Depends On:
Blocks: 1039135 1063653
  Show dependency treegraph
 
Reported: 2013-12-03 13:19 EST by Kyle Lape
Modified: 2014-06-29 18:22 EDT (History)
8 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
In previous versions of JBoss EAP 6, it was found that RESTEasy, while adhering to specification RFC 2616, did not always return the most appropriate media handler in instances where quality factors were equal but specificity was different. For instance, when given an Accept header of `application/json, */*` and variant values of `["application/xml","application/json"]`, RESTEasy's `Request.selectVariant()` would choose `application/xml` over `application/json`. In this release, specific Accept header values take precedence over less specific variant matches with the same quality value (if both have q=1.0 or q=0.5 for example).
Story Points: ---
Clone Of:
: 1063653 (view as bug list)
Environment:
Last Closed: 2014-06-28 11:41:28 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)


External Trackers
Tracker ID Priority Status Summary Last Updated
JBoss Issue Tracker RESTEASY-994 Major Closed Chosen variant is not always the best match 2014-07-24 03:26:01 EDT

  None (edit)
Description Kyle Lape 2013-12-03 13:19:29 EST
Given the Accept header:

  Accept: application/json, */*

And the set of variants:

  ["application/xml","application/json"]

request.selectVariant(...) will choose application/xml. Assuming variant selection should follow the HTTP protocol behavior for the Accept header, this should not be the case:

  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. HTTP spec, section 14.1 [1]

[1] http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.1
Comment 1 JBoss JIRA Server 2013-12-03 19:37:29 EST
Ron Sigal <ron.sigal@jboss.com> made a comment on jira RESTEASY-994

Hey Bill,

I have no problem with believing Resteasy SHOULD pick the most specific media type.  I just don't believe anything says it MUST do so.

In any case, having demonstrated my willingness to defend Resteasy to the death, I'll be glad to change it.  ;-)

I'll look into the master branch as well.

Erik, if you want to pursue a change to the JAX-RS spec, you could try this mailing list: issues-request@jax-rs-spec.java.net

-Ron
Comment 2 JBoss JIRA Server 2013-12-03 20:40:10 EST
Ron Sigal <ron.sigal@jboss.com> made a comment on jira RESTEASY-994

Given 

{code}
Accept: */*, application/json;q=0.5
{code}

and available media types "application/json" and "application/xml", who thinks Resteasy should return "application/json"?

Just asking.
Comment 3 JBoss JIRA Server 2013-12-03 20:43:41 EST
Ron Sigal <ron.sigal@jboss.com> made a comment on jira RESTEASY-994

Given 

{code}
Accept: text/*, application/json;q=0.5
{code}

and available media types "application/json" and "text/xml", who thinks Resteasy should return "application/json"?

Just asking.
Comment 4 JBoss JIRA Server 2013-12-03 21:32:58 EST
Kyle Lape <kyle.lape@redhat.com> made a comment on jira RESTEASY-994

I'd say {{text/xml}} should be chosen since the client is explicitly stating that it prefers that media range.
Comment 5 JBoss JIRA Server 2013-12-03 22:07:35 EST
Erik Mattheis <ironduck@mac.com> made a comment on jira RESTEASY-994

I agree. RFC 2616 section 14.1 says:
{quote}
The media type quality factor associated with a given type is determined by finding the media range with the highest precedence which matches that type.
{quote}
So {{text/xml}} would default to {{q=1.0}} and {{application/json}} would get {{q=0.5}}. We don't need to consider specificity to disambiguate two equal quality factors in this case.
Comment 6 JBoss JIRA Server 2013-12-04 00:28:04 EST
Ron Sigal <ron.sigal@jboss.com> made a comment on jira RESTEASY-994

1. With respect to Request.selectVariant(), governed by the HTTP spec, I agree.  The point I wanted to make is that the selection is based on the quality factor.

2. With respect to @Produces, governed by the JAX-RS spec, 

{code}
M = { S("text/*", "text/xml"), S("application/json;q=0.5", "application/json") }
   = { "text/xml;q=1.0", "application/json;q=0.5" }
{code}

with the ordering  "text/xml;q=1.0" > "application/json;q=0.5", and we get "text/xml" again.

Now, how about 

{code}
Accept: application/*;q=1.0, application/json;q=0.5
{code}

with available media types "application/json" and "application/xml"?
Comment 7 JBoss JIRA Server 2013-12-04 08:22:00 EST
Bill Burke <bill@jboss.org> made a comment on jira RESTEASY-994

Now, how about

Accept: application/*;q=1.0, application/json;q=0.5

with available media types "application/json" and "application/xml"?

application/xml should be picked as it has a higher qualifier.  Please nothing special should change for the algorithm beyond more specific mapping for equal qualifiers.
Comment 8 JBoss JIRA Server 2013-12-04 08:23:27 EST
Bill Burke <bill@jboss.org> made a comment on jira RESTEASY-994

Also, any changes to 3.x should be done in a branch and tested against the TCK.  The TCK has become quite sensitive to how Jersey has decided to implement things.
Comment 9 JBoss JIRA Server 2013-12-04 10:51:58 EST
Erik Mattheis <ironduck@mac.com> made a comment on jira RESTEASY-994

So we agree that servers in general SHOULD choose a media type that matched a more specific range in the event that multiple available types are determined to have the highest quality factor. I think I see how a strict interpretation of RFC 2616 would lead to the algorithm specified in JAX-RS, but I don't think it produces the result that user-agents expect 'in the wild'. Without deviating from the specified algorithm, I don't see how RESTEasy can do what we want, so I'm wondering whether RESTEasy can pass the TCK (now and in the future) if selectVariant is changed to match the actual type-selection behavior RESTEasy uses to fulfill requests.
Comment 10 JBoss JIRA Server 2013-12-04 15:09:19 EST
Ron Sigal <ron.sigal@jboss.com> made a comment on jira RESTEASY-994

Ok, I think we're beginning to be on the same page.

1. Erik, I didn't realize until you said "We don't need to consider specificity to disambiguate two equal quality factors in this case" that you are thinking of specificity of media ranges only in the context of breaking ties.  I don't see that in the HTTP spec, but it seems reasonable.  In fact, I read that the Apache web server adds q=.02 to media ranges like "application/\*" in the absence of an explicit quality factor, and it adds q=.01 to "\*/\*".  It seems that they're going beyond the spec to make things work the way everyone thinks they should.

2. Note that, given 

{code}
Accept: application/*;q=1.0, application/json;q=0.5
{code}

and

{code}
@Produces({ "application/json", "application/xml" })
{code}

we get

{code}
M = { S("application/*;q=1.0", "application/json"), S("application/*;q=1.0", "application/xml"), S("application/json;q=0.5', 'application/json") }
= { "application/json;q=1.0", "application/xml;q=1.0", "application/json;q=0.5" }
{code}

which can be ordered

{code}
"application/json;q=1.0" >= "application/xml;q=1.0" >= "application/json;q=0.5"
{code}

or

{code}
"application/xml;q=1.0" >= "application/json;q=1.0" >= "application/json;q=0.5"
{code}

so that either "application/json;q=1.0" or "application/xml;q=1.0" could be returned legally.  I think that's a problem.
Comment 11 JBoss JIRA Server 2013-12-04 15:11:48 EST
Ron Sigal <ron.sigal@jboss.com> made a comment on jira RESTEASY-994

Bill, do you know where the JAX-RS TCK lives?  I don't see it on https://jax-rs-spec.java.net/.
Comment 12 JBoss JIRA Server 2013-12-04 15:30:46 EST
Ron Sigal <ron.sigal@jboss.com> made a comment on jira RESTEASY-994

Ah, looks like the TCK is here: https://java-partner.sun.com/.
Comment 13 JBoss JIRA Server 2013-12-04 16:20:28 EST
Erik Mattheis <ironduck@mac.com> made a comment on jira RESTEASY-994

Ron - I think we're definitely speaking the same language, now. I was always approaching the issue from the perspective of two acceptable types with the same quality factor but differing specificity since that was my problem case. The HTTP spec doesn't appear to directly address this scenario so it's open to interpretation. After seeing the JAX-RS algorithm I see where you're coming from. I was expecting something along the lines of:

# sort types from Accept header by quality factor then specificity
# sort types from \@Produces by quality factor then specificity
# try to find a compatible type from \@Produces for each type in Accept and the first match wins

Obviously the JAX-RS spec has to deal with things that aren't an issue in my code such as non-concrete ranges in \@Produces, but I still think it misses the mark a little. Anyway, i think we're on the right track and I'm interested to see what you find. Thanks for the patience!
Comment 14 JBoss JIRA Server 2013-12-04 16:23:08 EST
Erik Mattheis <ironduck@mac.com> made a comment on jira RESTEASY-994

I'm still not clear what part of RFC 2616 section 14.1 you think specifies server behavior, but I've read JAX-RS 1.0 section 3.8 and I understand your concern now. The algorithm presented does not match my interpretation of RFC 2616 section 14.1 because the JAX-RS algorithm does not attempt to satisfy the acceptable media types in order of precedence. Instead, it compiles the complete set of potential matches and sorts them by specificity and q-value - this was the part I did not understand earlier, hence my confusion. I think the flaw here is that user-agent precedence is lost in the final sort. There is no indication that application/json;q=1.0 matched application/json (more specific) and application/xml;q=1.0 matched \*/\* (less specific). I think that RFC 2616 section 14.1 is clear that 'Accept: application/json, \*/\*' should be interpreted as 'I prefer application/json but I will accept anything if application/json is not available'. I would change steps 4 and 5 of the JAX-RS algorithm as follows:

{quote}
4. Obtain the acceptable media types _A_, sorted in descending order, with a primary key of q-value and a secondary key of specificity (n/m > n/\* > \*/\*). If _A_ = \{\}, set _A_ = \{‘\*/\*’\}
5. Set _M_ = \{\}. For each member of _A_,_a_:
* For each member of _P_,_p_:
** If _a_ is compatible with _p_, produce candidate type _c_ as _S(a,p)_, where the function _S_ returns the most specific media type of the pair with the q-value of _a_.
*** If _c_ is a concrete type, set _M_~selected~ = _c_, finish.
*** Else add _c_ to _M_
{quote}

Obviously I have not considered much beyond my current problem and I haven't done any testing, so I think this needs to be debated in a larger audience, or I just need some more prodding to see the light.
Comment 15 JBoss JIRA Server 2013-12-05 00:43:57 EST
Ron Sigal <ron.sigal@jboss.com> made a comment on jira RESTEASY-994

I just sent the following email to jsr339-experts@jax-rs-spec.java.net:

{noformat}
We've had a lengthy discussion about media type negotiation on https://issues.jboss.org/browse/RESTEASY-994, and I have a question.

The discussion started with the observation by a user, Erik Mattheis, that, given

  Accept: application/json, */*

the result returned by

   @GET
   @Path("automatic")
   @Produces({ APPLICATION_JSON, APPLICATION_XML })
   public Holder automatic()
   {
      return ...
   }

is in JSON format, but

   @GET
   @Path("variant")
   @Produces({ APPLICATION_JSON, APPLICATION_XML })
   public Holder variant()
   {
      List<Variant> XML_OR_JSON = Variant.mediaTypes(APPLICATION_XML_TYPE, APPLICATION_JSON_TYPE).build();
      System.out.println("selected variant: " + request.selectVariant(XML_OR_JSON));
   }

prints

   selected variant: Variant[mediaType=application/xml, language=null, encoding=null]

Now, assuming that Request.selectVariant() is governed by http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.1, according to which "application/json" and "application/xml" both have an implicit quality factor of 1.0, and given that http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.1 has no discussion of the breaking of ties, I believe that Resteasy isn't violating the spec when Request.selectVariant() chooses "application/xml".  On the other hand, it seems REASONABLE to select "application/json" instead.  Does that make sense?

Pursuing the matter further, I have a concern about the algorithm given in Section 3.8 "Determining the media type of responses".  Given

   Accept: application/*;q=1.0, application/json;q=0.5

and

   @Produces({ "application/json", "application/xml" })

in Step 6 we get

  M = { S("application/*;q=1.0", "application/json"), S("application/*;q=1.0", "application/xml"), S("application/json;q=0.5', "application/json") }
    = { "application/json;q=1.0", "application/xml;q=1.0", "application/json;q=0.5" }

which can be ordered

  "application/json;q=1.0" >= "application/xml;q=1.0" >= "application/json;q=0.5"

or

  "application/xml;q=1.0" >= "application/json;q=1.0" >= "application/json;q=0.5"

so that either "application/json;q=1.0" or "application/xml;q=1.0" could be returned legally.  That seems to be in contradiction to the mandate in http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.1 to use quality factors to select a media type.  In particular, "application.json" should have a quality factory of 0.5.

Any comments?

Thanks,
Ron
{noformat}
Comment 16 JBoss JIRA Server 2013-12-05 13:15:05 EST
Ron Sigal <ron.sigal@jboss.com> made a comment on jira RESTEASY-994

My email to jsr339-experts@jax-rs-spec.java.net bounced, so I sent it to users@jax-rs-spec.java.net.
Comment 17 JBoss JIRA Server 2013-12-07 01:26:13 EST
Ron Sigal <ron.sigal@jboss.com> made a comment on jira RESTEASY-994

Hmmm.  Which is more specific:  "application/json" or "*/*;q=0.5,a=1;b=2"?

I would think the former, but I don't see an answer anywhere.  Anyway, ServerDrivenNegotiation.getBestMatch() assigns Variant "application/json;a=1;b=2" a quality factor of 0.5 instead of 1.0. 

Just another little issue.
Comment 18 JBoss JIRA Server 2013-12-07 01:27:14 EST
Ron Sigal <ron.sigal@jboss.com> made a comment on jira RESTEASY-994

Hmmm.  Which is more specific:  "application/json" or "\*/\*;q=0.5,a=1;b=2"?

I would think the former, but I don't see an answer anywhere.  Anyway, ServerDrivenNegotiation.getBestMatch() assigns Variant "application/json;a=1;b=2" a quality factor of 0.5 instead of 1.0. 

Just another little issue.
Comment 19 JBoss JIRA Server 2013-12-07 16:58:16 EST
Ron Sigal <ron.sigal@jboss.com> made a comment on jira RESTEASY-994

Santiago Pericas-Geertsen posted the following response to users@jax-rs-spec.java.net:

Hello Ron,

 We spent some time evaluating our algorithm and found that it returns the expected result in the majority of cases. This one is a bit tricky because there's an overlap between the accept type patterns, and that typically results in ambiguity and less than ideal results. 

 If anyone has a suggestion on how to improve the algorithm (while keeping it simple!), we should file a JIRA for an update release. Designing a simple algorithm that will result in the expected results for all possible cases (including the one in question) isn't easy, but there's always room for improvement, of course.

 Thanks.

-- Santiago
Comment 20 JBoss JIRA Server 2013-12-07 17:01:08 EST
Ron Sigal <ron.sigal@jboss.com> made a comment on jira RESTEASY-994

And I answered:

{noformat}
Hi Santiago,

Thank you for your response.

I was surprised that the algorithm in Section 3.8 of the JAX-RS spec seems to be inconsistent with the algorithm implicit in http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.1, which I described on RESTEASY-994 as

    1. Let PO = the partial order on accepted media types induced by "is less specific than".
     
    2. Add "dummy/dummy;q=0" as the minimal element in PO, where "dummy/dummy;q=0" is a wildcard like "*/*"

    3. For each M in available media types:
          Let Q_M = max { q  |  q is the quality factor of a maximal element in PO }

    4. Let Q = max { Q_M  |  M in available media types }

    5. Let MT = { M | M is an available media type such that M_Q = Q }

    6. If no element in MT matches an accepted media type, return a "406" response.

    7. Return an arbitrary element of MT

Actually, there are a couple of gaps in http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.1:


1. There is no mention of tie breaking in the case that multiple available media type are assigned the same quality factor.  I think that the consensus on RESTEASY-994 is that the available media type that matches the more specific requested media range should be given preference.  For example, given

   Accept: application/json, */*

application/json and application/xml would both have a quality factor of 1.0, but application/json should be preferred.  Step 7 could be changed to implement tie breaking based on specificity.


2. There is no precise definition of "more specific".  For example, I would expect application/json to be more specific that */*;a=1;b=2;c=3, but Resteasy currently prefers the latter because it counts the number of parameters.  I think that should be changed with the ordering defined:

   media type 1 is more specific than media type 2 if and only if

   * media type 1 has a non wildcard type and media type 2 has a wildcard type, or
   * media type 1 has a non wildcard subtype and media type 2 has a wildcard subtype, or
   * media type 1 has more parameters than media type 2


3. It's a minor thing, but there's no suggestion for what to do with

   Accept: application/json, application/xml, application/json;q=0.5


Anyway, the way things are, Request.selectVariant() is governed by http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14 (I guess), and negotiation based on the @Produces annotation is governed by Section 3.8 of the JAX-RS spec, and I believe they can legally return different results.  In particular, the JAX-RS algorithm doesn't have a notion of basing the quality factor of an available media type on the most specific matching requested media range.  For example, given the somewhat artificial case

   Accept; application/json;q=0.5, application/*

and

   @Produces("application/json, application/xml")

the HTTP algorithm would assign a quality factor of 0.5 to application/json because application/json;q=0.5 is more specific than application/*, but the JAX-RS algorithm gives

   M = { S("application/json;q=0.5", "application/json"), S("application/*", "application/json"), S("application/*", "application/xml")
     = { application/json;q=0.5, application/json;q=1.0, application/xml;q=1.0 }

where

   application/json;q=1.0 >= application/xml;q=1.0 > application/json;q=0.5

and
  
   application/xml;q=1.0 >= application/json;q=1.0 > application/json;q=0.5
 
are both legal orderings, so that either application/json or application/xml could be returned.

-Ron
{noformat}
Comment 21 JBoss JIRA Server 2013-12-07 17:08:23 EST
Ron Sigal <ron.sigal@jboss.com> made a comment on jira RESTEASY-994

Hi Erik,

I was thinking about your proposed change to the JAX-RS algorithm, and I don't think it's quite right.  Given

{code}
   Accept: application/json;q=0.5, application/*
{code}

and

{code}
   @Produces("application/json, application/xml")
{code}

you would order the acceptable media ranges as follows:

{code}
   application/* > application/json;q=0.5
{code}

and Step 5 would return (assuming it looks at the available media types from left to right) application/json.
Comment 22 JBoss JIRA Server 2013-12-07 17:38:17 EST
Bill Burke <bill@jboss.org> made a comment on jira RESTEASY-994

Should be application/xml

Can we not overthink this please?  I don't see why this is so hard to understand and why we need 100 comments on this.
Comment 23 JBoss JIRA Server 2013-12-07 18:55:49 EST
Ron Sigal <ron.sigal@jboss.com> made a comment on jira RESTEASY-994

Just being anal retentive.  ;-)

Seriously, though, I think there are two pieces.

1. I think I know how Request.selectVariant() should work and how to fix it.  That's why this issue was created, so end of story as far as that goes.

2. I don't think the algorithm in Section 3.8 of the JAX-RS spec is consistent with the HTTP spec, so that's what I've been talking about lately.
Comment 25 JBoss JIRA Server 2014-01-24 16:59:23 EST
Bill Burke <bill@jboss.org> updated the status of jira RESTEASY-994 to Closed
Comment 28 Weinan Li 2014-01-28 10:20:51 EST
I'll handle this after back from holiday.
Comment 31 Petr Sakař 2014-04-08 05:17:37 EDT
Verified for EAP 6.3.0.ER1 using VariantTest test running on Arquillian
Comment 32 Scott Mumford 2014-04-23 23:38:36 EDT
Added the beginning of a release note draft but I'm having trouble parsing this discussion.

Any feedback would be greatly appreciated, particularly in regards to what/how RESTEasy was improved as a result of this issue.
Comment 33 sgilda 2014-05-12 16:02:15 EDT
Changed <literal></literal> tags in Doc Text to ticks (`) to fix Bug 1096865

Note You need to log in before you can comment on or make changes to this bug.