PSR-7: missing method withServerParams()

[Rasmus Schultz]

Dear FIG,

For some reason, a withServerParams() method appears to be missing from ServerRequestInterface of PSR-7.

Every other model property of every interface in PSR-7 has matching get_() and with_() method pairs, but for some reason this was omitted for server-params.

This is proving to be problematic and rather awkward with regards to PSR-17, the HTTP factory interfaces, where is has become impossible to create a simple, meaningful, generic interface for the creation of a ServerRequestInterface instance.

It has lead to a highly regrettable but unavoidable inconsistency in the factory-interfaces, per this thread:

https://github.com/http-interop/http-factory/pull/20#issuecomment-249440215

I can't find any mention of a withServerParams() method having ever been mentioned or discussed in this forum, and I can't find anything is the PSR-7 spec or meta documents stating that this was deliberately omitted for any reason.

This omission means that the initialization of server-params can only be done in an implementation-specific way, e.g. by a constructor or factory-method.

Was this in deed a mere oversight, as it would seem, or an undocumented deliberate choice?

Either way, it's the sort of thing someone maintaining a piece of software would simply correct and version-bump on any normal day.

I was told earlier today that there is no such thing as making a correction to a PSR, post approval, if such a correction constitutes a breaking change?

I don't think I need to explain why being unable (or forbidden, or even discouraged) to correct mistakes in software is extremely problematic? You're all software developers, and you all know the realities of software development - software has bugs, and bugs spread, such as it would appear to be doing in this case.

If I understand correctly, the only way to add the missing method, would be to start an entirely new, derivative PSR with a new number?

Declaring an entirely new standard, as opposed to declaring a new version of an existing standard, just to fix one problem, seems unlikely to happen - it seems like something people would oppose on the mere grounds that PSR-7 is already a known, well-established, widely-used thing.

In other words, people would not logically oppose this on the grounds that it's an insurmountable effort or in any way a severe problem to add such a method - most projects would be able to add this method, update their composer file, and upgrade to a new version of the interface in about two minutes.

Does the nature of a PSR standard itself prevent (or at least discourage) us from making obvious, rational improvements?

Please don't misunderstand me, I'm not pointing fingers or trying to accuse anyone of anything - I myself reviewed PSR-7 many times while it was in development, and never noticed this; I could have easily made this mistake myself, for that matter.

The issue is not who's at fault, but rather, how can we fix such mistakes?

And if we really, truly can't (or are severely discouraged from) fixing such mistakes, how can we remedy that situation?

Having standards is wonderful, but being unable to correct things is potentially detremental to the eco-system as it leads to a kind of long-term decay that would appear to spread.

Is this one of those "love it or shove it" situations, or can we do something to improve this?

Thanks,

  Rasmus

[Magnus Nordlander]

While this is less than ideal, I don't think having to create a new, derivative PSR is necessarily that difficult. To me, it seems to be mostly an issue of nomenclature. What really is the practical difference between calling the corrected version PSR-7 rev. B and calling it PSR-18? Since it's a breaking change, implementing packages will still need to change to make sure they implement the new interface.

There's also the additional indirection provided by the Composer package. Realistically, regardless of whether you call it PSR-7 rev. B or PSR-18, it will still be psr/http-message version 2.0.0.

--
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+unsubscribe@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/09d24771-1a91-4199-a98e-bd16a759a0af%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Matthew Weier O'Phinney]

On Mon, Sep 26, 2016 at 2:54 PM, Rasmus Schultz <ras...@mindplay.dk> wrote:
> For some reason, a withServerParams() method appears to be missing from
> ServerRequestInterface of PSR-7.
>
> Every other model property of every interface in PSR-7 has matching get_()
> and with_() method pairs, but for some reason this was omitted for
> server-params.
>
> This is proving to be problematic and rather awkward with regards to PSR-17,
> the HTTP factory interfaces, where is has become impossible to create a
> simple, meaningful, generic interface for the creation of a
> ServerRequestInterface instance.
>
> It has lead to a highly regrettable but unavoidable inconsistency in the
> factory-interfaces, per this thread:
>
> https://github.com/http-interop/http-factory/pull/20#issuecomment-249440215
>
> I can't find any mention of a withServerParams() method having ever been
> mentioned or discussed in this forum, and I can't find anything is the PSR-7
> spec or meta documents stating that this was deliberately omitted for any
> reason.
>
> This omission means that the initialization of server-params can only be
> done in an implementation-specific way, e.g. by a constructor or
> factory-method.
>
> Was this in deed a mere oversight, as it would seem, or an undocumented
> deliberate choice?

It was deliberate. I thought we'd documented it, but evidently this is
not the case.

Server params are a direct map to `$_SERVER`. Those values are created
exactly once, when PHP initializes, and *should not change* over the
course of the request (although it *is* possible to do so, as, like
other superglobals in PHP, it's not an immutable value; doing so can
have unexpected consequences, though).

It's very much *unlike* the other parameters:

- Query params, while they typically map to $_GET, might be something
you want to change as you traverse different layers of your
application. Additionally, depending on the Server API (SAPI), these
may need to be *calculated* based on the incoming URI.
- Cookie params typically map to $_COOKIE, but, like query parameters,
may need to be calculated from the Cookie header if the SAPI does not
provide $_COOKIE.
- Uploaded files typically maps to $_FILES... except when you have a
non-POST request, or your SAPI doesn't create $_FILES.
- Body params *can* be created from $_POST, except when you have a
non-POST request, or a non-form-urlencoded request, or your SAPI
doesn't populate the superglobal.
- Attributes, as documented, are specifically to allow for a "bucket"
to hold additional values derived from processing the request (e.g.,
products of routing).

In other words, server parameters, unlike the other parameter types,
cannot be the product of calculations, and should be static for the
given request. This is why we did not provide a `withServerParams()`
method. The expectation is that they would be injected during
instantiation.

Considering PSR-17, I still don't see what the problem is.
Implementations would likely need to have a request prototype
composed, or compose the various artifacts for generating a new
instance. Alternately, these could be provided during invocation of
the factory if desired, likely from an existing request.

> Either way, it's the sort of thing someone maintaining a piece of software
> would simply correct and version-bump on any normal day.
>
> I was told earlier today that there is no such thing as making a correction
> to a PSR, post approval, if such a correction constitutes a breaking change?
>
> I don't think I need to explain why being unable (or forbidden, or even
> discouraged) to correct mistakes in software is extremely problematic?
> You're all software developers, and you all know the realities of software
> development - software has bugs, and bugs spread, such as it would appear to
> be doing in this case.
>
> If I understand correctly, the only way to add the missing method, would be
> to start an entirely new, derivative PSR with a new number?
>
> Declaring an entirely new standard, as opposed to declaring a new version of
> an existing standard, just to fix one problem, seems unlikely to happen - it
> seems like something people would oppose on the mere grounds that PSR-7 is
> already a known, well-established, widely-used thing.
>
> In other words, people would not logically oppose this on the grounds that
> it's an insurmountable effort or in any way a severe problem to add such a
> method - most projects would be able to add this method, update their
> composer file, and upgrade to a new version of the interface in about two
> minutes.
>
> Does the nature of a PSR standard itself prevent (or at least discourage) us
> from making obvious, rational improvements?

A specification is not the same as software.

Specifications do not change, and should not, because doing so then
presents a moving target for implementors.

As an example, if I, as an implementor, read the spec one day and
implement it, I should be satisfied that my implementation can be
dropped in anywhere. Alternately, if I am a consumer of it, I should
be satisfied that if I target the specification, my code will work as
expected. However, if the specification changes, I may find that my
implementation is now missing implementation of new methods, or, as a
consumer, that I'm not passing enough arguments to an existing method,
or getting back something I did not expect.

Yes, proper versioning is necessary, but that's exactly what the
specification process provides; this is why FIG, and other standards
bodies like IETF, W3C, and ISO, provide a process for marking a
specification as obsolete or abandoned when another standard
supercedes it. When a specification is voted on and approved, it's
final. If you want to make a change, you create a *new* specification,
and, if approved, it marks the existing specification as obsolete, so
that developers can choose which to implement or target, and have a
path for doing so that does not require immediate breakage.

Implementors choose whether or not they will adopt the new standard,
but in the meantime, any software written for the existing standard
*continues to work*. That's the whole point.

It may seem like it would be easier to allow versioning a
specification. But you end up then with somebody saying, "We implement
Standard X", and then you have to ask, "Okay, but *which version*?"
Having new recommendations supercede existing ones solves that
problem.

