-
Notifications
You must be signed in to change notification settings - Fork 648
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
context params accessed via ctx.params in the handlers need to be decoded #2310
Comments
Cool idea. I wrote a test like this: Deno.test("param with encoding", async () => {
await withFakeServe("./tests/fixture/dev.ts", async (server) => {
const doc = await server.getHtml(`/decode-params/Hello%20World`);
assertEquals(
doc.querySelector("body")?.textContent,
"Hello World",
);
});
}); which invokes: import { defineRoute } from "$fresh/src/server/defines.ts";
export default defineRoute((req, ctx) => {
return ctx.params.id;
}); Prior to the change, the test fails. But then I do this: --- a/src/server/router.ts
+++ b/src/server/router.ts
@@ -189,9 +189,13 @@ export function getParamsAndRoute(
const res = route.pattern.exec(url);
if (res !== null) {
+ const decodedParams: Record<string, string | undefined> = {};
+ for (const [key, value] of Object.entries(res.pathname.groups)) {
+ decodedParams[key] = value ? decodeURIComponent(value) : value;
+ }
return {
route: route,
- params: res.pathname.groups,
+ params: decodedParams,
isPartial,
};
} and the test starts passing 🎉 But I think such a brutal approach would be a breaking change. Who knows what sort of logic people have written with the expectation that things remain encoded. Tomorrow I'll do a more subtle approach involving a new configuration option that allows this to be enabled. |
A possible why behind returning a raw value. decodeURI throws if original value arrives wrongly formatted. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/decodeURI If decoding is the user's responsibility the they will handle the error. Alternative would be to return a null for wrongly encoded values. |
I just realized that I needed to encode my params in the urls too, otherwise whatever is passed may lack some portions of the data containing reserved symbols (for instance, after a It must be an overlook that proper constructions of the urls should indeed include an encoder.
Now the mirrored decoding does not look too strange. If everything I said is right, would it make sense to add a note somewhere in the docs? If so I will gladly PR this and it wouldn't hurt #2311 |
Imagine a route file named
[product].tsx
. When you handle a request, you are reaching for the param named"product"
. If the product sent contains a space, it comes with a%20
when reaching its value viactx.params["product"]
. The feeling is that it would be more convenient if returning value was already decoded according to how a browser would so that the user wouldn't need to guess which function to utilize.The text was updated successfully, but these errors were encountered: