% cannot be escaped in url params #1746

andykais · 2021-06-23T16:45:44Z

Describe the bug
parameterized routes do not properly parse the % sign in a url.

If I have a url that contains a percent that needs to be escaped, I would use the function encodeUri. For example:

encodeURI('/sample/test%20me') // returns "/sample/test%2520me"

What actually happens, is svelte kit reads this url as test me

Logs

To Reproduce

npm init svelte@next
npm install

create the file src/routes/sample/[param].svelte with the following contents:

<script context="module">
  export function load({ page }) {
    const { param } = page.params
    return {
      props: { param }
    }
  }
</script>

<script>
  export let param
</script>
param is: "{param}"

opening this url: http://localhost:3000/sample/test%2520me displays param is "test me"

Expected behavior
the page should display param is test%20me.

Stacktraces

Information about your SvelteKit Installation:

Diagnostics

System: OS: Linux 5.12 Arch Linux CPU: (4) x64 Intel(R) Core(TM) i5-6300U CPU @ 2.40GHz Memory: 5.48 GB / 18.96 GB Container: Yes Shell: 5.8 - /usr/bin/zsh Binaries: Node: 16.3.0 - /usr/local/bin/node Yarn: 1.22.4 - ~/.npm-packages/bin/yarn npm: 7.11.2 - ~/.npm-packages/bin/npm Browsers: Firefox: 89.0.1 npmPackages: @sveltejs/kit: next => 1.0.0-next.116 svelte: ^3.34.0 => 3.38.3

Your browser: Google Chrome 91.0.4472.114

