fix(wip): avoid mangling Host header handling

- splitting into subdomain and dnslink gateway variants - towards avoiding running everything three times - keep custom Host header when defined
ipfs · May 24, 2024 · 3503595 · 3503595
1 parent 3e74a1a
commit 3503595
Show file tree

Hide file tree

Showing 6 changed files with 100 additions and 66 deletions.
diff --git a/.github/workflows/test-kubo-e2e.yml b/.github/workflows/test-kubo-e2e.yml
@@ -46,11 +46,7 @@ jobs:
       - name: Provision Kubo Gateway
         run: |
           # Import car files
-          cars=$(find ./fixtures -name '*.car')
-          for car in $cars
-          do
-              ipfs dag import --pin-roots=false --stats "$car"
-          done
+          find ./fixtures -name '*.car' -exec ipfs dag import --pin-roots=false {} \;
 
           # Import ipns records
           records=$(find ./fixtures -name '*.ipns-record')
@@ -63,7 +59,7 @@ jobs:
         uses: ./gateway-conformance/.github/actions/test
         with:
           gateway-url: http://127.0.0.1:8080
-          subdomain-url: http://example.com
+          subdomain-url: http://example.com:8080
           json: output.json
           xml: output.xml
           html: output.html

diff --git a/fixtures/redirects_file/dnslink.yml b/fixtures/redirects_file/dnslink.yml
@@ -1,10 +1,10 @@
 # yaml-language-server: $schema=fixture.schema.json
 dnslinks:
-  custom-dnslink:
-    domain: dnslink-enabled-on-fqdn.example.org
+  redirects-examples:
+    domain: dnslink-redirects-examples.example.org
     # cid of ./redirects.car:/examples/
     path: /ipfs/QmYBhLYDwVFvxos9h8CGU2ibaY66QNgv8hpfewxaQrPiZj
-  dnslink-spa:
+  redirects-spa:
     domain: dnslink-enabled-with-spa.example.org
     # cid of ./redirects-spa.car
     path: /ipfs/bafybeib5lboymwd6p2eo4qb2lkueaine577flvsjjeuevmp2nlio72xv5q