If it helps, think of the new specification as the next major version
of the spec.

(Also, as a point of reference, PEP 3333, which is the current WSGI
standard for Python, superseded PEP 333. My point is: this happens in
*every* standards body. It's expected.)

> Please don't misunderstand me, I'm not pointing fingers or trying to accuse
> anyone of anything - I myself reviewed PSR-7 many times while it was in
> development, and never noticed this; I could have easily made this mistake
> myself, for that matter.

As noted, it isn't a mistake. It was deliberate. If you feel the
decision should be documented, I'd be happy to open an errata for the
meta document that explains the decision.

> The issue is not who's at fault, but rather, how can we fix such mistakes?
>
> And if we really, truly can't (or are severely discouraged from) fixing such
> mistakes, how can we remedy that situation?
>
> Having standards is wonderful, but being unable to correct things is
> potentially detremental to the eco-system as it leads to a kind of long-term
> decay that would appear to spread.

One thing about standards are they are intended to be long-lived. If
you make a change every time somebody comes along and disagrees with
any aspect of it, we no longer have a standard; at that point,
consensus has been lost, as we've lost the benefits of peer review and
consensus.

Standards are expected to provide the expectation that the
specification will not change, so that developers can ensure that code
written to the specification will work now and into the future. As
noted on wikipedia, "The existence of a published standard does not
imply that it is always useful or correct."
(https://en.wikipedia.org/wiki/Technical_standard#usage) It does,
however, imply that efforts have been made to ensure it is correct and
useful by the parties creating the specification, and that, regardless
of success, any code written to it will work as specified. **It
ensures that a peer-review process was followed, and consensus
reached.**

As Magnus writes elsewhere in this thread, if we have a limited scope
of changes we want to accomplish, we can create a successor to an
existing standard relatively quickly. On top of that, as he also
notes, since it covers the same domain, the interfaces themselves
would be published under the same package as a different version most
likely anyways.

So, if we have any required changes, we can address those and consider
a new PSR. In the meantime, I hope my explanation above helps shine a
light on why we may not need to do so in this particular case.

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

[Rasmus Schultz]

Okay, so I understand everything you said about not changing an existing PSR - this seems to be consistent with other types of specifications, such as RFCs, which also do not change, but instead, if they change, spawn a new, derived RFC. Gotcha.

As far as server-params being immutable in the server request model, I don't follow.

Your examples with regards to other super-globals is great, but none of that amounts of an argument against modification of server-params.

Calculated values, for example, could well be a case for server-params as well - for example, if you're implementing a SAPI (an HTTP server stack) in native PHP, you might import environment variables, and then subsequently import overrides from a configuration file.

While surely you could do that "all at once", the same is true for all the modifications to other super-globals you describe - it wouldn't be very practical, no, but certainly possible; so you could use the same argument to say that all the other properties that map to values from super-globals should be immutable too.

As said, that's not practical though, and the same is the true for server-params - immutability just means you need to create a mutable collection of server-params elsewhere and defer the creation of the actual server-request; the requirement doesn't go away, it just moves somewhere else, which complicates things.

What I'm missing is an explanation as to how permitting modification of server-params is somehow harmful or wrong?

If it isn't downright harmful or wrong, it should be permitted, for sheer consistency: all other properties of the model have matching factory-methods, so unless having that for server-params is somehow harmful, *not* having them is actually harmful and (above all) highly impractical.

> Server params are a direct map to `$_SERVER`. Those values are created 

> exactly once, when PHP initializes, and *should not change* over the 
> course of the request

The same is true for any super-global.

> (although it *is* possible to do so, as, like 
> other superglobals in PHP, it's not an immutable value; doing so can 
> have unexpected consequences, though). 

Again, true for the modification of $_GET, $_POST, etc.

I don't see the difference.

Yes, modification of any of these values could have unexpected consequences - it could also, on the other hand, have expected consequences.

In particular, writing tests for components that respond to differences in server-params is unnecessarily encumbered by this restriction.

In summary, I remain convinced that this restriction is impractical, inconsistent with the rest of the model, and, above all, unnecessary.

It's what I like to call an "artificial constraint" - it's not a constraint that is there to protect against any sort of problem (is it?) but seems to be solely based on somebody's opinion or feeling that "this should not be modified".

I'm not looking to create a problem here - I'd prefer not to create a problem.

However, there is a problem.

Plain and simple.

:-/

[Rasmus Schultz]

A little update on this issue...

> It was deliberate. I thought we'd documented it, but evidently this is not the case. 

It turns out, yes, it was documented after all - in the inline documentation block of ServerRequestInterface.

It states:

> $_SERVER values MUST be treated as immutable, as they represent application state at the time of request; as such, no methods are provided to allow modification of those values. The other values provide such methods, as they can be restored from $_SERVER or the request body, and may need treatment during the application

Unfortunately this argument is (well, excuse me, but) nonsense.

$_SERVER itself of course must be treated as immutable and does "represent application state at the time of request" - but it's an array, so of course this will be *copied* to the request model. (at least by default, unless you were to explicitly use the reference operator or something.)

The entire request model "represents application state at the time of request", so the same argument would work against modifying anything at all. But the model is immutable, so the argument isn't even relevant in the first place - you can't affect the state of $_SERVER once that state is copied to the model, and secondly, you can't affect the state of "server params" once they're in the model either, anymore than you can modify any other value in the model, you can only generate new request instances.

Nothing is mutable in the first place, so that line or argumentation really doesn't work at all.

I understand the potential harm in modifying $_SERVER, but that's not what we're talking about. We're not modifying any superglobals. A new request-model can *always* be generated again from the superglobals, at any time.

As you can see, this decisions leads to weirdness in the PSR-17 factory interface:

https://github.com/http-interop/http-factory/blob/master/src/ServerRequestFactoryInterface.php#L28

The $server argument is an artifact stemming from the fact that injecting $_SERVER variables can only be done in an implementation-specific manner, via a proprietary constructor.

Omitting this was a mistake.

And any day now, we'll have PSR-15 middleware and PSR-17 factory classes depending on this spec, at which point it becomes ever more difficult to fix - once those are in the water, I'd say it's unlikely this will *ever* change... :-(

But my guess is, no one wants to even deal with the process of starting a new PSR for this, and instead, we'll simply inherit this mistake, which will spread in the form of artifacts into PSR-17 and elsewhere.

I hate that we can't version a PSR. I find it hugely problematic, because PSR-7 is more than just a "standard" - it's a set interfaces, which are source-code, and there's a reason why source-code dependencies are versioned as packages... I understand *why* the standard can't change, it just doesn't change the fact that sometimes you realize too late that it *should*...

Time makes fools of us all :-/

On Tuesday, September 27, 2016 at 11:17:03 PM UTC+2, Matthew Weier O'Phinney wrote:

[Rasmus Schultz]

For the record, this problem is now about to spread to PSR-17, where it creates the need for two ugly method-signatures to work around the fact that you can't generate new objects with different server-params.

https://github.com/http-interop/http-factory/pull/33#pullrequestreview-38005141

This shit sucks.

I honestly think we should let PSR-7 be superseded by an identical PSR with the missing server-params factory method added.

I know that's not going to be popular, but given the alternative... we just live with this mistake and let it spread ugliness everywhere in the future? That's crap.

We ought to fix this.

Painstaking as that would be... :-/

--
You received this message because you are subscribed to a topic in the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/php-fig/70j-bBIMn_E/unsubscribe.
To unsubscribe from this group and all its topics, send an email to php-fig+unsubscribe@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/b75861f1-f384-490b-8531-e6e2997577bc%40googlegroups.com.

[Nicholas Ruunu]

I believe you are overthinking this too much.

To me it's more important to be able to trust that the ServerRequest has the correct, unmodified values from the $_SERVER global.

Introducing a withServerParams method will jeopardise that trust and will be more error/bug prone.

Regarding ServerRequestFactoryInterface is the least important part of PSR17 just for that reason.

You should only create this object once, and if you want to change implementations, you only need to change this one thing.

Not in multiple places like RequestFactoryInterface.

Also, I think the proposed fix in PSR17 makes little sense:

public function createServerRequest($method, $uri, $server = []);

The URI and method is derived from $_SERVER, that would mean that $method and $uri would be left empty in 99% of cases.

I see some value in having $method and $uri for testing purposes but not much more than that.

I'd rather see the method being completely empty of parameters like createFromGlobals(), but this is the second best thing imo.

To unsubscribe from this group and all its topics, send an email to php-fig+u...@googlegroups.com.

To post to this group, send email to php...@googlegroups.com.