Severity
I have a local web app that displays system files using a route like src/routes/file/[...filepath].svelte. For whatever reason, some files happen to contain %20 inside them (likely because they were encoded weirdly once upon a time. With this bug as it stands, I cannot display any files that contain percents inside them, because I have no way of telling the server that a percent is part of the filepath.

Additional context
I found a possibly related issue in the polka server repo: lukeed/polka#119 (I assume polka is still the internal server)

The text was updated successfully, but these errors were encountered:

benmccann · 2021-06-24T22:11:49Z

Hmm, some similar issues were fixed in #1548 and #740. However, Polka only runs in production mode and possibly preview (I can't remember). Is the behavior the same for npm run dev, npm run preview, and npm run build && node build?

andykais · 2021-06-25T22:21:52Z

If I add @sveltejs/adapter-node and run preview mode, an invalid url (like localhost:3000/sample/test%) it crashes the whole server

 andrew  sveltekit-percent-bug  pnpm run preview

> [email protected] preview /home/andrew/Code/scratchwork/sveltekit-percent-bug
> svelte-kit preview


  SvelteKit v1.0.0-next.117

  local:   http://localhost:3000
  network: not exposed

  Use --host to expose server to other devices on this network


file:///home/andrew/Code/scratchwork/sveltekit-percent-bug/node_modules/.pnpm/@[email protected][email protected]/node_modules/@sveltejs/kit/dist/chunks/index7.js:316
					path: decodeURIComponent(parsed.pathname),
					      ^

URIError: URI malformed
    at decodeURIComponent (<anonymous>)
    at file:///home/andrew/Code/scratchwork/sveltekit-percent-bug/node_modules/.pnpm/@[email protected][email protected]/node_modules/@sveltejs/kit/dist/chunks/index7.js:316:12
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
 ERROR  Command failed with exit code 1.

npm run build && node build returns URI malformed to the browser if I send a valid (https://localhost:3000/sample/test%25me`) (e.g. it still parses incorrectly, but it doesnt crash the server)

 ✘ andrew  sveltekit-percent-bug  node build     
Listening on port 3000
URIError: URI malformed
    at decodeURIComponent (<anonymous>)
    at Object.params (file:///home/andrew/Code/scratchwork/sveltekit-percent-bug/build/index.js:1531:33)
    at respond$1 (file:///home/andrew/Code/scratchwork/sveltekit-percent-bug/build/index.js:807:24)
    at render_page (file:///home/andrew/Code/scratchwork/sveltekit-percent-bug/build/index.js:967:28)
    at async resolve (file:///home/andrew/Code/scratchwork/sveltekit-percent-bug/build/index.js:1209:93)
    at async respond (file:///home/andrew/Code/scratchwork/sveltekit-percent-bug/build/index.js:1187:12)
    at async Array.<anonymous> (file:///home/andrew/Code/scratchwork/sveltekit-percent-bug/build/index.js:15027:22)

benmccann · 2021-08-05T16:45:25Z

Can you test again with the latest version of SvelteKit? I'm wondering if this was fixed by #2078

acoyfellow · 2021-08-29T17:48:49Z

in @sveltejs/[email protected]: I've got a url parameter that has a %252520 in the filename - but when I read this param in an endpoint it comes in as %20. I'm wondering if this is related

benmccann · 2021-08-31T15:26:47Z

Thanks for the update @acoyfellow. I tracked this down to a bug in Vite and upgraded Vite, so it should be fixed in the next release of SvelteKit

andykais · 2021-08-31T20:29:01Z

@benmccann I dont think this is actually fixed and the issue should stay open. I just tested with the latest version of sveltekit (1.0.0-next.160) which contains an upgraded vite (^2.5.2).

The original error I described (http://localhost:3000/sample/test%2520me. translating the param to "test me" rather than correctly parsing "test%20me" is still present. The one improvement I will point out is that with adapter-node it is now not possible to crash the server with a malformed url. It just returns a 500

benmccann · 2021-08-31T21:33:22Z

well I guess we went from triple decoding to just double decoding 😕

benmccann · 2021-08-31T22:00:03Z

So looking through the code, here's what I think is happening...

We decode to match the route:

kit/packages/kit/src/runtime/server/index.js

Line 59 in 3d4172c

const match = route.pattern.exec(decoded);

And then decode again to get the params:

kit/packages/kit/src/core/dev/index.js

Line 255 in 3d4172c

params[key] = decodeURIComponent(match[i + 1]);

Which is called on the already decoded params:

kit/packages/kit/src/runtime/server/page/index.js

Line 21 in 3d4172c

const params = route.params(match);

I'm not entirely sure it's valid that we decode before matching the URL pattern. What if we get a request URL with %2F in the path?

andykais · 2021-09-01T14:09:45Z

I'm not entirely sure it's valid that we decode before matching the URL pattern. What if we get a request URL with %2F in the path?

That doesnt necessarily make sense to me either... but I will say, in the example above, the url http://localhost:3000/sample%2Ftest doesnt match to the src/routes/sample/[param].svelte page. It 404s

andykais · 2021-09-07T17:33:12Z

thanks @benmccann! Is there a way for me to test this myself using npm to confirm the fix? Or a way to know when a particular pr has made it into a release?

benmccann · 2021-09-07T17:54:17Z

it's released now: https://github.com/sveltejs/kit/blob/master/packages/kit/CHANGELOG.md#100-next163

andykais · 2021-09-07T19:12:28Z

can confirm the fix works! http://localhost:3000/sample/test%2520me renders this page:

although...for what its worth, passing in an invalid url like http://localhost:3000/sample/test% crashes the dev server

/Users/andrew/Code/development/svelte-test2/node_modules/.pnpm/[email protected]/node_modules/vite/dist/node/chunks/dep-a5ad1936.js:65107
        let url = decodeURI(removeTimestampQuery(req.url)).replace(NULL_BYTE_PLACEHOLDER, '\0');
                  ^

URIError: URI malformed
    at decodeURI (<anonymous>)
    at viteTransformMiddleware (/Users/andrew/Code/development/svelte-test2/node_modules/.pnpm/[email protected]/node_modules/vite/dist/node/chunks/dep-a5ad1936.js:65107:19)
    at call (/Users/andrew/Code/development/svelte-test2/node_modules/.pnpm/[email protected]/node_modules/vite/dist/node/chunks/dep-a5ad1936.js:54082:7)
    at next (/Users/andrew/Code/development/svelte-test2/node_modules/.pnpm/[email protected]/node_modules/vite/dist/node/chunks/dep-a5ad1936.js:54026:5)
    at /Users/andrew/Code/development/svelte-test2/node_modules/.pnpm/[email protected]/node_modules/vite/dist/node/chunks/dep-a5ad1936.js:64814:28
    at viteServePublicMiddleware (/Users/andrew/Code/development/svelte-test2/node_modules/.pnpm/[email protected]/node_modules/vite/dist/node/chunks/dep-a5ad1936.js:64853:9)
    at call (/Users/andrew/Code/development/svelte-test2/node_modules/.pnpm/[email protected]/node_modules/vite/dist/node/chunks/dep-a5ad1936.js:54082:7)
    at next (/Users/andrew/Code/development/svelte-test2/node_modules/.pnpm/[email protected]/node_modules/vite/dist/node/chunks/dep-a5ad1936.js:54026:5)
    at next (/Users/andrew/Code/development/svelte-test2/node_modules/.pnpm/[email protected]/node_modules/vite/dist/node/chunks/dep-a5ad1936.js:54004:14)
    at next (/Users/andrew/Code/development/svelte-test2/node_modules/.pnpm/[email protected]/node_modules/vite/dist/node/chunks/dep-a5ad1936.js:54004:14)
 ERROR  Command failed with exit code 1.

The node adapter production server does not crash

URIError: URI malformed
    at decodeURIComponent (<anonymous>)
    at Object.params (file:///Users/andrew/Code/development/svelte-test2/build/middlewares.js:2449:32)
    at render_page (file:///Users/andrew/Code/development/svelte-test2/build/middlewares.js:2027:24)
    at resolve (file:///Users/andrew/Code/development/svelte-test2/build/middlewares.js:2220:110)
    at Object.handle (file:///Users/andrew/Code/development/svelte-test2/build/middlewares.js:2457:65)
    at respond (file:///Users/andrew/Code/development/svelte-test2/build/middlewares.js:2204:33)
    at render (file:///Users/andrew/Code/development/svelte-test2/build/middlewares.js:2490:10)
    at Array.<anonymous> (file:///Users/andrew/Code/development/svelte-test2/build/middlewares.js:2611:28)

benmccann · 2021-09-07T19:31:55Z

You could file an issue with Vite. It looks like that's where it's coming from

ignatiusmb added the pending clarification label Aug 23, 2021

benmccann added bug Something isn't working and removed pending clarification labels Aug 29, 2021

benmccann added this to the 1.0 milestone Aug 29, 2021

benmccann mentioned this issue Aug 29, 2021

[fix] upgrade to Vite 2.5.2 to fix URL decoding #2323

Merged

benmccann closed this as completed in #2323 Aug 31, 2021

benmccann reopened this Aug 31, 2021

benmccann mentioned this issue Sep 3, 2021

[fix] don't decode URL when finding matching route #2354

Merged

benmccann closed this as completed in #2354 Sep 5, 2021

benmccann mentioned this issue Sep 15, 2021

@ not working in route paths #2401

Closed

benmccann added a commit that referenced this issue Sep 15, 2021

fix #1746 by decoding the diff of characters

74e8e9f

benmccann mentioned this issue Sep 15, 2021

[fix] revert #2354 and fix double character decoding a different way #2435

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

% cannot be escaped in url params #1746

% cannot be escaped in url params #1746

andykais commented Jun 23, 2021

benmccann commented Jun 24, 2021

andykais commented Jun 25, 2021

benmccann commented Aug 5, 2021

acoyfellow commented Aug 29, 2021

benmccann commented Aug 31, 2021

andykais commented Aug 31, 2021

benmccann commented Aug 31, 2021

benmccann commented Aug 31, 2021 •

edited

Loading

andykais commented Sep 1, 2021

andykais commented Sep 7, 2021

benmccann commented Sep 7, 2021

andykais commented Sep 7, 2021 •

edited

Loading

benmccann commented Sep 7, 2021

% cannot be escaped in url params #1746

% cannot be escaped in url params #1746

Comments

andykais commented Jun 23, 2021

benmccann commented Jun 24, 2021

andykais commented Jun 25, 2021

benmccann commented Aug 5, 2021

acoyfellow commented Aug 29, 2021

benmccann commented Aug 31, 2021

andykais commented Aug 31, 2021

benmccann commented Aug 31, 2021

benmccann commented Aug 31, 2021 • edited Loading

andykais commented Sep 1, 2021

andykais commented Sep 7, 2021

benmccann commented Sep 7, 2021

andykais commented Sep 7, 2021 • edited Loading

benmccann commented Sep 7, 2021

benmccann commented Aug 31, 2021 •

edited

Loading

andykais commented Sep 7, 2021 •

edited

Loading