From 120678cd3aa10ebd6ec18258404f7fc08308d137 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Fri, 24 May 2024 23:01:02 +0200 Subject: [PATCH] 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 --- .github/workflows/test-kubo-e2e.yml | 8 +-- fixtures/redirects_file/dnslink.yml | 6 +-- tests/dnslink_gateway_test.go | 33 ++++--------- tests/redirects_file_test.go | 76 +++++++++++++++++++++-------- tooling/helpers/subdomain.go | 52 +++++++++++++------- tooling/test/sugar.go | 14 +++++- 6 files changed, 118 insertions(+), 71 deletions(-) diff --git a/.github/workflows/test-kubo-e2e.yml b/.github/workflows/test-kubo-e2e.yml index 205fdd2d3..60e65b090 100644 --- 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 index 69c5f2cc7..0bafb8867 100644 --- 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 index 15c8614cd..bfa5ba6a4 100644 --- a/tests/dnslink_gateway_test.go +++ b/tests/dnslink_gateway_test.go @@ -1,7 +1,6 @@ package tests import ( - "net/url" "testing" "github.com/ipfs/gateway-conformance/tooling" @@ -11,7 +10,6 @@ import ( "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 +18,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 +38,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 +49,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 +66,13 @@ func TestDNSLinkGatewayUnixFSDirectoryListing(t *testing.T) { And( Contains("Index of"), Contains(`..`), - Contains(`/ipns/{{hostname}}/ą/ę`, dnsLinkHostname), + Contains(`/ipns/{{hostname}}/ą/ę`, dnsLink), Contains(`file-źł.txt`), Contains(``, file.Cid()), ), ), }, - }...) + } RunWithSpecs(t, helpers.UnwrapSubdomainTests(t, tests), specs.DNSLinkGateway) } diff --git a/tests/redirects_file_test.go b/tests/redirects_file_test.go index 20dca0048..1a667b3d9 100644 --- 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,20 +236,14 @@ 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( @@ -256,10 +251,11 @@ func TestRedirectsFileSupportWithDNSLink(t *testing.T) { ), }, { - 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 index 06d04f19f..8dbd8f325 100644 --- a/tooling/helpers/subdomain.go +++ b/tooling/helpers/subdomain.go @@ -25,28 +25,46 @@ func UnwrapSubdomainTests(t *testing.T, tests test.SugarTests) test.SugarTests { func unwrapSubdomainTest(t *testing.T, unwraped test.SugarTest) test.SugarTests { t.Helper() - baseURL := unwraped.Request.GetURL() + var baseURL, rawURL string req := unwraped.Request expected := unwraped.Response + host := req.GetHeader("Host") + if host != "" { + // when custom Host header and Path are present we skip legacy magic + // and use them as-is + u, err := url.Parse(test.GatewayURL) + if err != nil { + panic("failed to parse GatewayURL") + } + // rawURL is gateway-url + Path + u.Path = unwraped.Request.Path_ + unwraped.Request.Path_ = "" + rawURL = u.String() + // baseURL is rawURL with hostname from Host header + u.Host = host + baseURL = u.String() + } else { + // Legacy flow based on URL instead of Host header + baseURL := unwraped.Request.GetURL() - u, err := url.Parse(baseURL) - 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. + u, err := url.Parse(baseURL) + if err != nil { + t.Fatal(err) + } + + // change 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 - // host is the subdomain ggateway origin we are testing, it might be `localhost` or `example.com` - host := u.Host + rawURL = u.String() + } - // rawURL is the low level HTTP endpoint that is supposed to understand Host header (it might be `http://127.0.0.1/ipfs/something`) - 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{ { diff --git a/tooling/test/sugar.go b/tooling/test/sugar.go index 5c5a7a2a9..705270661 100644 --- 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("calling GetURL() is not supported when Path is set") } 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