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

feat: basic support for Ipfs-Path-Affinity from IPIP-462 #592

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ The following emojis are used to highlight certain changes:
### Added

* `routing/http/server` now adds `Cache-Control` HTTP header to GET requests: 15 seconds for empty responses, or 5 minutes for responses with providers.
* `gateway` now supports optional `Ipfs-Path-Affinity` hints from [IPIP-462](https://github.com/ipfs/specs/pull/462).

### Changed

Expand Down
101 changes: 100 additions & 1 deletion gateway/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,8 @@
return
}

i.handlePathAffinityHints(w, r, contentPath, logger)
Copy link
Member

Choose a reason for hiding this comment

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

What about only calling this if it's a request for raw blocks or CAR files as indicated here? Or do you want to extend the scope?


Copy link
Member

Choose a reason for hiding this comment

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

As someone not familiar with go nor this codebase, these variable names need work 😳

Copy link
Contributor

Choose a reason for hiding this comment

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

@SgtPooki In Go HTTP server request handlers, w and r are pretty much convention for http.ResponseWriter and *http.Request respectively. This is also seen the Go docs and examples.

// Detect when explicit Accept header or ?format parameter are present
responseFormat, formatParams, err := customResponseFormat(r)
if err != nil {
Expand Down Expand Up @@ -752,7 +754,7 @@
}

