[PSR-7] UriInterface: rootless path, delimiters, consistency

[Tobion]

• support rootless path as continuation of Remove empty path normalization to support base path use-case #487
  • the path can now also be $request->withPath('rootless/foo')
  • it does not complicate anything and REMOVES the optional magic that was present that the path MAY be prefixed by / automatically
  • is more in line with RFC 3986 that also supports rootless paths, e.g. mailto:[email protected]
  • we need it for resolving URIs relative to a base path (another URI) as explained by @mtdowling in the ticket
• clarify delimiters that replaces [PSR-7] Scheme delimiters clarification #480
  • that getQuery() for http://example.org/path?foo=bar returns foo=bar (without ?) is clear and also documented. this is task of the code that splits a URI into its components, e.g. the constructor of Uri. there is no method defined for that in PSR-7
  • but addditionally, before there was magic defined that $request->withQuery('?foo=bar')->getQuery() returns foo=bar which makes no sense as I tried to explain in [PSR-7] Scheme delimiters clarification #480 (comment)
    • magic
    • error prone
    • slows down every operation unessarily
    • it must return %3Ffoo=bar instead
• consistent wording, e.g. sometimes there was an article sometimes not and it now also uses the same wording for the same concepts for each method
• scheme and host are case insensitive and normalized to lowercase which is important for usability allowing simple comparison, e.g. $request->getScheme() === 'http'

[weierophinney]

@Tobion — thanks!

My argument in #480 regarding the delimiters is this: the verbiage about delimiters exists to ensure that consumers know that they are receiving only the data for the component, without the delimiters. It was never about normalization; it was about ensuring consistency of return value, so that the consumer knows what they can do with it.

I don't want them to assume anything; I want them to know what the value is.

Other than those changes, I'm fine with this PR. But I think the delimiter specification is important for interface consumers.

@nyamsprod and @simensen — what are your thoughts?

[mtdowling]

Maybe this should instead link to the ABNF in RFC 3986? https://tools.ietf.org/html/rfc3986#appendix-A

[Tobion]

I can add @see https://tools.ietf.org/html/rfc3986#section-3.2 as we do in the other methods. Is that enough?

[Tobion]

But I think having [user-info@]host[:port] in the text is important for some users to understand it without following the link.

[weierophinney]

Agreed with @Tobion — it's nice to have it in the text so that the consumer does not need to look it up, but an @see annotation can be used to provide more information.

[Tobion]

@weierophinney yes that's what I figured. I didn't remove the verbiage about delimiters in the getters, see diff. So users don't have to assume things.
But what caused this delimiters discussion was the wording in the with methods, e.g. If the query string is prefixed by "?", that character MUST be removed. in withQuery. This means something completely else as I demonstrated above. I guess it just wasn't meant this way for implementations, but only for users as a reminder that the query does not include the delimiter itself.

[weierophinney]

@Tobion Okay, thanks for the clarification; missed that the verbiage was still in the getters.

Will review fully in a bit; we should be able to close #480 and merge this one later today. Thanks for making clear what your intentions were!

[mtdowling]

What's the purpose of this statement? Why must it be parseable by PHP's parse_str()? Shouldn't the only requirement be that it adheres to RFC 3986?

[Tobion]

I didn't add it and also wondered. It basically means that only a subset of query strings need to be supported. Because parse_str('=bar') doesn't work but would be a valid query string AFAIK.

[weierophinney]

This is why it's a SHOULD and not a MUST. Typically, for PHP, if it's not parseable by parse_str(), $_GET, and thus getQueryParams(), will be empty. However, since we are also targeting client-side usage, I think we can safely remove it.

[Tobion]

removed the sentence

[weierophinney]

Once @Tobion has made the last few requested changes, this one is ready to merge!

[mtdowling]

LGTM

[Tobion]

I added two commits that clarify some more things. Also added lowercase normalization of of scheme and host as we already agreed on in #480

[Tobion]

Now that we have rootless path support as well, there is nothing that hinders UriInterface to be used generally. It's just that it doesn't define all possible methods to work with URIs, e.g. URI comparision etc. I hope I clarified that while also giving arguments why it's in the Http/Message namespace.

[weierophinney]

👍

@pmjones and/or @simensen — ready to merge!