Fix parsing complex accept headers #2603
Conversation
| if aq == bq { | ||
| return .orderedSame | ||
| } else if aq > bq { | ||
| return .orderedAscending | ||
| } else { | ||
| return .orderedDescending | ||
| } else { | ||
| return .orderedAscending |
There was a problem hiding this comment.
This was twisted - as aq is the left hand side input parameter the return value should be .orderedDescending when larger than the right hand side.
| self.pop() | ||
| } else if let comma = self.current.firstIndex(of: .comma) { | ||
| value = self.pop(to: comma) | ||
| } else if let separatorMatch = self.firstIndex(matchingAnyOf: .comma, .semicolon) { |
There was a problem hiding this comment.
Instead of always preferring semicolon first we also check if there are comma along the way.
|
|
||
| // Simple accept type | ||
| do { | ||
| headers.replaceOrAdd(name: .accept, value: "text/html") |
There was a problem hiding this comment.
Just for complete sanity and to avoid future regressions could you add an assert to these tests that ensures that the accepted media types match the ones in the header. E.g.
XCTAssertTrue(headers.accept.mediaTypes.contains(.html))There was a problem hiding this comment.
Thanks for the feedback. I've added some cases + removed the catchall from the second, more complex test to allow testing for concrete types…
…or into bugfix/complex-accept-headers
|
Original comment: Fixes #2283 This PR fixes an issue that parsing more complex accept headers like text/html,application/xhtml+xml,application/xml;q=0.9,/;q=0.8 failed. When adding unit tests for those more complex accept headers I realized that also the logic in HTTPMediaTypePreference.swift for comparing two media types is incorrect. I fixed this along the way and also added a test for this |
|
These changes are now available in 4.44.1 |
Fixes an issue when parsing complex
Acceptheaders, e.g.text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8The accept header should now be correctly parsed and all accepted types available. Also fixes an issue inHTTPMediaTypePreferencewhen comparing two media types. (#2603)Fixes #2283