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.

Sign up for GitHub

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

Jump to bottom

feat(gw): support UnixFS 1.5 on gateway responses as Last-Modified #659

Merged
merged 4 commits into from
Aug 28, 2024
Merged

feat(gw): support UnixFS 1.5 on gateway responses as Last-Modified #659

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1fcc68b
feat(gw): set Last-Modified to value from dag-pb
lidel Aug 19, 2024
d634588
test: file support for If-Modified-Since
lidel Aug 27, 2024
c833ab9
docs(changelog): If-Modified-Since
lidel Aug 27, 2024
2e946d1
test: before and after If-Modified-Since
lidel Aug 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
172 changes: 106 additions & 66 deletions gateway/gateway_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,72 +93,6 @@ func TestGatewayGet(t *testing.T) {
}
}

// Testing a DAG with (optional) UnixFS1.5 modification time
func TestHeadersUnixFSModeModTime(t *testing.T) {
t.Parallel()

ts, _, root := newTestServerAndNode(t, "unixfs-dir-with-mode-mtime.car")
var (
rootCID = root.String() // "bafybeidbcy4u6y55gsemlubd64zk53xoxs73ifd6rieejxcr7xy46mjvky"
filePath = "/ipfs/" + rootCID + "/file1"
dirPath = "/ipfs/" + rootCID + "/dir1/"
)

t.Run("If-Modified-Since matching UnixFS 1.5 modtime returns Not Modified", func(t *testing.T) {
test := func(responseFormat string, path string, entityType string, supported bool) {
t.Run(fmt.Sprintf("%s/%s support=%t", responseFormat, entityType, supported), func(t *testing.T) {
// Make regular request and read Last-Modified
url := ts.URL + path
req := mustNewRequest(t, http.MethodGet, url, nil)
req.Header.Add("Accept", responseFormat)
res := mustDoWithoutRedirect(t, req)
_, err := io.Copy(io.Discard, res.Body)
require.NoError(t, err)
defer res.Body.Close()
require.Equal(t, http.StatusOK, res.StatusCode)
lastModified := res.Header.Get("Last-Modified")
if supported {
assert.NotEmpty(t, lastModified)
} else {
assert.Empty(t, lastModified)
}

// Make second request with If-Modified-Since and value read from response to first request
req = mustNewRequest(t, http.MethodGet, url, nil)
req.Header.Add("Accept", responseFormat)
req.Header.Add("If-Modified-Since", lastModified)
res = mustDoWithoutRedirect(t, req)
_, err = io.Copy(io.Discard, res.Body)
require.NoError(t, err)
defer res.Body.Close()
if supported {
assert.Equal(t, http.StatusNotModified, res.StatusCode)
} else {
assert.Equal(t, http.StatusOK, res.StatusCode)
}
})
}

file, dir := "file", "directory"
// supported on file-based web responses
test("", filePath, file, true)
test("text/html", filePath, file, true)

// not supported on other formats
// we may implement support for If-Modified-Since for below request types
// if users raise the need, but If-None-Match is way better
test(carResponseFormat, filePath, file, false)
test(rawResponseFormat, filePath, file, false)
test(tarResponseFormat, filePath, file, false)

test("", dirPath, dir, false)
test("text/html", dirPath, dir, false)
test(carResponseFormat, dirPath, dir, false)
test(rawResponseFormat, dirPath, dir, false)
test(tarResponseFormat, dirPath, dir, false)
})
}

func TestHeaders(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -615,6 +549,112 @@ func TestHeaders(t *testing.T) {
})
}

// Testing a DAG with (optional) UnixFS1.5 modification time
func TestHeadersUnixFSModeModTime(t *testing.T) {
t.Parallel()

ts, _, root := newTestServerAndNode(t, "unixfs-dir-with-mode-mtime.car")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ created with Kubo 0.30.0-rc1

var (
rootCID = root.String() // "bafybeidbcy4u6y55gsemlubd64zk53xoxs73ifd6rieejxcr7xy46mjvky"
filePath = "/ipfs/" + rootCID + "/file1"
dirPath = "/ipfs/" + rootCID + "/dir1/"
)

t.Run("If-Modified-Since matching UnixFS 1.5 modtime returns Not Modified", func(t *testing.T) {
test := func(responseFormat string, path string, entityType string, supported bool) {
t.Run(fmt.Sprintf("%s/%s support=%t", responseFormat, entityType, supported), func(t *testing.T) {
// Make regular request and read Last-Modified
url := ts.URL + path
req := mustNewRequest(t, http.MethodGet, url, nil)
req.Header.Add("Accept", responseFormat)
res := mustDoWithoutRedirect(t, req)
_, err := io.Copy(io.Discard, res.Body)
require.NoError(t, err)
defer res.Body.Close()
require.Equal(t, http.StatusOK, res.StatusCode)
lastModified := res.Header.Get("Last-Modified")
if supported {
assert.NotEmpty(t, lastModified)
} else {
assert.Empty(t, lastModified)
lastModified = "Mon, 13 Jun 2022 22:18:32 GMT" // manually set value for use in next steps
}

ifModifiedSinceTime, err := time.Parse(time.RFC1123, lastModified)
require.NoError(t, err)
oneHourBefore := ifModifiedSinceTime.Add(-1 * time.Hour).Truncate(time.Second)
oneHourAfter := ifModifiedSinceTime.Add(1 * time.Hour).Truncate(time.Second)
oneHourBeforeStr := oneHourBefore.Format(time.RFC1123)
oneHourAfterStr := oneHourAfter.Format(time.RFC1123)
lastModifiedStr := ifModifiedSinceTime.Format(time.RFC1123)

// Make second request with If-Modified-Since and value read from response to first request
req = mustNewRequest(t, http.MethodGet, url, nil)
req.Header.Add("Accept", responseFormat)
req.Header.Add("If-Modified-Since", lastModifiedStr)
res = mustDoWithoutRedirect(t, req)
_, err = io.Copy(io.Discard, res.Body)
require.NoError(t, err)
defer res.Body.Close()
if supported {
// 304 on exact match, can skip body
assert.Equal(t, http.StatusNotModified, res.StatusCode)
} else {
assert.Equal(t, http.StatusOK, res.StatusCode)
}

// Make third request with If-Modified-Since 1h before value read from response to first request
// and expect HTTP 200
req = mustNewRequest(t, http.MethodGet, url, nil)
req.Header.Add("Accept", responseFormat)
req.Header.Add("If-Modified-Since", oneHourBeforeStr)
res = mustDoWithoutRedirect(t, req)
_, err = io.Copy(io.Discard, res.Body)
require.NoError(t, err)
defer res.Body.Close()
// always return 200 with body because mtime from unixfs is after value from If-Modified-Since
// so it counts as an update
assert.Equal(t, http.StatusOK, res.StatusCode)

// Make third request with If-Modified-Since 1h after value read from response to first request
// and expect HTTP 200
req = mustNewRequest(t, http.MethodGet, url, nil)
req.Header.Add("Accept", responseFormat)
req.Header.Add("If-Modified-Since", oneHourAfterStr)
res = mustDoWithoutRedirect(t, req)
_, err = io.Copy(io.Discard, res.Body)
require.NoError(t, err)
defer res.Body.Close()
if supported {
// 304 because mtime from unixfs is before value from If-Modified-Since
// so no update, can skip body
assert.Equal(t, http.StatusNotModified, res.StatusCode)
} else {
assert.Equal(t, http.StatusOK, res.StatusCode)
}
})
}

file, dir := "file", "directory"
// supported on file-based web responses
test("", filePath, file, true)
test("text/html", filePath, file, true)

// not supported on other formats
// we may implement support for If-Modified-Since for below request types
// if users raise the need, but If-None-Match is way better
test(carResponseFormat, filePath, file, false)
test(rawResponseFormat, filePath, file, false)
test(tarResponseFormat, filePath, file, false)

test("", dirPath, dir, false)
test("text/html", dirPath, dir, false)
test(carResponseFormat, dirPath, dir, false)
test(rawResponseFormat, dirPath, dir, false)
test(tarResponseFormat, dirPath, dir, false)
})
}

