PSR-7 Errata discussion

[Larry Garfield]

(Starting new thread)

I'd not seen the PR itself before now. However, I must object to it,
and this vote.

Errata belong in the metadoc. Period. They should not be modifying the
spec itself.

Per bylaw, the only acceptable amendments are:

1) Marking it deprecated.

2) Tweaking a dependency (eg, s/PSR-0/PSR-0 or PSR-4/)

3) Annotations (ie, adding a link TO the errata section in the metadoc,
where relevant)

4) Formatting and typos.

See: http://www.php-fig.org/bylaws/psr-amendments/

This PR:

1) adds an errata section to the metadoc (which is fine, and I've no
objection to it)

2) tweaks some capitalization (covered under formatting and typos)

3) changes "hostname" to "host" in the body of the spec (is that a typo?
Debatable)

4) Removes a @throw statement from a method (wait, what?)

5) Adds 7 more @throw statements

6) Removes a considerable amount of text from __toString()'s docblock
that provides guidance about how to handle complex cases, but that text
does not, as far as I can see, get replaced elsewhere.

Points 1-3 are fine for errata. Points 4-6 are not. They would also
necessitate a new version of the interface package as it changes the
exceptions that may be thrown in different places.

While I have no objection to the clarifications in the errata section,
this diff is too invasive under current bylaws and should not be allowed.

--Larry Garfield

[Paul Jones]

> On Jun 3, 2016, at 08:55, Larry Garfield <la...@garfieldtech.com> wrote:
>
> (Starting new thread)
>
> I'd not seen the PR itself before now.

Must I remind the secretaries that there is, by custom, a 2-week discussion before votes? If the "self-throttling" guidelines are unofficial but enforced, then the 2-week discussion period is likewise unofficial but should be enforced.

I ask that the secretaries rescind the call-for-votes, and begin a thread for the discussion of the measure.


--

Paul M. Jones
http://paul-m-jones.com

[Michael Cullum]

Honestly, I'd missed the PR affected the spec as well and just seen the errata in the meta doc.

The vote is for the errata in the meta document alone, not the changes to the specification. I'll ask Tobion to split these off.

--

Michael C

--Larry Garfield

--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.
To post to this group, send email to php...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/57518C61.1080608%40garfieldtech.com.
For more options, visit https://groups.google.com/d/optout.

[Michael Cullum]

Paul,

Sorry, I replied fast to Larry's email so missed yours.

This had been pending for a while, but as I've had to pull the vote now, I'll leave this discussion open for a while in case there was further discussion to be had. My belief was the 2 week period applied to policy changes (inc. bylaw changes and membership votes) and review stage (before acceptance votes) but was not required by all votes (Precedent has been for some other votes such as entrance votes, errata etc. to not have a formal two week discussion period). I apologise although this emphasises now is probably a good time to get something in the bylaws about this though so that everyone is on the same page. If voting members would prefer to leave this discussion open for two weeks, then I will delay any votes until then.

--

Thanks,

Michael

[Paul Jones]

> On Jun 3, 2016, at 09:17, Michael Cullum <m...@michaelcullum.com> wrote:
>
> I apologise

Accepted.

> although this emphasises now is probably a good time to get something in the bylaws about this though so that everyone is on the same page.

I agree. The fewer "customs and conventions" that are arbitrarily enforced/not-enforced, the better.

[Tobion]

> 3) changes "hostname" to "host" in the body of the spec (is that a typo? 
Debatable) 

If you think this is debatable then the whole PSR-7 is debatable because it already used "host" in all other occurences.

So if you argue the meaning "hostname" != "host" then PSR-7 is contradicting itself making not implementable.

> 4) Removes a @throw statement from a method (wait, what?) 

Where? Only the descriptions have been generalized but nothing removed.

> 5) Adds 7 more @throw statements 

I only added it to `withFragment` to be consistent with all the other "with*" methods.

I can remove that again if it's better to keep PSR-7 inconsistent.

Btw, diactoros for example already throws an exception there.

So if we do not change it, there is probably no conforming PSR-7 implementation available currently.

> 6) Removes a considerable amount of text from __toString()'s docblock 
that provides guidance about how to handle complex cases, but that text 
does not, as far as I can see, get replaced elsewhere. 

This part is irrelevant as it has been discussed before and as I also mentioned in the rationale of the errata.

This cases it covers shouldn't happen in the first place (and it doesn't even cover every case anyway).

So if we keep it, it would just cause confusion.

For to me all those changes in the spec are covered by "formatting and typo" because it doesn't change the meaning.

The UriInterface can already be implemented the way we want to clarify with the errata without violating the current spec.

So there is not BC break by changing this.

[Dracony]

To be fair, the errata does make sense. We have to put those changes somewhere, and there aren't enough of them to warrant a separate PSR, imho. So I'm for the merge from a personal level, and it won't hurt my project since it doesn't require any change in the implementation. So that is where my +1 comes from.

But I would really like to avoid changing PSRs in the future.

[Tobion]

I added a new commit to https://github.com/php-fig/fig-standards/pull/756 to use a different example for invalid URI modification.

According to RFC 3986 the authority can actually have a userinfo or port without a host (which is quite surprising).

It would not be valid according to RFC 7230 Section 2.7 but UriInterface is not supposed to know the individual requirements per scheme.

So that example was not really fitting.

[ignace nyamagana]

The UriInterface interface in PSR-7 is primaly defined for the HTTP/HTTPS schemes so it is supposed to know at least the requirement for these specific schemes. That being said I may have misread the RFC but can you link me to the passage in the RFC that says that an host component of an authority is not required ?

[Tobion]

https://tools.ietf.org/html/rfc3986#appendix-A

authority     = [ userinfo "@" ] host [ ":" port ]

host          = IP-literal / IPv4address / reg-name
reg-name      = *( unreserved / pct-encoded / sub-delims )

As you can see reg-name can be empty which means the host can be empty which means
"urn://user:pass@:123/path" is a valid URI. The reasoning is also given:

https://tools.ietf.org/html/rfc3986#section-3.2.2
> If the URI scheme defines a default for host, then that default
   applies when the host subcomponent is undefined or when the
   registered name is empty (zero length).  For example, the "file" URI
   scheme is defined so that no authority, an empty host, and
   "localhost" all mean the end-user's machine, whereas the "http"
   scheme considers a missing authority or empty host invalid.

Unfortunately parse_url fails to parse that, but the regex from RFC 3986
can parse it: https://3v4l.org/Nv1ml

[ignace nyamagana]

Thanks for the clarification. In regards to parse_url this is just one of the multiple issue with the function. In League URI I use my own parser based on the RFC3986 regex.

[Matthew Weier O'Phinney]

On Mon, Jun 6, 2016 at 4:29 AM, Tobion <tob...@gmail.com> wrote:
> https://tools.ietf.org/html/rfc3986#appendix-A
>
>
> authority = [ userinfo "@" ] host [ ":" port ]
>
> host = IP-literal / IPv4address / reg-name
>
> reg-name = *( unreserved / pct-encoded / sub-delims )
>
>
> As you can see reg-name can be empty which means the host can be empty which
> means
>
> "urn://user:pass@:123/path" is a valid URI. The reasoning is also given:
>
>
> https://tools.ietf.org/html/rfc3986#section-3.2.2
>
>> If the URI scheme defines a default for host,

That's the key phrase, then. HTTP and HTTPS, which are the only
schemes supported by UriInterface, do not define defaults for the
host, which would make lack of a host an invalid URI for those
schemes.

--
Matthew Weier O'Phinney
mweiero...@gmail.com
https://mwop.net/

[Tobion]

Yep. But that creates the problem that in theory one of the most common approaches would turn invalid:

(new Uri)->withScheme('http')->withHost('example.org');

This would be invalid if you enforce that because the moment the URI has the http scheme, there is no host yet.

To avoid this problem I proposed for guzzle/psr7 to just define localhost as default host for http(s) URIs when

there is no host yet. See https://github.com/guzzle/psr7/pull/94

This also makes sure that you can call

$uri->withPort(8080)->withHost('host')

whatever the scheme might be.

[ignace nyamagana]

Yep, but adding a default host is trying to solve a educational problem with something I'd consider being a hack. Better let the developer understand why it fails and learn from it too.

my .2$

[ignace nyamagana]

Bumping up this discussion as it seems it was overlooked/forgotten.

@Larry did our responses correctly answer your concerns? If so can this be put into vote. If not what still need to be addressed ? What changes do you want tobion to make to his PR. FWIW there was an alternative to this proposal here

Ignace

[Larry Garfield]

As I recall, my issues weren't with the content per se (I am happy to let MWOP manage that) but with the fact that the PR under vote modified the spec itself, which is a no-no.  Adding Errata to the metadoc to clarify points that were unclear or not perfectly consistent in the original spec is fine; for details of what that Errata should say, for my part if MWOP is happy then I'm (probably) happy.

--Larry Garfield

--

You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.
To post to this group, send email to php...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/013686ad-2615-4caf-a663-f69e5bdebe2f%40googlegroups.com.

[ignace nyamagana]

@Tobion

could you then "expurge" your PR of all the typo changes so that we can move on with this vote. I don't think we should remove any text if the errata is well written and the bylaws accepts the use of annotations for erratas to help implementations where confusion may arise.

[Tobion]

I reverted the typo fixes that are unrelated to the errata.

[ignace nyamagana]

@Tobion to summarize Larry concerns.

When submitting an errata you can not update the interface docblock, but you have

• rewrote the text around InvalidArgumentException for all with* methods
• removed some part of the __toString method docblock.

But according to the bylaws this is not possible. Erratas can not change the voted docblock texts unless you submit a brand new PSR to supersede PSR7.

That's why I'm suggesting using annotations to indicate the importance of the errata submitted. This annotation can be add in the paragraph which defines the URIInterface Without resorting to updating the UriInterface docblock directly. The errata text in itself is good IMHO, we just need to find a good formatting/highlighting mode to ensure everyone has access to it.

Does anyone have a suggestion ?