// Detect 'Cache-Control: only-if-cached' in request and return data if it is already in the local datastore.
// https://github.com/ipfs/specs/blob/main/http-gateways/PATH_GATEWAY.md#cache-control-request-header
// https://specs.ipfs.tech/http-gateways/path-gateway/#cache-control-request-header
func (i *handler) handleOnlyIfCached(w http.ResponseWriter, r *http.Request, contentPath path.Path) bool {
if r.Header.Get("Cache-Control") == "only-if-cached" {
if !i.backend.IsCached(r.Context(), contentPath) {
Expand Down Expand Up @@ -887,6 +889,103 @@
return true
}

// Detect 'Ipfs-Path-Affinity' (IPIP-462) headers in request and use values as a content
// routing hints if passed paths are not already in the local datastore.
// These optional hints are mostly useful for trustless block requests.
// See https://github.com/ipfs/specs/pull/462
func (i *handler) handlePathAffinityHints(w http.ResponseWriter, r *http.Request, contentPath path.Path, logger *zap.SugaredLogger) {
headerName := "Ipfs-Path-Affinity"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
headerName := "Ipfs-Path-Affinity"
const headerName = "Ipfs-Path-Affinity"

// Skip if no header
if r.Header.Get(headerName) == "" {
return
}
// Skip if contentPath is already locally cached
if i.backend.IsCached(r.Context(), contentPath) {
return
}

Check warning on line 905 in gateway/handler.go

View check run for this annotation

Codecov / codecov/patch

gateway/handler.go#L903-L905

Added lines #L903 - L905 were not covered by tests
// Check canonical header name
// NOTE: we don't use r.Header.Get() because client can send this header more than once
headerValues := r.Header[headerName]
// If not found, try lowercase version.
// NOTE: this is done manually because direct key access does not come with canonicalization, like Header.Get() does
if len(headerValues) == 0 {
headerValues = r.Header[strings.ToLower(headerName)]
}

Check warning on line 913 in gateway/handler.go

View check run for this annotation

Codecov / codecov/patch

gateway/handler.go#L908-L913

Added lines #L908 - L913 were not covered by tests

// Limit the headerValues to the first 3 items (abuse protection)
if len(headerValues) > 3 {
headerValues = headerValues[:3]
}

Check warning on line 918 in gateway/handler.go

View check run for this annotation

Codecov / codecov/patch

gateway/handler.go#L916-L918

Added lines #L916 - L918 were not covered by tests

// Process affinity hints
for _, headerValue := range headerValues {
// Non-ascii paths are percent-encoded.
// Decode if the value starts with %2F (percent-encoded '/')
if strings.HasPrefix(headerValue, "%2F") {
decodedValue, err := url.PathUnescape(headerValue)
if err != nil {
logger.Debugw("skipping invalid Ipfs-Path-Affinity hint", "error", err)
continue

Check warning on line 928 in gateway/handler.go

View check run for this annotation

Codecov / codecov/patch

gateway/handler.go#L921-L928

Added lines #L921 - L928 were not covered by tests
}
headerValue = decodedValue

Check warning on line 930 in gateway/handler.go

View check run for this annotation

Codecov / codecov/patch

gateway/handler.go#L930

Added line #L930 was not covered by tests
}
// Confirm it is a valid content path
affinityPath, err := path.NewPath(headerValue)
if err != nil {
logger.Debugw("skipping invalid Ipfs-Path-Affinity hint", "error", err)

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

Sensitive data returned by HTTP request headers
flows to a logging call.
Sensitive data returned by HTTP request headers
flows to a logging call.
continue

Check warning on line 936 in gateway/handler.go

View check run for this annotation

Codecov / codecov/patch

gateway/handler.go#L933-L936

Added lines #L933 - L936 were not covered by tests
}

// Skip duplicated work if immutable affinity hint is a subset of requested immutable contentPath
// (protect against broken clients that use affinity incorrectly)
if !contentPath.Mutable() && !affinityPath.Mutable() && strings.HasPrefix(contentPath.String(), affinityPath.String()) {
Comment on lines +939 to +941
Copy link
Contributor

@gammazero gammazero Jun 24, 2024

Choose a reason for hiding this comment

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

The comment is a little confusing about which is a prefix of the other.

if immutable affinity hint is a subset of requested immutable contentPath

It could mean that the contentPath should be the prefix, since the subset is a more specific (longer) path:

contentPath = "/top-level"
affinityPath = "/top-level/sub-path"

or it could me that the affinity path is a string subset of the longer content path:

contentPath = "/top-level/stuff/sub-stuff"
affinityPath = "/top-level/stuff"

logger.Debugw("skipping redundant Ipfs-Path-Affinity hint", "affinity", affinityPath)

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

Sensitive data returned by HTTP request headers
flows to a logging call.
Sensitive data returned by HTTP request headers
flows to a logging call.
continue

Check warning on line 943 in gateway/handler.go

View check run for this annotation

Codecov / codecov/patch

gateway/handler.go#L941-L943

Added lines #L941 - L943 were not covered by tests
}

// Process hint in background without blocking response logic for contentPath
go func(contentPath path.Path, affinityPath path.Path, logger *zap.SugaredLogger) {
var immutableAffinityPath path.ImmutablePath
logger.Debugw("async processing of Ipfs-Path-Affinity hint", "affinity", affinityPath)

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

Sensitive data returned by HTTP request headers
flows to a logging call.
Sensitive data returned by HTTP request headers
flows to a logging call.
if affinityPath.Mutable() {
// Skip work if mutable affinity hint is a subset of mutable contentPath
if contentPath.Mutable() && strings.HasPrefix(contentPath.String(), affinityPath.String()) {
Copy link
Member

Choose a reason for hiding this comment

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

Seems like hasPrefix check could be pulled out into a more semantic function since it's used at least twice.

logger.Debugw("skipping redundant Ipfs-Path-Affinity hint", "affinity", affinityPath)

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

Sensitive data returned by HTTP request headers
flows to a logging call.
Sensitive data returned by HTTP request headers
flows to a logging call.
return
}
immutableAffinityPath, _, _, err = i.backend.ResolveMutable(r.Context(), affinityPath)
if err != nil {
logger.Debugw("error while resolving mutable Ipfs-Path-Affinity hint", "affinity", affinityPath, "error", err)

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

Sensitive data returned by HTTP request headers
flows to a logging call.
Sensitive data returned by HTTP request headers
flows to a logging call.

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

Sensitive data returned by HTTP request headers
flows to a logging call.
Sensitive data returned by HTTP request headers
flows to a logging call.
return
}
} else {
ipath, ok := affinityPath.(path.ImmutablePath)
if !ok {
return
}
immutableAffinityPath = ipath

Check warning on line 966 in gateway/handler.go

View check run for this annotation

Codecov / codecov/patch

gateway/handler.go#L947-L966

Added lines #L947 - L966 were not covered by tests
}
// Skip if affinity path is already cached
if !i.backend.IsCached(r.Context(), immutableAffinityPath) {
// The intention of below code is to asynchronously preconnect
// gateway with providers of the affinityPath in
// Ipfs-Path-Affinity hint. Once connected, these peers can be
// asked directly (via mechanism like bitswap) for blocks
// related to main request for contentPath, and retrieve them,
// even when no other routing system had them announced. If
// original contentPath was received and returned to HTTP
// client before below get is done, the work is cancelled.

logger.Debugw("started async search for providers of Ipfs-Path-Affinity hint", "affinity", affinityPath)

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

Sensitive data returned by HTTP request headers
flows to a logging call.
Sensitive data returned by HTTP request headers
flows to a logging call.
_, _, err = i.backend.GetBlock(r.Context(), immutableAffinityPath)
logger.Debugw("ended async search for providers of Ipfs-Path-Affinity hint", "affinity", affinityPath, "error", err)

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

Sensitive data returned by HTTP request headers
flows to a logging call.
Sensitive data returned by HTTP request headers
flows to a logging call.

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

Sensitive data returned by HTTP request headers
flows to a logging call.
Sensitive data returned by HTTP request headers
flows to a logging call.
} else {
logger.Debugw("skipping Ipfs-Path-Affinity hint due to data being locally cached", "affinity", affinityPath)

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

Sensitive data returned by HTTP request headers
flows to a logging call.
Sensitive data returned by HTTP request headers
flows to a logging call.
}

Check warning on line 984 in gateway/handler.go

View check run for this annotation

Codecov / codecov/patch

gateway/handler.go#L969-L984

Added lines #L969 - L984 were not covered by tests
Comment on lines +969 to +984
Copy link
Contributor

Choose a reason for hiding this comment

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

minor: Instead of if .. else, check if cached and return if it is.

Suggested change
if !i.backend.IsCached(r.Context(), immutableAffinityPath) {
// The intention of below code is to asynchronously preconnect
// gateway with providers of the affinityPath in
// Ipfs-Path-Affinity hint. Once connected, these peers can be
// asked directly (via mechanism like bitswap) for blocks
// related to main request for contentPath, and retrieve them,
// even when no other routing system had them announced. If
// original contentPath was received and returned to HTTP
// client before below get is done, the work is cancelled.
logger.Debugw("started async search for providers of Ipfs-Path-Affinity hint", "affinity", affinityPath)
_, _, err = i.backend.GetBlock(r.Context(), immutableAffinityPath)
logger.Debugw("ended async search for providers of Ipfs-Path-Affinity hint", "affinity", affinityPath, "error", err)
} else {
logger.Debugw("skipping Ipfs-Path-Affinity hint due to data being locally cached", "affinity", affinityPath)
}
if i.backend.IsCached(r.Context(), immutableAffinityPath) {
logger.Debugw("skipping Ipfs-Path-Affinity hint due to data being locally cached", "affinity", affinityPath)
return
}
// The intention of below code is to asynchronously preconnect
// gateway with providers of the affinityPath in
// Ipfs-Path-Affinity hint. Once connected, these peers can be
// asked directly (via mechanism like bitswap) for blocks
// related to main request for contentPath, and retrieve them,
// even when no other routing system had them announced. If
// original contentPath was received and returned to HTTP
// client before below get is done, the work is cancelled.
logger.Debugw("started async search for providers of Ipfs-Path-Affinity hint", "affinity", affinityPath)
_, _, err = i.backend.GetBlock(r.Context(), immutableAffinityPath)
logger.Debugw("ended async search for providers of Ipfs-Path-Affinity hint", "affinity", affinityPath, "error", err)

}(contentPath, affinityPath, logger)
}
}

// getTemplateGlobalData returns the global data necessary by most templates.
func (i *handler) getTemplateGlobalData(r *http.Request, contentPath path.Path) assets.GlobalData {
// gatewayURL is used to link to other root CIDs. THis will be blank unless
Expand Down
Loading