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

πŸ› Bug Report β€” fetch not checking if resolved redirect URL is http(s) #2100

Open
jasnell opened this issue May 9, 2024 · 1 comment

Comments

@jasnell
Copy link
Member

jasnell commented May 9, 2024

Reproduction:

export default {
  async fetch(req, env) {
    if (req.url.endsWith('/sub')) {
      return new Response("Hello World\n", {
        status: 302, headers: {
          location: 'file://what/is/the/url'
        }
      });
    } else {
      const resp = await fetch('http://localhost:8080/sub', { redirect: 'follow'});
      console.log(resp.status);
      console.log(resp.url);
      return new Response("ok");
    }
  }
};

Expected result:

A useful error or proper network error response as defined by the spec

Actual result:

The runtime throws an "internal error" due to a triggered assert in kj/compat/http.c++

What's the issue:

In the initial request, the runtime will validate that the request URL is http:// or https://, with additional special handling around ws:// and wss:// URLs. When a redirect response is processed and we generate a new request URL from the location header, we are not applying the same checks so we end up passing a non-http URL down to the kj http client, which fails with an assertion. We need to be properly validating the redirect url.

@jasnell
Copy link
Member Author

jasnell commented May 10, 2024

Hmm... interesting... further testing is showing this case being handled correctly in some instances. I've seen the assertion in some cases (like the repro above) but not others (which end up returning a proper 404 not found)... weird. Will keep investigating.

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

No branches or pull requests

1 participant