func TestGoGetSupport(t *testing.T) {
ts, _, root := newTestServerAndNode(t, "fixtures.car")

Expand Down
2 changes: 1 addition & 1 deletion gateway/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,8 @@

// Detect when If-Modified-Since HTTP header + UnixFS 1.5 allow returning HTTP 304 Not Modified.
if i.handleIfModifiedSince(w, r, rq) {
return
}

Check warning on line 306 in gateway/handler.go

View check run for this annotation

Codecov / codecov/patch

gateway/handler.go#L305-L306 

Added lines #L305 - L306 were not covered by tests

// Support custom response formats passed via ?format or Accept HTTP header
switch responseFormat {
Expand Down Expand Up @@ -419,8 +419,8 @@
// built into modern browsers: https://github.com/ipfs/kubo/pull/8074#pullrequestreview-645196768
modtime = time.Now()
} else {
// set Last-Modified to a meaningful value e.g. one read from dag-pb (UnixFS 1.5, mtime field)
// or the last time DNSLink / IPNS Record was modified / resoved or cache

Check warning on line 423 in gateway/handler.go

View check run for this annotation

Codecov / codecov/patch

gateway/handler.go#L422-L423 

Added lines #L422 - L423 were not covered by tests
modtime = lastMod
}

Expand Down Expand Up @@ -526,12 +526,12 @@
if ifModifiedSinceHeader == "" || lastModified.IsZero() {
return false
}
ifModifiedSinceTime, err := time.Parse(time.RFC1123, ifModifiedSinceHeader)
if err != nil {
return false
}

Check warning on line 532 in gateway/handler.go

View check run for this annotation

Codecov / codecov/patch

gateway/handler.go#L529-L532 

Added lines #L529 - L532 were not covered by tests
// ignoring fractional seconds (as HTTP dates don't include fractional seconds)
return lastModified.Truncate(time.Second).After(ifModifiedSinceTime)
return !lastModified.Truncate(time.Second).After(ifModifiedSinceTime)

Check warning on line 534 in gateway/handler.go

View check run for this annotation

Codecov / codecov/patch

gateway/handler.go#L534 

Added line #L534 was not covered by tests
}

// etagMatch evaluates if we can respond with HTTP 304 Not Modified
Expand Down Expand Up @@ -782,35 +782,35 @@
// Resolve path to be able to read pathMetadata.ModTime
pathMetadata, err := i.backend.ResolvePath(r.Context(), rq.immutablePath)
if err != nil {
var forwardedPath path.ImmutablePath
var continueProcessing bool
if isWebRequest(rq.responseFormat) {
forwardedPath, continueProcessing = i.handleWebRequestErrors(w, r, rq.mostlyResolvedPath(), rq.immutablePath, rq.contentPath, err, rq.logger)
if continueProcessing {
pathMetadata, err = i.backend.ResolvePath(r.Context(), forwardedPath)
}

Check warning on line 791 in gateway/handler.go

View check run for this annotation

Codecov / codecov/patch

gateway/handler.go#L785-L791 

Added lines #L785 - L791 were not covered by tests
}
if !continueProcessing || err != nil {
err = fmt.Errorf("failed to resolve %s: %w", debugStr(rq.contentPath.String()), err)
i.webError(w, r, err, http.StatusInternalServerError)
return true
}

Check warning on line 797 in gateway/handler.go

View check run for this annotation

Codecov / codecov/patch

gateway/handler.go#L793-L797 

Added lines #L793 - L797 were not covered by tests
}

// Currently we only care about optional mtime from UnixFS 1.5 (dag-pb)
// but other sources of this metadata could be added in the future
lastModified := pathMetadata.ModTime
if lastModifiedMatch(ifModifiedSince, lastModified) {
w.WriteHeader(http.StatusNotModified)
return true
}

Check warning on line 806 in gateway/handler.go

View check run for this annotation

Codecov / codecov/patch

gateway/handler.go#L804-L806 

Added lines #L804 - L806 were not covered by tests

// Check if the resolvedPath is an immutable path.
_, err = path.NewImmutablePath(pathMetadata.LastSegment)
if err != nil {
i.webError(w, r, err, http.StatusInternalServerError)
return true
}

Check warning on line 813 in gateway/handler.go

View check run for this annotation

Codecov / codecov/patch

gateway/handler.go#L811-L813 

Added lines #L811 - L813 were not covered by tests

rq.pathMetadata = &pathMetadata
return false
Expand Down
Loading