-
Notifications
You must be signed in to change notification settings - Fork 90
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
inconsistent application of encodeUri
#234
Comments
Is there some reason we're using 'encodeURI' for file names? or can this simply be removed? Happy to submit a PR. |
#166 suggest this feature is needed. What's the work around here? |
I don't understand what is the ask or problem. Reserved characters in uri MUST be percent-encoded (it's the spec). How does it break your app? Could you write a code example that would fail? |
Summary: I'm not arguing with the spec. What I'm saying is that when the wildcard is enabled all files are served successfully. Whether or not they contain escapable characters in the path. When you disable wildcard serving in import Fastify from "fastify";
import fastifyStatic from "fastify-static"
import fastifyRoutes from "fastify-routes"
import path from "path"
import fetch from "node-fetch";
const fastify = new Fastify({ ignoreTrailingSlash: true });
fastify.register(fastifyRoutes, {});
fastify.register(fastifyStatic, {
wildcard: false, //Change this to see tests pass or fail.
root: path.resolve("./test_public"),
})
fastify.listen(3000, async (err, address) => {
if (err) throw err;
console.log(`server listening on ${address}`);
console.log("Listening on routes: ", fastify.routes.keys())
for (let path of ["/afile.json", "/bfile[...].json", "/[...]/cfile.json"]) {
console.log(`Testing GET ${path}`);
const res = await fetch(`http://localhost:3000${path}`);
if (res.status >= 200 && res.status < 300) {
const body = await res.json();
console.log(`${res.url} passed: ${res.status}`);
console.log(`response body: ${JSON.stringify(body)}`);
} else {
console.log(`${res.url} not found: ${res.status}`);
console.log(`This is expected to work`)
}
}
fastify.close();
}); My guess the issues is that when the wild card is being used it doesn't encode the names of available files or the incoming request path. Thus all comparisons just work. But, when the wild card serving is not being used, the list of available file names is being encoded. However, when a request comes in and that URL is compared agains available files it's not being encoded. If encoding URIs is so important that's fine, the issue is we aren't consistently doing so and that's causing bugs in certain configurations. |
It might be that a different/better fix for #143 is possible, something that does not break normal usage. Would you like to give it a go and send a PR? |
I'll look into it as I can. Not sure if this is a |
Thanks! |
@moonmeister according to rfc the const res = await fetch(encodeURI(`http://localhost:3000${path}`)); Another solution would be to PR the code and add un-encoded (decoded) routes along with encoded ones. But I am not sure how bad this solution is. |
That's super helpful, and I was starting to suspect it with be a character specific issue. I'll think on the best course of action here. |
@moonmeister actually it goes even deeper. The thing is that fastify itself handles it wrongly. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Still needs work |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
The issue on the fastify side is closed as completed. |
Would you like to send a Pull Request to address this issue? Remember to add unit tests. |
I think this issue can be closed.
Whatever the value of wildcard ( Edit: PR that adds tests. |
Thanks @gurgunday! |
Prerequisites
Fastify version
3.21.1
Plugin version
4.2.3
Node.js version
14.17.6
Operating system
macOS
Operating system version (i.e. 20.04, 11.3, 10)
11.6
Description
I needed to disable the
wildcard
setup in favor of explicit static routes so I could use the catch all route for handling non-static pages.This broke some unrelated route handling later on and I've tracked it back to this change. I have files that need to be served from the
/page-data/app/[...]/page-data.json
route. This works fine whenwildcard: true
. However, whenwildcard: false
the code usesencodeUri
on all the file names meaning the server is now listening for/page-data/app/%5B...%5D/page-data.json
and breaks my app.Steps to Reproduce
Try serving a file with
fastify-static
andwildcard: false
. If the file has characters modified byencodeUri
then the file will 404.Expected Behavior
I'd expect that setting
wildcard: false
would still allow/page-data/app/[...]/page-data.json
to be served at it's correct path.The text was updated successfully, but these errors were encountered: