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

Improve subdomain detection logic #249

Open
lidel opened this issue May 9, 2024 · 2 comments
Open

Improve subdomain detection logic #249

lidel opened this issue May 9, 2024 · 2 comments
Labels
effort/hours Estimated to take one or several hours enhancement New feature or request exp/intermediate Prior experience is likely helpful kind/architecture Core architecture of project kind/enhancement A net-new feature or improvement to an existing feature P1 High: Likely tackled by core team if no one steps up status/ready Ready to be worked

Comments

@lidel
Copy link
Member

lidel commented May 9, 2024

Problem: brittle subdomain mode support detection

Right now we make request to bafkqaaa on subdomain via fetch:

https://github.com/ipfs-shipyard/service-worker-gateway/blob/289aca764da893789508c56d7f9e0caeb5711d90/src/lib/path-or-subdomain.ts#L35-L39

This is too brittle, because it will likely break once people start deployign their own instances without doing all the steps we did:

  • false-positive if server is a regular subdomain gateway (e.g.g rainbow returns HTTP 200) just fine
  • false-negative if subdomains have no CORS safelist or other headers

Solution: switch to img onload

We should have a static /1x1.png in public/ and request that asset specifically.

This solves two problems:

  • confirming subdomain response is SW payload, and nothing else (solves false-positive)
  • image loads not impacted by CORS, onload will succeed even without * CORS (solves false-negative)
@lidel lidel added the need/triage Needs initial labeling and prioritization label May 9, 2024
@lidel lidel added this to the Prod: Initial release milestone May 9, 2024
@lidel lidel added kind/enhancement A net-new feature or improvement to an existing feature and removed need/triage Needs initial labeling and prioritization labels May 9, 2024
@SgtPooki
Copy link
Member

SgtPooki commented May 9, 2024

This will also resolve the "test this once" todo because it will be cached and return instantly with subsequent requests.

@SgtPooki
Copy link
Member

SgtPooki commented May 9, 2024

one gotcha with the suggested fix is that this is also called from sw.ts code, and image onload won't work there, so we won't be able to do redirects from sw anymore, which will result in a little more assets rendered than we need.. but we have the redirect already working in client side code so it shouldn't break anything.

@SgtPooki SgtPooki added enhancement New feature or request effort/hours Estimated to take one or several hours exp/intermediate Prior experience is likely helpful kind/architecture Core architecture of project P1 High: Likely tackled by core team if no one steps up status/ready Ready to be worked labels Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/hours Estimated to take one or several hours enhancement New feature or request exp/intermediate Prior experience is likely helpful kind/architecture Core architecture of project kind/enhancement A net-new feature or improvement to an existing feature P1 High: Likely tackled by core team if no one steps up status/ready Ready to be worked
Projects
None yet
Development

No branches or pull requests

2 participants