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

Fix redirect as middleware #281

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

hugojosefson
Copy link

@hugojosefson hugojosefson commented Oct 16, 2020

Regarding the following error:

  • You mount superstatic as a middleware on a path, instead of on no path (root), for example: app.use('/basepath', superstatic()) instead of app.use(superstatic()).
  • You make a request that superstatic is configured to redirect, such as /basepath/something/index.html/basepath/something.
  • The error was that superstatic did not take /basepath into account when redirecting, and would redirect only to the path below /basepath. In the example above, that would be to /something. When the client makes that new request to /something, express or connect responds 404 because superstatic is not mounted on that path.

This PR fixes that, by keeping req.url and req.originalUrl specifically apart for the purpose of redirects.

  • req.url containing information needed for mapping to a real file within the public directory, and
  • req.originalUrl containing information about how the request was actually made by the client. This is used for calculating correct redirects.

I updated files.spec.ts to run all its static file tests both with superstatic mounted as before, and as a middleware mounted under a base path.

@hugojosefson hugojosefson marked this pull request as ready for review October 16, 2020 12:31
@hugojosefson
Copy link
Author

Rebased this fix on top of master. Apparently needs approval for PR build.

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.

1 participant