inconsistent application of `encodeUri`

[moonmeister]

Prerequisites

• [X] I have written a descriptive issue title
• [X] I have searched existing issues to ensure the bug has not already been reported

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 when wildcard: true. However, when wildcard: false the code uses encodeUri 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 and wildcard: false. If the file has characters modified by encodeUri 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.

[moonmeister]

Is there some reason we're using 'encodeURI' for file names? or can this simply be removed? Happy to submit a PR.

[moonmeister]

https://github.com/fastify/fastify-static/pull/166 suggest this feature is needed. What's the work around here?

[mcollina]

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?

[moonmeister]

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 fastify-static all the sudden files with escapable characters start to 404. see the example code/tests:

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.

[mcollina]

It might be that a different/better fix for https://github.com/fastify/fastify-static/issues/143 is possible, something that does not break normal usage.

Would you like to give it a go and send a PR?

[moonmeister]

I'll look into it as I can. Not sure if this is a find-my-way, fastify, or fastify-static, or expectations issue. Have to do some more in depth debugging.

[mcollina]

Thanks!

[mav-rik]

@moonmeister according to rfc the [ and ] characters must always be encoded (stackoverflow). But it looks like browsers (like chrome) and node-fetch just ignore that rule and send [ and ] as is. When you set wildcard: false the framework lists all the files in root directory and creates handlers for each file doing encodeURI for files paths which is the right thing to do. But node-fetch or chrome send un-escaped [ ] so the path does not match to any file. When you set wildcard: true obviously the framework creates a single handler for root + '*' path. Then no matter if browser encodes the path or not it gets to the handler with asterisk. In your case I would suggest to always encode path when requesting the file (which won't work in browser):

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.

[moonmeister]

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.

[mav-rik]

@moonmeister actually it goes even deeper. The thing is that fastify itself handles it wrongly. I created an issue for fastify and described the problem. When the fastify issue is resolved the PR #166 must be undone and then this issue will be solved.

[stale[bot]]

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.

[moonmeister]

Still needs work

[stale[bot]]

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.

[meskill]

The issue on the fastify side is closed as completed. Is there gonna be update on the current issue?

[mcollina]

Would you like to send a Pull Request to address this issue? Remember to add unit tests.

[gurgunday]

I think this issue can be closed.

Try serving a file with fastify-static and wildcard: false. If the file has characters modified by encodeUri then the file will 404.

Whatever the value of wildcard (true or false), the file is served correctly.

Edit: PR that adds tests.

[mcollina]

Thanks @gurgunday!