Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

q-value choice on wildcards ignores default q-value of 1 #771

Merged
merged 3 commits into from
Jun 12, 2015
Merged

q-value choice on wildcards ignores default q-value of 1 #771

merged 3 commits into from
Jun 12, 2015

Conversation

kevinpeno
Copy link
Contributor

See #769

Server responds with highest priority q-value when wildcards are used, but ignores setting the default q-value to 1 for mime-types without q-value specified.

Example:
Client: */*
Server: application/json, text/plain; q=0.9, application/octet-stream; q=0.2
Result: text/plain
Expected: application/json

Verified

This commit was signed with the committer’s verified signature.
adamrusted Adam Rusted
…s fail to choose best mime when wildcards are used is sent
@yunong
Copy link
Member

yunong commented Mar 10, 2015

@kevinpeno For some reason I only see the tests as part of this commit, and not the actual restify changes. Are we missing files from the commit? Also, could you please prefix the test with the issue #? e.g. '#771 qvalue...'

@kevinpeno
Copy link
Contributor Author

@yunong I didn't include a fix per our convo (not sure where to apply it). Any pointers on ideas/where to apply the fix is welcome. I'll update to add the issue number to the test.

@kevinpeno
Copy link
Contributor Author

Unfortunately I run windows at home and am having issues running npm test so I'll take a look at both a fix and fixing the test name tomorrow.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@kevinpeno
Copy link
Contributor Author

@yunong updated the test per request and added code to make the test pass. I made q-value default to 1 per RFC 2616 sec 14 (emphasis added):

Each media-range MAY be followed by one or more accept-params, beginning with the "q" parameter for indicating a relative quality factor. The first "q" parameter (if any) separates the media-range parameter(s) from the accept-params. Quality factors allow the user or user agent to indicate the relative degree of preference for that media-range, using the qvalue scale from 0 to 1 (section 3.9). The default value is q=1.

This is very likely a breaking change as people may have used this bug as a work around for q-value ordering

@micahr
Copy link
Member

micahr commented Jun 5, 2015

Thank you for the PR @kevinpeno . Can you fix up the merge conflicts so this can be integrated?

@kevinpeno
Copy link
Contributor Author

Sure thing. I'll work on that today. Sorry for the delay!

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@kevinpeno
Copy link
Contributor Author

@micahr All merged.

@micahr micahr added the Bug label Jun 10, 2015
yunong added a commit that referenced this pull request Jun 12, 2015

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
q-value choice on wildcards ignores default q-value of 1
@yunong yunong merged commit 37377f3 into restify:master Jun 12, 2015
@kevinpeno kevinpeno deleted the accept-header-any-q-value branch July 2, 2015 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants