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.

Sign up for GitHub

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

Jump to bottom

URL forwarding removes query string #643

Open
C0pyR1ght opened this issue Feb 22, 2021 · 8 comments
Open

URL forwarding removes query string #643

C0pyR1ght opened this issue Feb 22, 2021 · 8 comments

Comments

@C0pyR1ght
Copy link

When navigating tosearch.html Serve forwards the request to /search.
so far so good. But query-strings at the end of an URL are removed while forwarding:

search.html?s=hello -> /search

This is not a behavior I would expect in normal operation
@C0pyR1ght C0pyR1ght changed the title URL Forwarding removes query string URL forwarding removes query string Feb 22, 2021
@oshi97
Copy link

oshi97 commented Apr 14, 2021

Any updates on this issue?

@mvolkmann
Copy link

This is a huge issue for me. To get around this I used http-server instead of serve which does not lose query parameters.

@C0pyR1ght
Copy link
Author

@mvolkmann yepp.. Makes serve unusable for any project with query strings.

This seems to be an issue of serve-handler which is the serve core. Even found an old issue from 2019 about this vercel/serve-handler#97 and also a PR that fixes it vercel/serve-handler#144. But sadly no one cares

This was referenced May 5, 2021
Closed
Merged
oshi97 added a commit to yext/answers-hitchhiker-theme that referenced this issue May 12, 2021
@oshi97
use express instead of serve-handler for snapshots (#766)
f358aeb 
This pr updates our tests to use express instead of serve-handler.
serve (likely due to using serve-handler) currently has a bug where,
if any redirects occur, it will lose any query params you had in the url.
See: vercel/serve#643

On top of that, serve will automatically redirect urls like
/index.html to just /, which breaks our iframe integration for universal.

J=SLAP-1313
TEST=auto

percy snapshots for universal on iframe are now corrected and
show the correct query
oshi97 added a commit to yext/answers-hitchhiker-theme that referenced this issue May 12, 2021
@oshi97
use express instead of serve-handler for snapshots (#766)
a729e57 
This pr updates our tests to use express instead of serve-handler.
serve (likely due to using serve-handler) currently has a bug where,
if any redirects occur, it will lose any query params you had in the url.
See: vercel/serve#643

On top of that, serve will automatically redirect urls like
/index.html to just /, which breaks our iframe integration for universal.

J=SLAP-1313
TEST=auto

percy snapshots for universal on iframe are now corrected and
show the correct query
oshi97 added a commit to yext/answers-hitchhiker-theme that referenced this issue May 12, 2021
@oshi97
use express instead of serve-handler for snapshots (#766)
2eda3c4 
This pr updates our tests to use express instead of serve-handler.
serve (likely due to using serve-handler) currently has a bug where,
if any redirects occur, it will lose any query params you had in the url.
See: vercel/serve#643

On top of that, serve will automatically redirect urls like
/index.html to just /, which breaks our iframe integration for universal.

J=SLAP-1313
TEST=auto

percy snapshots for universal on iframe are now corrected and
show the correct query
@warren-bank
Copy link

shameless self-promotion alert:
this feature is fixed in my @warren-bank/serve fork of serve

@canvaspixels
Copy link

@warren-bank I get this when I install it, could be because i'm using nvm.

npm i -g @warren-bank/serve
npm ERR! code 1
npm ERR! path /$USER/.nvm/versions/node/v16.13.2/lib/node_modules/@warren-bank/serve
npm ERR! command failed
npm ERR! command sh -c npm install ./lib/serve-handler && npm install ./lib/serve
npm ERR! added 45 packages, and audited 48 packages in 8s
npm ERR!
npm ERR! 12 packages are looking for funding
npm ERR!   run `npm fund` for details
npm ERR!
npm ERR! found 0 vulnerabilities
npm ERR! npm WARN cleanup Failed to remove some directories [
npm ERR! npm WARN cleanup   [
npm ERR! npm WARN cleanup     undefined,
npm ERR! npm WARN cleanup     AssertionError [ERR_ASSERTION]: rimraf: missing path
npm ERR! npm WARN cleanup         at rimraf (/$USER/.nvm/versions/node/v16.13.2/lib/node_modules/npm/node_modules/rimraf/rimraf.js:54:3)

@warren-bank
Copy link

warren-bank commented Feb 16, 2022 •
edited
Loading

@canvaspixels

  1. since neither serve nor my fork use rimraf (to delete directories in npm scripts)..
    probably an issue is with your environment:
    • either (unlikely) a bug in nvm
    • or (more likely) you need elevated permissions to run install on your system
  2. do you get the same error when you install serve (upstream)
  3. if not.. and this error message only occurs for my fork.. then:
    • please create an issue in my repo..
      so we don't hijack this thread with an off-topic conversation

update:

  • you're right.. I'm wrong
  • xref: to issue
  • problem on posix w/ symbolic link to libraries.. will fix ASAP
  • sorry! ..but thanks for reporting

final update:

  • sorry everybody for the unwanted noise
  • this issue w/ global npm install is now fixed in v130002.16.0
    • which has been pushed to npm

@bramus
Copy link

bramus commented Apr 4, 2023

+1 on this. I think serve should preserve the querystring when redirecting, like it does with the location’s hash.

@Pete-Y-CS Pete-Y-CS mentioned this issue May 17, 2023
Closed
@morganney
Copy link

Just use http-server instead.

vincentmarchetti added a commit to vincentmarchetti/manifesto that referenced this issue Feb 20, 2024
@vincentmarchetti
reinstalled the serve-static module, it is required by the test/index…
f302e99 
….js file

for unit testing.

serve-static had been removed when http-server was installed, http-server was required
for the webpage because serve-static does not include query parameters in the
url it sends in the headers to the client

See vercel/serve#643
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@mvolkmann @bramus @morganney @canvaspixels @warren-bank @C0pyR1ght @oshi97