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

Path parameters not unescaped correctly #88

Open
gimenete opened this issue Aug 10, 2023 · 4 comments
Open

Path parameters not unescaped correctly #88

gimenete opened this issue Aug 10, 2023 · 4 comments

Comments

@gimenete
Copy link
Contributor

gimenete commented Aug 10, 2023

Not really sure if it's a bug with itty-router or itty-router-openapi but it affects itty-router-openapi users.

Given an endpoint like this (using .original for simplicity):

router.original.get(
  "/something/:id",
  (request) => new Response(`id: ${request.params.id}`)
);

Making a query to /something/user%3A1234 returns the following response: id: user%3A1234. While it should be id: user:1234.

Just to make sure my assumption is correct I made an express application with a similar endpoint and the response is correct (id: user:1234):

app.get("/something/:id", (req, res) => {
  res.send(`id: ${req.params.id}`);
});

My workaround is to decode the full URL before passing it to the library. Like this:

export async function handleRequest(
  request: Request,
  env: Bindings,
  context: ExecutionContext
) {
  const req = new Request(decodeURIComponent(request.url), request);
  return router.handle(req, env, context);
}

Not sure if it's the best approach.

@G4brym
Copy link
Member

G4brym commented Aug 31, 2023

Hey @gimenete, i've just published the new v1.0.0 release that fixes this issue, i'm now using the decodeURIComponent in the url before parsing the parameters
Let me know if you are still having this issue
Migration guide to 1.0.0 available here

@G4brym G4brym closed this as completed Aug 31, 2023
@gimenete
Copy link
Contributor Author

gimenete commented Sep 4, 2023

Hey @G4brym

I think this is still an issue for path params. I see that decodeURIComponent is being used inside extractQueryParameters but I guess that only affects query parameters and not path parameters, right?

@G4brym
Copy link
Member

G4brym commented Sep 5, 2023

@gimenete path parameters are parsed on itty-router, i've openned a pull request there is the required changes
kwhitley/itty-router#184

@G4brym G4brym reopened this Sep 5, 2023
@kwhitley
Copy link
Contributor

kwhitley commented Sep 6, 2023

Thanks @G4brym - taking a look now!

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

No branches or pull requests

3 participants