diff --git a/tests/dnslink_gateway_test.go b/tests/dnslink_gateway_test.go
@@ -1,17 +1,14 @@
 package tests
 
 import (
-	"net/url"
 	"testing"
 
 	"github.com/ipfs/gateway-conformance/tooling"
 	"github.com/ipfs/gateway-conformance/tooling/car"
 	. "github.com/ipfs/gateway-conformance/tooling/check"
 	"github.com/ipfs/gateway-conformance/tooling/dnslink"
-	"github.com/ipfs/gateway-conformance/tooling/helpers"
 	"github.com/ipfs/gateway-conformance/tooling/specs"
 	. "github.com/ipfs/gateway-conformance/tooling/test"
-	"github.com/ipfs/gateway-conformance/tooling/tmpl"
 )
 
 func TestDNSLinkGatewayUnixFSDirectoryListing(t *testing.T) {
@@ -20,28 +17,15 @@ func TestDNSLinkGatewayUnixFSDirectoryListing(t *testing.T) {
 	fixture := car.MustOpenUnixfsCar("dir_listing/fixtures.car")
 	file := fixture.MustGetNode("ą", "ę", "file-źł.txt")
 
-	// DNSLink domain and fixture we will be using for Host headerthis test
-	dnsLinkHostname := "dnslink-website.example.org"
 	dnsLinks := dnslink.MustOpenDNSLink("dir_listing/dnslink.yml")
 	dnsLink := dnsLinks.MustGet("dir-listing-website")
 
-	tests := SugarTests{}
-
-	// Sent requests to endpoint defined by --gateway-url
-	u, err := url.Parse(GatewayURL)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	// Host header should use dnslink domain with the same scheme as --gateway-url
-	hostHdr := tmpl.Fmt("{{scheme}}://{{dnslink}}", u.Scheme, dnsLink)
-
-	tests = append(tests, SugarTests{
+	tests := SugarTests{
 		{
 			Name: "Backlink on root CID should be hidden (TODO: cleanup Kubo-specifics)",
 			Request: Request().
-				Header("Host", hostHdr).
-				URL(GatewayURL),
+				Path("/").
+				Header("Host", dnsLink),
 			Response: Expect().
 				Body(
 					And(
@@ -53,8 +37,8 @@ func TestDNSLinkGatewayUnixFSDirectoryListing(t *testing.T) {
 		{
 			Name: "Redirect dir listing to URL with trailing slash",
 			Request: Request().
-				Header("Host", hostHdr).
-				URL(GatewayURL + "/ą/ę"),
+				Path("/ą/ę").
+				Header("Host", dnsLink),
 			Response: Expect().
 				Status(301).
 				Headers(
@@ -64,8 +48,8 @@ func TestDNSLinkGatewayUnixFSDirectoryListing(t *testing.T) {
 		{
 			Name: "Regular dir listing (TODO: cleanup Kubo-specifics)",
 			Request: Request().
-				Header("Host", hostHdr).
-				URL(GatewayURL + "/ą/ę"),
+				Path("/ą/ę/").
+				Header("Host", dnsLink),
 			Response: Expect().
 				Headers(
 					Header("Etag").Contains(`"DirIndex-`),
@@ -81,13 +65,14 @@ func TestDNSLinkGatewayUnixFSDirectoryListing(t *testing.T) {
 					And(
 						Contains("Index of"),
 						Contains(`<a href="/%C4%85/%C4%99/..">..</a>`),
-						Contains(`/ipns/<a href="//{{hostname}}/">{{hostname}}</a>/<a href="//{{hostname}}/%C4%85">ą</a>/<a href="//{{hostname}}/%C4%85/%C4%99">ę</a>`, dnsLinkHostname),
+						Contains(`/ipns/<a href="//{{hostname}}/">{{hostname}}</a>/<a href="//{{hostname}}/%C4%85">ą</a>/<a href="//{{hostname}}/%C4%85/%C4%99">ę</a>`, dnsLink),
 						Contains(`<a href="/%C4%85/%C4%99/file-%C5%BA%C5%82.txt">file-źł.txt</a>`),
 						Contains(`<a class="ipfs-hash" translate="no" href="https://cid.ipfs.tech/#{{cid}}" target="_blank" rel="noreferrer noopener">`, file.Cid()),
 					),
 				),
 		},
-	}...)
+	}
 
-	RunWithSpecs(t, helpers.UnwrapSubdomainTests(t, tests), specs.DNSLinkGateway)
+	// TODO: add explicit proxy tests for specs.DNSLinkGateway
+	RunWithSpecs(t, tests, specs.DNSLinkGateway)
 }
diff --git a/tests/redirects_file_test.go b/tests/redirects_file_test.go
@@ -35,11 +35,12 @@ func TestRedirectsFileSupport(t *testing.T) {
 
 	redirectDirBaseURL := Fmt("{{scheme}}://{{cid}}.ipfs.{{host}}", u.Scheme, redirectDirCID, u.Host)
 
+	// TODO hostHdr := Fmt("{{cid}}.ipfs.{{host}}", redirectDirCID, u.Host)
+
 	tests = append(tests, SugarTests{
 		{
 			Name: "request for $REDIRECTS_DIR_HOSTNAME/redirect-one redirects with default of 301, per _redirects file",
 			Request: Request().
-				Header("Host", u.Host).
 				URL("{{url}}/redirect-one", redirectDirBaseURL),
 			Response: Expect().
 				Status(301).
@@ -235,31 +236,26 @@ func TestRedirectsFileSupport(t *testing.T) {
 func TestRedirectsFileSupportWithDNSLink(t *testing.T) {
 	tooling.LogTestGroup(t, GroupDNSLink)
 	dnsLinks := dnslink.MustOpenDNSLink("redirects_file/dnslink.yml")
-	dnsLink := dnsLinks.MustGet("custom-dnslink")
-
-	u, err := url.Parse(SubdomainGatewayURL)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	dnsLinkBaseUrl := Fmt("{{scheme}}://{{dnslink}}.{{host}}", u.Scheme, dnsLink, u.Host)
+	dnsLink := dnsLinks.MustGet("redirects-examples")
 
 	tests := SugarTests{
 		{
-			Name: "request for $DNSLINK_FQDN/redirect-one redirects with default of 301, per _redirects file",
+			Name: "request for //{dnslink} redirects with default of 301, per _redirects file",
 			Request: Request().
-				URL("{{url}}/redirect-one", dnsLinkBaseUrl),
+				Header("Host", dnsLink).
+				Path("/redirect-one"),
 			Response: Expect().
 				Status(301).
 				Headers(
 					Header("Location", "/one.html"),
 				),
 		},
 		{
-			Name: "request for $DNSLINK_FQDN/en/has-no-redirects-entry returns custom 404, per _redirects file",
+			Name: "request for //{dnslink}/en/has-no-redirects-entry returns custom 404, per _redirects file",
 			Hint: `ensure custom 404 works and has the same cache headers as regular /ipns/ paths`,
 			Request: Request().
-				URL("{{url}}/not-found/has-no-redirects-entry", dnsLinkBaseUrl),
+				Header("Host", dnsLink).
+				Path("/not-found/has-no-redirects-entry"),
 			Response: Expect().
 				Status(404).
 				Headers(
@@ -274,30 +270,69 @@ func TestRedirectsFileSupportWithDNSLink(t *testing.T) {
 		},
 	}
 
-	RunWithSpecs(t, helpers.UnwrapSubdomainTests(t, tests), specs.DNSLinkGateway, specs.RedirectsFile)
+	// TODO:
+	RunWithSpecs(t, tests, specs.DNSLinkGateway, specs.RedirectsFile)
 }
 
 func TestRedirectsFileWithIfNoneMatchHeader(t *testing.T) {
 	fixture := car.MustOpenUnixfsCar("redirects_file/redirects-spa.car")
 
 	dnsLinks := dnslink.MustOpenDNSLink("redirects_file/dnslink.yml")
-	dnsLink := dnsLinks.MustGet("dnslink-spa")
+	dnsLink := dnsLinks.MustGet("redirects-spa")
 
 	u, err := url.Parse(SubdomainGatewayURL)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	pageURL := Fmt("{{scheme}}://{{dnslink}}.{{host}}/missing-page", u.Scheme, dnsLink, u.Host)
+	dnslinkAtSubdomainGw := Fmt("{{dnslink}}.ipns.{{host}}", dnslink.InlineDNS(dnsLink), u.Host)
 
 	var etag string
 
 	RunWithSpecs(t, helpers.UnwrapSubdomainTests(t, SugarTests{
 		{
-			Name: "request for $DNSLINK_FQDN/missing-page returns body of index.html as per _redirects",
+			Name: "request for //{{dnslink}}.ipns.{{subdomain-gateway}}/missing-page returns body of index.html as per _redirects",
+			Request: Request().
+				Path("/missing-page").
+				Headers(
+					Header("Host", dnslinkAtSubdomainGw),
+					Header("Accept", "text/html"),
+				),
+			Response: Expect().
+				Status(200).
+				Headers(
+					Header("Etag").
+						Checks(func(v string) bool {
+							etag = v
+							return v != ""
+						}),
+				).
+				Body(fixture.MustGetRawData("index.html")),
+		},
+	}), specs.SubdomainGatewayIPNS, specs.RedirectsFile)
+
+	RunWithSpecs(t, helpers.UnwrapSubdomainTests(t, SugarTests{
+		{
+			Name: "request for //{dnslink}.ipns.{subdomain-gw}/missing-page with If-None-Match returns 304",
+			Request: Request().
+				Path("/missing-page").
+				Headers(
+					Header("Host", dnslinkAtSubdomainGw),
+					Header("Accept", "text/html"),
+					Header("If-None-Match", etag),
+				),
+			Response: Expect().
+				Status(304),
+		},
+	}), specs.SubdomainGatewayIPNS, specs.RedirectsFile)
+
+	RunWithSpecs(t, helpers.UnwrapSubdomainTests(t, SugarTests{
+		{
+			Name: "request for //{dnslink}/missing-page returns body of index.html as per _redirects",
 			Request: Request().
-				URL(pageURL).
+				Path("/missing-page").
 				Headers(
+					Header("Host", dnsLink),
 					Header("Accept", "text/html"),
 				),
 			Response: Expect().
@@ -315,10 +350,11 @@ func TestRedirectsFileWithIfNoneMatchHeader(t *testing.T) {
 
 	RunWithSpecs(t, helpers.UnwrapSubdomainTests(t, SugarTests{
 		{
-			Name: "request for $DNSLINK_FQDN/missing-page with If-None-Match returns 301",
+			Name: "request for //{dnslink}/missing-page with If-None-Match returns 304",
 			Request: Request().
-				URL(pageURL).
+				Path("/missing-page").
 				Headers(
+					Header("Host", dnsLink),
 					Header("Accept", "text/html"),
 					Header("If-None-Match", etag),
 				),

diff --git a/tooling/helpers/subdomain.go b/tooling/helpers/subdomain.go
@@ -33,21 +33,26 @@ func unwrapSubdomainTest(t *testing.T, unwraped test.SugarTest) test.SugarTests
 	if err != nil {
 		t.Fatal(err)
 	}
-	// Because you might be testing an IPFS node in CI, or on your local machine, the test are designed
-	// to test the subdomain behavior (querying http://{CID}.my-subdomain-gateway.io/) even if the node is
-	// actually living on http://127.0.0.1:8080 or somewhere else.
-	//
-	// The test knows two addresses:
-	// 		- GatewayURL: the URL we connect to, it might be "dweb.link", "127.0.0.1:8080", etc.
-	// 		- SubdomainGatewayURL: the origin that informs value in Host HTTP header used for subdomain requests, it might be "dweb.link", "localhost", "example.com", etc.
 
-	// host is the subdomain ggateway origin we are testing, it might be `localhost` or `example.com`
-	host := u.Host
+	host := req.GetHeader("Host")
+	if host == "" {
+		// if no custom Host header was defined, create explicit one based on the Request.GetURL()
+		host = u.Host
+	}
 
-	// rawURL is the low level HTTP endpoint that is supposed to understand Host header (it might be `http://127.0.0.1/ipfs/something`)
+	// change the the low level HTTP endpoint to one defined via --gateway-url
+	// to allow testing Host-based logic against arbitrary gateway URL (useful on CI)
 	u.Host = test.GatewayHost
+
 	rawURL := u.String()
 
+	// TODO: we want to refactor this magic into explicit Proxy test suite.
+	// Having this magic here silently modifies headers such as Host, and if a
+	// test fails, it is difficult to grasp how much really  is broken, because
+	// number of errors is always multiplied x3. We should have standalone
+	// proxy test for subdomain gateway and dnslink (simple GET should be
+	// enough) and remove need for UnwrapSubdomainTests.
+
 	return test.SugarTests{
 		{
 			Name: fmt.Sprintf("%s (direct HTTP)", unwraped.Name),

diff --git a/tooling/test/sugar.go b/tooling/test/sugar.go
@@ -52,12 +52,24 @@ func (r RequestBuilder) Query(key, value string, args ...any) RequestBuilder {
 
 func (r RequestBuilder) GetURL() string {
 	if r.Path_ != "" {
-		panic("not supported")
+		// This seems to be some tech debt. Generally, we want to move away from URL,
+		// and instead just provide Path + Host header
+		panic("GetURL() is not supported when Path is provided instead of full URL")
 	}
 
 	return r.URL_
 }
 
+func (r RequestBuilder) GetHeader(hdr string) string {
+	if r.Headers_ != nil {
+		v, ok := r.Headers_[hdr]
+		if ok {
+			return v
+		}
+	}
+	return ""
+}
+
 func (r RequestBuilder) Proxy(path string, args ...any) RequestBuilder {
 	r.Proxy_ = tmpl.Fmt(path, args...)
 	return r