jhermsmeier / node-http-link-header

Parse & format HTTP link headers according to RFC 8288
MIT License
39 stars 12 forks source link

URI relations should not be lowercased #22

Closed Vinnl closed 4 years ago

Vinnl commented 4 years ago

The rel can be a URI:

The ABNF for the rel and rev parameters’ values is:

relation-type *( 1*SP relation-type )

where:

relation-type  = reg-rel-type / ext-rel-type
reg-rel-type   = LOALPHA *( LOALPHA / DIGIT / "." / "-" )
ext-rel-type   = URI ; Section 3 of [RFC3986]

https://httpwg.org/specs/rfc8288.html#header-type

Looking at that RFC3986, while the scheme and the host are case insensitive, the other parts can be case sensitive, depending on the scheme used:

When a URI uses components of the generic syntax, the component syntax equivalence rules always apply; namely, that the scheme and host are case-insensitive and therefore should be normalized to lowercase. For example, the URI HTTP://www.EXAMPLE.com/ is equivalent to http://www.example.com/. The other generic syntax components are assumed to be case-sensitive unless specifically defined otherwise by the scheme (see Section 6.2.3).

https://tools.ietf.org/html/rfc3986#section-6.2.2.1

However, when encountering a rel with non-lowercase characters in the other components, http-link-header converts them to lowercase:

const httpLinkHeader = require("http-link-header")

const links = httpLinkHeader.parse('<https://some.url>; rel="http://some.rel/caseSensitive"');

console.log(links.refs);
// => [Object {rel: "http://some.rel/casesensitive", uri: "https://some.url"}]

https://codesandbox.io/s/charming-tdd-syip6

(Apologies if I misinterpreted any of the above RFCs.)

jhermsmeier commented 4 years ago

Thanks! I don't have time to read into this right now, but looks like a bug to me at first glance. I'll check this out in depth later!

jhermsmeier commented 4 years ago

Turns out that according to rfc8288#section-2.1.2 extension rels should be lowercase, they don't have to be – they only have to be compared as lowercase strings. So there's definitely a bug here.

Vinnl commented 4 years ago

Wow, that was quick - and even resolved my other issue as well. Thanks @jhermsmeier!