Any release concerns?

[aidantwoods]

If there are any concerns that need addressing prior to a release (and therefore subsequent locking of the public API compatibility to the major version) please open a new issue or raise them below :)

I think it is probably a good idea to bump this to 1.0 prior to, or at the very latest in sync with the next minor version of SecureHeaders (in which the built in adapter will be deprecated).

[jakejohns]

Most of the public api here is define by external interfaces so not too much to consider.

What about the semantics of "get final response"? It's not necessarily "final". What about "get secured response" and the method might throw an exception if "send headers" hasn't been called?

I can work a PR in a while if that sounds like something to consider.

[aidantwoods]

Yeah I see what you mean there, if sendHeaders hadn't yet been called then you'd get the original response without SecureHeaders applying anything to it – which kinda defies the method spec of:

Retrieve the new PSR-7 response object, with security headers applied

Replacing the previous comment here after reading through the code again properly ;p

I think you are right in that the method should throw if send headers has not been called. Possibly we should also allow the user to also pre-empt the throw by allowing them to read the state as to whether or not the adapter has yet recorded a call to sendHeaders.

[aidantwoods]

Another option to throwing may be removing the guarantee that what is returned has been passed through SecureHeaders. Might seem a little unintuitive, but I don't think that is really a guarantee we can make from the adapter (e.g. the user could call sendHeaders themselves).

Instead we should perhaps convey that the user is responsible for ensuring that the adapter has been though the applymethod in SecureHeaders, and then present the getter as a way to retrieve the (now mutated) stored contents?

[franzliedke]

Honestly, this is mostly a theoretical discussion, as far as I am concerned.

This adapter will mostly (only?) be used with the provided middleware, hence correct usage will be guaranteed. For extra safety, we can throw the exception when the adapter is used incorrectly. This will help to educate users.

@aidantwoods Giving access to the original response instance does not help, it cannot. PSR-7 responses are immutable - you have to create new instances to change any attributes.

[aidantwoods]

PSR-7 responses are immutable - you have to create new instances to change any attributes.

Indeed, I had mistakenly recalled that they were mutable and didn't check to see that the value was replaced upon value change (hence the replacement of the comment after reading the code ;p) – my mistake!

Honestly, this is mostly a theoretical discussion, as far as I am concerned.

Theoretical provided the user understands the purpose of the adapter is to be a middleman for SecureHeaders interacting with some implementation of the PSR adapter – I think it might be good to make it clear that the adapter does not interact with SecureHeaders itself. 'tis to say, I'd rather over explain than leave the wrong impression.

You are quite right in that: in correct usage, people should be reading the docs and thus understand that the purpose of the adapter is not to call SecureHeades on your behalf ;p

[franzliedke]

Once we add instructions for the new PSR-15 middleware, there won't have to be much doc reading. :)

[franzliedke]

Now that PSR-15 is merged (#7, thanks @jakejohns), let's release 2.0? We could still release as 1.0, too - the branch is named 2.0, but we've only had 0.x releases so far.

@aidantwoods Okay for you?

[aidantwoods]

Sounds good to me, though given the delay since this was all planned maybe let's just release this as 1.0? :)

Reasoning behind labelling as 2.0 was so that we could offer the first release (1.0) at the same PHP version as SecureHeaders 2.0 (#7 (comment)).
But since every release of PHP 5 is now EOL, I think that requiring people update to use PHP 7 for something that is new is much less aggressive. In any case, there was never any plan to remove the current way of doing things from SecureHeaders 2.0, just to deprecate in the next minor (so nothing wrong here as far as SemVer is concerned).

[jakejohns]

Any further thoughts on a release here?

[franzliedke]

I'm all 👍. @aidantwoods Are you up for it?

[jakejohns]

@aidantwoods release?