Skip to content

Cleanup / fix delegated routing response headers#241

Open
byo wants to merge 2 commits into
mainfrom
cleanup-delegated-routing-response-headers
Open

Cleanup / fix delegated routing response headers#241
byo wants to merge 2 commits into
mainfrom
cleanup-delegated-routing-response-headers

Conversation

@byo
Copy link
Copy Markdown
Contributor

@byo byo commented May 7, 2026

  • Always send X-Content-Type-Options: nosniff to prevent detection of content type
  • Add Vary: Accept to inform clients that the response depends on the Accept header
  • Fix Allow header when invalid method was used
  • Always send 404 Not found with text/plain content type
  • Always send the Cache-control: public header

@byo byo requested review from gammazero and willscott May 7, 2026 16:08
byo added 2 commits May 7, 2026 18:12
* Always send `X-Content-Type-Options: nosniff`
  to prevent detection of content type
* Add `Vary: Accept` to inform clients that the response
  depends on the Accept header
* Fix `Allow` header when invalid method was used
* Always send 404 Not found with `text/plain` content type
* Always send the `Cache-control: public` header
@byo byo force-pushed the cleanup-delegated-routing-response-headers branch from 37b7302 to 12a1396 Compare May 7, 2026 16:13
Copy link
Copy Markdown

@nymd nymd left a comment

Choose a reason for hiding this comment

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

Worth addressing? but not a blocker

Comment thread delegated_translator.go
h.Add("X-Content-Type-Options", "nosniff")
h.Add("Vary", "Accept")
h.Add("Cache-Control", "public")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe max-age is a good addition as well so we are specific about acceptable staleness.

Copy link
Copy Markdown

@lidel lidel May 8, 2026

Choose a reason for hiding this comment

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

fysa specs for /routing/v1 have some suggested values to work well with HTTP caches https://specs.ipfs.tech/routing/http-routing-v1/#response-headers

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, will extend the PR. But that's something to be easily configured through env vars, and with both max-age and s-maxage so that we can easily control cacheability on both the front cache and clients.

What I would like to avoid is for the client to aggressively cache 404 not-found responses. If that happens then we have no way of purging that cache. So what I believe would be a good solution is:

  • For 200 respones use max-age with the same TTL as what is configured in cloudfront ATM, specify s-maxage to the same value
  • For 404 responses do not specify max-age but set s-maxage to the value that is configured as defaultTTL for 404 error response in cloudfront. This should result in client re-querying for the data on every subsequent request, but cloudfront would still keep this in cache protecting our backend from too much traffic.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@lidel thanks, will take a look at this as well

@nymd nymd assigned byo May 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants