-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add HTTP Host header to support Docker v1.12 #1550
Conversation
Can one of the admins verify this patch? |
@@ -67,6 +67,7 @@ protected DockerResponse request(String method, String path, String query, List< | |||
for (Pair<String, ?> header : headers) { | |||
connection.setRequestProperty(header.first, String.valueOf(header.second)); | |||
} | |||
connection.setRequestProperty("Host", url.toString()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case not only host will be put in that header but host + path +query.
@l0rd Do you think it is OK or only host+port should be put in that header.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@garagatyi you are absolutely right. We should set it to host+port only. Although I've test it and it worked, path+query should not be there. I'll fix that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still can use empty value. it is valid value due to the spec.
Signed-off-by: Mario Loriedo <mloriedo@redhat.com>
LGTM |
@skabashnyuk Please take a look |
ok |
LGTM |
OK |
Signed-off-by: Mario Loriedo <mloriedo@redhat.com>
Host
is a mandatory header in HTTP 1.1 specifications. In previous versions of Docker that header wasn't required but release v1.12 use golang 1.6.2 that makes this header mandatory.Therefore Docker v1.12 daemon doesn't accept HTTP 1.1 requests with no
Host
header set (you can find some references to the discussions here).This works:
echo -e "GET /containers/json HTTP/1.1\r\nHost: \r\n\r\n" | netcat -U /var/run/docker.sock
This doesn't work:
echo -e "GET /containers/json HTTP/1.1\r\n\r\n" | netcat -U /var/run/docker.sock
As a consequence in Che we had this nasty error message whenever we tried to create a new workspace:
This PR is related to #1482 and #745.