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

"Automatic pass through" of query parameters in Redirect rules does not work in netlify dev-served site #1699

Closed
nonAlgebraic opened this issue Jan 6, 2021 · 10 comments
Labels
type: bug code to address defects in shipped code

Comments

@nonAlgebraic
Copy link

nonAlgebraic commented Jan 6, 2021

Describe the bug

A recent and welcome change described in this post on the community forums, wherein redirect rules that don't specify expected query parameters in the "from" path pass all query parameters through to the "to" path by default on the CDN, does not seem to work when using netlify dev locally.

To Reproduce

  1. Clone this minimal reproduction: https://github.com/nonAlgebraic/netlify-cli-redirect-bug-repro
  2. Run yarn install
  3. Run yarn netlify dev. This statrs a servør local server on port 8080 and netlify dev on port 9090
  4. Browse to any path with a query string in the URL, e.g. http://localhost:9090/?foo=bar
  5. See that you are redirected to http://localhost:9090/some-page.html, as the redirect rule defined in netlify.toml would suggest, but that the "?foo=bar" query string is not passed through automatically

Configuration

Contents of netlify.toml:

[dev]
  command = "yarn start"
  framework = "#custom"
  publish = "public"
  targetPort = 8080
  port = 9090

[[redirects]]
  from = "/*"
  to = "/some-page.html"
  status = 301
  force = false

Output of envinfo:

System:
  OS: macOS 11.1
  CPU: (12) x64 Intel(R) Core(TM) i7-8750H CPU @ 2.20GHz
  Memory: 136.63 MB / 16.00 GB
  Shell: 3.2.57 - /bin/bash
Binaries:
  Node: 14.15.0 - ~/.nvm/versions/node/v14.15.0/bin/node
  Yarn: 1.22.10 - ~/projects/netlify-cli-redirect-bug-repro/node_modules/.bin/yarn
  npm: 6.14.8 - ~/.nvm/versions/node/v14.15.0/bin/npm
  Watchman: 4.9.0 - /usr/local/bin/watchman
npmPackages:
  netlify-cli: ^2.70.0 => 2.70.0

Expected behavior

Redirects defined in the config file without specific query parameter settings should pass through any and all query parameters in the requested path to the redireced-to path.

@nonAlgebraic nonAlgebraic added the type: bug code to address defects in shipped code label Jan 6, 2021
@nonAlgebraic nonAlgebraic changed the title "Automatic pass through" of query parameters in Redirect rules does not work "Automatic pass through" of query parameters in Redirect rules does not work in netlify dev-served site Jan 6, 2021
@calavera
Copy link
Contributor

calavera commented Jan 6, 2021

Can you check if that works using the option -t? netlify dev -t, that will use our newest redirect engine that matches more closely to our production infra.

@nonAlgebraic
Copy link
Author

nonAlgebraic commented Jan 6, 2021

@calavera I notice two things when I use the -t option:

1. Requests to any path other than / now redirect with query parameters passed through, yay!
~2. Requests to / are not redirected at all, despite the fact that the single redirect rule has "/*" in its "from" path. Without the -t option, requests to "/" are redirected as well, as expected (but without the query string).

Disregard! That was my own error. Writing a better response now.

@nonAlgebraic
Copy link
Author

@calavera using -t seems to make it work! Awesome!

I did notice one thing: in this particular circumstance (i.e. with servør as the underlying local server, etc.), the -t option also caused requests to the root path ("/") not to be redirected unless the "force" flag was set to true on the redirect rule.

This makes partial sense to me, since servør serves a directory listing page in the absence of an index.html file, which netlify dev may be experiencing as a "real page", thus not redirecting from it without force = "true". However, I am surprised that, without the -t option, requests to "/" were being redirected, still.

@calavera
Copy link
Contributor

calavera commented Jan 6, 2021

However, I am surprised that, without the -t option, requests to "/" were being redirected, still.

Not redirecting is the right behavior, since we don't redirect in production unless there is an explicit force = true in your configuration.

There are two reasons why the behavior is even more different:

  1. When we use the -t flag, we send a HEAD request to servør to verify the shadowing behavior. If it returns a 200, we won't redirect. This response matches what you'd expect in production, although we do it a little bit differently there.
  2. When you don't use the -t flag, redirects always apply, ignoring the force flag completely, see File shadowing not working for netlify dev and redirects always apply. #1299

@nonAlgebraic
Copy link
Author

@calavera that makes perfect sense. Thanks so much for your help! I will close this issue now.

@nonAlgebraic
Copy link
Author

nonAlgebraic commented Jan 7, 2021

@calavera I've encountered a different problem, but am unsure if it deserves a new issue. I am hoping commenting here and asking for your take is cool.

I have noticed that, with the -t option, and with a redirect role like this:

/*  /subpath/:splat 301!

That requests to Netlify functions (at /.netlify/*) get redirected too, which of course causes calls to these functions to fail. For example, a call to a function deployed at /.netlify/the-function are 301'd to /subpath/.netlify/the-function.

This does not occur without the -t flag. This also does not occur on a live site on the Netlify CDN.

Also, it's not possible to address this with a preceeding rule like

/.netlify/*  /.netlify/:splat  200

(a 'trick' I've used before to disable a catch-all redirect rule for a specific path), because "/.netlify" is not a valid source path for redirect rules - at least not according to your own netlify-redirect-parser package.

Am I missing something? Greatly appreciate your attention and help.

@calavera
Copy link
Contributor

calavera commented Jan 7, 2021

ohh that's a very interesting finding @nonAlgebraic! That's for sure a new bug, but I think I can get a patch out for you today. I'll update this thread when I have some more information.

@nonAlgebraic
Copy link
Author

@calavera amazing! thank you so much. Please let me know if I can be of any further help.

@nonAlgebraic
Copy link
Author

@calavera would it be helpful if I open a separate issue for the above, with reproduction etc.?

@nonAlgebraic
Copy link
Author

Went ahead and did so #1721 for tidiness.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug code to address defects in shipped code
Projects
None yet
Development

No branches or pull requests

2 participants