Support no-value Host header in HttpParser #592

Closed
kyyash opened this Issue May 26, 2016 · 6 comments

Comments

Projects
None yet
4 participants
@kyyash

kyyash commented May 26, 2016

Hi Team,

Please review and confirm,
As per HTTP 1.1 spec for Host header,
https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html

A client MUST include a Host header field in all HTTP/1.1 request messages . If the requested URI does not include an Internet host name for the service being requested, then the Host header field MUST be given with an empty value. An HTTP/1.1 proxy MUST ensure that any request message it forwards does contain an appropriate Host header field that identifies the service being requested by the proxy. All Internet-based HTTP/1.1 servers MUST respond with a 400 (Bad Request) status code to any HTTP/1.1 request message which lacks a Host header field.

This seems to be a bug from Jetty 9.2, as per HttpParser.java code snippet of Jetty Source

method name : handleKnownHeaders

It expects host header to have a value instead of just being empty. However, the HTTP 1.1 spec says, send at least an empty header.

As per HTTP 1.1 spec, sending empty host header is valid but Jetty considers it as a bad request.

Please advise.

http://grepcode.com/file/repo1.maven.org/maven2/org.eclipse.jetty/jetty-http/9.2.9.v20150224/org/eclipse/jetty/http/HttpParser.java#HttpParser

@kyyash kyyash closed this May 26, 2016

@kyyash kyyash reopened this May 26, 2016

@joakime

This comment has been minimized.

Show comment
Hide comment
@joakime

joakime May 26, 2016

Member

RFC2616 is obsolete.

Replaced with RFC7230 - https://tools.ietf.org/html/rfc7230#section-5.4

A no-value Host header is only valid if you don't have an authority on the request URI.

Request URI's without an authority are exceedingly rare (usually only seen in interprocess requests on the same server). With the Servlet security model, not having an authority is probably forbidden (have to see if the Servlet spec has that detail).

HTTPS, TLS, and HTTP/2 has mandatory authority.

Member

joakime commented May 26, 2016

RFC2616 is obsolete.

Replaced with RFC7230 - https://tools.ietf.org/html/rfc7230#section-5.4

A no-value Host header is only valid if you don't have an authority on the request URI.

Request URI's without an authority are exceedingly rare (usually only seen in interprocess requests on the same server). With the Servlet security model, not having an authority is probably forbidden (have to see if the Servlet spec has that detail).

HTTPS, TLS, and HTTP/2 has mandatory authority.

@kyyash

This comment has been minimized.

Show comment
Hide comment
@kyyash

kyyash May 26, 2016

Hi Joakime,

Thank you for the inputs, so the scenario we are facing is Http client in a load balancer polling the Jetty which hosts the web apps. It is as good as inter process requests.

Jetty returns 400 Bad host header response.

So will it be addressed by Jetty ?

From,
Yashwanth

kyyash commented May 26, 2016

Hi Joakime,

Thank you for the inputs, so the scenario we are facing is Http client in a load balancer polling the Jetty which hosts the web apps. It is as good as inter process requests.

Jetty returns 400 Bad host header response.

So will it be addressed by Jetty ?

From,
Yashwanth

@sbordet

This comment has been minimized.

Show comment
Hide comment
@sbordet

sbordet May 26, 2016

Contributor

It would be way simpler for you to be compliant and add a proper Host header.

Contributor

sbordet commented May 26, 2016

It would be way simpler for you to be compliant and add a proper Host header.

@joakime joakime changed the title from Jetty 9.2 HttpParser.java not as per HTTP 1.1 Spec, to Support no-value Host header in HttpParser May 27, 2016

@gregw gregw self-assigned this Jul 20, 2016

@gregw gregw closed this in 7e16731 Jul 20, 2016

@gregw

This comment has been minimized.

Show comment
Hide comment
@gregw

gregw Jul 20, 2016

Contributor

On review, allowing empty host headers is acceptable. fixed

Contributor

gregw commented Jul 20, 2016

On review, allowing empty host headers is acceptable. fixed

@gregw gregw reopened this Sep 27, 2016

@gregw

This comment has been minimized.

Show comment
Hide comment
@gregw

gregw Sep 27, 2016

Contributor

We need to fix this in the master branch

Contributor

gregw commented Sep 27, 2016

We need to fix this in the master branch

gregw added a commit that referenced this issue Oct 5, 2016

@gregw

This comment has been minimized.

Show comment
Hide comment
@gregw

gregw Oct 5, 2016

Contributor

fixed

Contributor

gregw commented Oct 5, 2016

fixed

@gregw gregw closed this Oct 5, 2016

gregw added a commit that referenced this issue Jan 1, 2017

backport of fixes for #592 (#1208)
Signed-off-by: olivier lamy <olamy@webtide.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment