Skip to content

Commit

Permalink
fix(wip): avoid mangling Host header handling
Browse files Browse the repository at this point in the history
- splitting into subdomain and dnslink gateway variants
- towards avoiding running everything three times
- keep custom Host header when defined
  • Loading branch information
lidel committed May 25, 2024
1 parent 3e74a1a commit 120678c
Show file tree
Hide file tree
Showing 6 changed files with 118 additions and 71 deletions.
8 changes: 2 additions & 6 deletions .github/workflows/test-kubo-e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions fixtures/redirects_file/dnslink.yml
Original file line number Diff line number Diff line change
@@ -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
33 changes: 9 additions & 24 deletions tests/dnslink_gateway_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package tests

import (
"net/url"
"testing"

"github.com/ipfs/gateway-conformance/tooling"
Expand All @@ -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) {
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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-`),
Expand All @@ -81,13 +66,13 @@ 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)
}
76 changes: 56 additions & 20 deletions tests/redirects_file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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(
Expand All @@ -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().
Expand All @@ -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),
),
Expand Down
52 changes: 35 additions & 17 deletions tooling/helpers/subdomain.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
{
Expand Down
14 changes: 13 additions & 1 deletion tooling/test/sugar.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 120678c

Please sign in to comment.