|Summary:||Tomcat should allow blank Host header|
|Product:||Tomcat 7||Reporter:||Michael Orr <michaelomichael>|
|Component:||Connectors||Assignee:||Tomcat Developers Mailing List <dev>|
|OS:||Mac OS X 10.1|
Description Michael Orr 2018-09-19 12:09:06 UTC
Changes to request handling in 7.0.87 mean that sending an empty string for the "Host" request header now results in a "400 Bad Request" response, even though the request is legal. This can be demonstrated by running the following command: curl -XGET -v -I -H "Host: " "http://localhost:8080/RELEASE-NOTES.txt" The request that is sent is: GET /RELEASE-NOTES.txt HTTP/1.1 Host: User-Agent: curl/7.54.0 Accept: */* According to RFC 7230 (https://tools.ietf.org/html/rfc7230#section-5.4) it is legal for the Host header to have an empty field value: "If the authority component is missing or undefined for the target URI, then a client MUST send a Host header field with an empty field-value." While uncommon, I have come across such cases. For example, an F5 load balancer wants to make an HTTP request to help determine whether or not a given HTTP server is still 'alive', so it creates a TCP connection and then sends the following text string: GET /alive.html HTTP/1.1\r\nHost:\r\n\r\n Changing this string so that the Host header is not blank would, of course, be the easiest option, but it is not always possible to do, especially if the same string is used for many disparate server types. We should modify the code (AbstractHttp11Processor, I'm guessing?) to allow a blank Host header value.
Comment 1 Michael Orr 2018-09-19 15:30:35 UTC
I should add that I'm happy to investigate a fix for this, assuming there are no objections to the proposed change.
Comment 2 Mark Thomas 2018-09-21 13:23:41 UTC
Please go ahead. We typically (and this is a typical case) fix the issue in trunk first and then back-port. My initial impression is that this is a fairly simple fix. Don't forget to check/add/update the unit tests. Patches can be provided by either attaching them to this issue (in diff -u format please) or by creating a PR against the GitHub mirror https://github.com/apache/tomcat If you have any questions, just ask.
Comment 3 Mark Thomas 2018-10-01 19:14:06 UTC
Is there an ETA on your fix? We have reached the start of the month and I expect we'll want to tag a new version with this fix included in the next few days.
Comment 4 Michael Orr 2018-10-02 09:18:17 UTC
Hi Mark, it won't happen this week. Possibly by the end of next week.
Comment 5 Mark Thomas 2018-10-02 15:40:00 UTC
OK. The other open issues are looking to be trickier than expected so we have a little more time than I first thought. I'll leave this until last to give you as much time as possible.
Comment 6 Michael Orr 2018-10-03 07:22:05 UTC
I managed to find time after all! Here's the pull request: https://github.com/apache/tomcat/pull/124
Comment 7 Mark Thomas 2018-10-03 11:49:41 UTC
Thanks. Looking at this now.
Comment 8 Mark Thomas 2018-10-03 15:51:39 UTC
Patch looks good - thanks for including the tests. My main thought at this point is what should HttpServletRequest.getServerName() return? My reading of the Javadoc is if the Host header is present, its value should be returned by HttpServletRequest.getServerName(). That requires a slightly different fix (in the parser to allow empty values). Either solution enables the new tests to pass. Leaving this open for a little while to think about it.
Comment 9 Michael Orr 2018-10-03 17:22:30 UTC
Good question. Let me know how you want to proceed, and I'll add tests and code accordingly.
Comment 10 Mark Thomas 2018-10-04 10:29:28 UTC
Thanks for the offer of an updated patch. Lets go with HttpServletRequest.getServerName() returning an empty string in this case as that is consistent with the Host header that was sent.
Comment 11 Michael Orr 2018-10-04 23:23:28 UTC
Done. Pull request has been updated. Let me know if there's anything I've missed.
Comment 12 Mark Thomas 2018-10-05 10:38:57 UTC
Fixed in: - trunk for 9.0.13 onwards - 8.5.x for 8.5.35 onwards - 7.0.x for 7.0.92 onwards