Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing protocolVersion in response header #15

Closed
kenjis opened this issue Mar 11, 2016 · 6 comments
Closed

Missing protocolVersion in response header #15

kenjis opened this issue Mar 11, 2016 · 6 comments

Comments

@kenjis
Copy link
Member

kenjis commented Mar 11, 2016

We can't see the Welcome page, because the response header is invalid.

HTTP/ 200 OK   <-- here
Host: localhost:8000
Connection: close
X-Powered-By: PHP/7.0.4
Cache-control: no-store, max-age=0, no-cache
Content-type: text/html; charset=UTF-8

We should set protocolVersion somewhere, but I'm not sure where should be.

@lonnieezell
Copy link
Member

Thanks for the reminder! I needed to investigate how the HTTP spec said things should work between major versions and it sounds like we just must specify the highest version that we conditionally support, so I've created a config setting and set the default to 1.1. With HTTP/2 here, though, we needed some way for the developer to specify. And I probably need to investigate HTTP/2 in a little bit more detail sometime soon, though from what I've read it doesn't affect the app itself too much, though there's some push-type items that we might be able to provide for.

Fixed in e4cdd26

@narfbg
Copy link
Collaborator

narfbg commented Mar 11, 2016

It doesn't work that way ... you're reversing the client and server roles - you can't respond with a version higher than the one specified by the client request. It's the client that always requests its own highest supported version.
If the client requests 1.1, you can downgrade to 1.0 because that's the natural thing to do if you don't have 1.1 on the server, but the opposite doesn't make sense.

@lonnieezell
Copy link
Member

Good point, and that's what I was thinking in my head. In CI3 we specify 1.1 explicitly IIRC, And we needed a way to know the highest protocol that our app would support in order to check whether we needed to downgrade, as I understand it. So, I made a config item for the version the application supports so that we can know whether it supports 2 or not. It defaults to 1.1 currently, so you're right we need to check to see if it needs to be downgraded or not.

Or do we not even need to worry about that and let the server software handle it?

Does that make sense? I've got a massive headache at the moment so not 100% sure :)

@lonnieezell
Copy link
Member

@narfbg Thinking through this a little more, I'm wondering. Does it simply make sense to return the protocol version in the request? It seems that the only thing in our code that HTTP2 would really affect is the ability to do server side push, and this seems to be currently handled via the Link header (at least in NGINX, unsure of others ATM) and those headers would be ignored on HTTP/1.x.

So, if the client sends a request for 2 and the server only supports 1.1, it will downgrade it before it gets to us, I believe. Does that sound right?

@narfbg
Copy link
Collaborator

narfbg commented Mar 11, 2016

2 doesn't bring any changes at the application level, it's all about data transmission - you can't control that anyway.

As for 1.1 vs 1.0 ... it doesn't make sense to configure what you support on the server side - you either support 1.1 or you don't. There's no reason to limit yourself to 1.0.
You could just output the client-requested version, indeed - that's the easy part. The hard part is the logical one - some of the features we're used to today don't exist in 1.0.

@lonnieezell lonnieezell reopened this Mar 11, 2016
@lonnieezell
Copy link
Member

Have revised to simply send back the requested version, then. Simplifies everything and the rest is not really under the framework's control.

Thanks!

Fixed in 0333a63

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants