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

go.mod: github.com/docker/docker 8443a06149b5 (v24.0.5-dev) #10810

Merged
merged 1 commit into from
Jul 17, 2023

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Jul 16, 2023

relevant changes:

  • client: define a "dummy" hostname to use for local connections fixes "http: invalid Host header" errors when compiling with go1.20.6 or go1.19.11

full diff: moby/moby@v24.0.4...8443a06

diff in code used by compose ('vendored code"):

diff --git a/vendor/github.com/docker/docker/client/client.go b/vendor/github.com/docker/docker/client/client.go
index 1c081a51a..54fa36cca 100644
--- a/vendor/github.com/docker/docker/client/client.go
+++ b/vendor/github.com/docker/docker/client/client.go
@@ -56,6 +56,36 @@ import (
 	"github.com/pkg/errors"
 )
 
+// DummyHost is a hostname used for local communication.
+//
+// It acts as a valid formatted hostname for local connections (such as "unix://"
+// or "npipe://") which do not require a hostname. It should never be resolved,
+// but uses the special-purpose ".localhost" TLD (as defined in [RFC 2606, Section 2]
+// and [RFC 6761, Section 6.3]).
+//
+// [RFC 7230, Section 5.4] defines that an empty header must be used for such
+// cases:
+//
+//	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.
+//
+// However, [Go stdlib] enforces the semantics of HTTP(S) over TCP, does not
+// allow an empty header to be used, and requires req.URL.Scheme to be either
+// "http" or "https".
+//
+// For further details, refer to:
+//
+//   - https://github.com/docker/engine-api/issues/189
+//   - https://github.com/golang/go/issues/13624
+//   - https://github.com/golang/go/issues/61076
+//   - https://github.com/moby/moby/issues/45935
+//
+// [RFC 2606, Section 2]: https://www.rfc-editor.org/rfc/rfc2606.html#section-2
+// [RFC 6761, Section 6.3]: https://www.rfc-editor.org/rfc/rfc6761#section-6.3
+// [RFC 7230, Section 5.4]: https://datatracker.ietf.org/doc/html/rfc7230#section-5.4
+// [Go stdlib]: https://github.com/golang/go/blob/6244b1946bc2101b01955468f1be502dbadd6807/src/net/http/transport.go#L558-L569
+const DummyHost = "api.moby.localhost"
+
 // ErrRedirect is the error returned by checkRedirect when the request is non-GET.
 var ErrRedirect = errors.New("unexpected redirect in response")
 
diff --git a/vendor/github.com/docker/docker/client/hijack.go b/vendor/github.com/docker/docker/client/hijack.go
index 6bdacab10..4dcaaca4c 100644
--- a/vendor/github.com/docker/docker/client/hijack.go
+++ b/vendor/github.com/docker/docker/client/hijack.go
@@ -64,7 +64,11 @@ func fallbackDial(proto, addr string, tlsConfig *tls.Config) (net.Conn, error) {
 }
 
 func (cli *Client) setupHijackConn(ctx context.Context, req *http.Request, proto string) (net.Conn, string, error) {
-	req.Host = cli.addr
+	req.URL.Host = cli.addr
+	if cli.proto == "unix" || cli.proto == "npipe" {
+		// Override host header for non-tcp connections.
+		req.Host = DummyHost
+	}
 	req.Header.Set("Connection", "Upgrade")
 	req.Header.Set("Upgrade", proto)
 
diff --git a/vendor/github.com/docker/docker/client/request.go b/vendor/github.com/docker/docker/client/request.go
index c799095c1..bcedcf3bd 100644
--- a/vendor/github.com/docker/docker/client/request.go
+++ b/vendor/github.com/docker/docker/client/request.go
@@ -96,16 +96,14 @@ func (cli *Client) buildRequest(method, path string, body io.Reader, headers hea
 		return nil, err
 	}
 	req = cli.addHeaders(req, headers)
+	req.URL.Scheme = cli.scheme
+	req.URL.Host = cli.addr
 
 	if cli.proto == "unix" || cli.proto == "npipe" {
-		// For local communications, it doesn't matter what the host is. We just
-		// need a valid and meaningful host name. (See #189)
-		req.Host = "docker"
+		// Override host header for non-tcp connections.
+		req.Host = DummyHost
 	}
 
-	req.URL.Host = cli.addr
-	req.URL.Scheme = cli.scheme
-
 	if expectedPayload && req.Header.Get("Content-Type") == "" {
 		req.Header.Set("Content-Type", "text/plain")
 	}

What I did

Related issue

(not mandatory) A picture of a cute animal, if possible in relation to what you did

relevant changes:

- client: define a "dummy" hostname to use for local connections
  fixes "http: invalid Host header" errors when compiling with
  go1.20.6 or go1.19.11

full diff: moby/moby@v24.0.4...8443a06

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah changed the title go.mod: github.com/docker/docker 36e9e796c6fc (v24.0.5-dev) go.mod: github.com/docker/docker 8443a06149b5 (v24.0.5-dev) Jul 17, 2023
@codecov
Copy link

codecov bot commented Jul 17, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.03 🎉

Comparison is base (852e192) 59.14% compared to head (0769e18) 59.17%.

Additional details and impacted files
@@            Coverage Diff             @@
##               v2   #10810      +/-   ##
==========================================
+ Coverage   59.14%   59.17%   +0.03%     
==========================================
  Files         115      115              
  Lines        9869     9869              
==========================================
+ Hits         5837     5840       +3     
+ Misses       3438     3436       -2     
+ Partials      594      593       -1     

see 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@thaJeztah thaJeztah mentioned this pull request Jul 17, 2023
1 task
@milas milas self-assigned this Jul 17, 2023
@milas milas added the dependencies Pull requests that update a dependency file label Jul 17, 2023
@milas milas merged commit ce8a09b into docker:v2 Jul 17, 2023
@thaJeztah thaJeztah deleted the update_docker branch July 17, 2023 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants