Comments (11)
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.
from psradapter.
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
.
from psradapter.
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 apply
method in SecureHeaders, and then present the getter as a way to retrieve the (now mutated) stored contents?
from psradapter.
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.
from psradapter.
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
from psradapter.
Once we add instructions for the new PSR-15 middleware, there won't have to be much doc reading. :)
from psradapter.
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?
from psradapter.
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).
from psradapter.
Any further thoughts on a release here?
from psradapter.
I'm all 👍. @aidantwoods Are you up for it?
from psradapter.
@aidantwoods release?
from psradapter.
Related Issues (1)
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from psradapter.