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 special characters in request params #184

Open
wants to merge 1 commit into
base: v4.x
Choose a base branch
from

Conversation

G4brym
Copy link

@G4brym G4brym commented Sep 5, 2023

Description

Quote from itty-router-openapi/issues/88 @gimenete

Given an endpoint like this:

router.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.

The fix here was to decodeURIComponent the request url before parsing the parameters, i've also added a unit test for this, but now this test is failing due to URI malformed.

@kwhitley can you propose a fix here, should i edit the failing test to a valid one?

Type of Change (select one and follow subtasks)

  • Maintenance or repo-level work (e.g. linting, build, tests, refactoring, etc.)
  • Bug fix (non-breaking change which fixes an issue)

@kwhitley
Copy link
Owner

Hey there - I somehow missed this one (it came in right before my wedding) - taking a look now.

My gut feeling is that this is prob a safe addition and the test should be modified, but lemme play a bit with it...

@kwhitley kwhitley added the BUG Something isn't working label Dec 14, 2023
@kwhitley
Copy link
Owner

Targeting v4.1 release for this :)

@kwhitley kwhitley added the PENDING RELEASE Final stages before release! :D label Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BUG Something isn't working PENDING RELEASE Final stages before release! :D
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants