withastro / astro

The web framework for content-driven websites. ⭐️ Star to support our work!
https://astro.build
Other
46.93k stars 2.49k forks source link

Route regex mismatch and server gives 500 #12469

Closed patheticGeek closed 4 days ago

patheticGeek commented 4 days ago

Astro Info

Astro                    v4.16.13
Node                     v18.20.4
System                   Linux (x64)
Package Manager          npm
Output                   server
Adapter                  @astrojs/node
Integrations             @astrojs/svelte

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

Open the reproduction and go to the following path /products/(select(0)from(select(sleep(15)))v)%2f*'+(select(0)from(select(sleep(15)))v)+'%22+(select(0)from(select(sleep(15)))v)+%22*%2f-9BM4ESb_xARsStc7tdySTQLssA

The server will give 500 with following error:

15:52:26 [ERROR] Missing parameter: id
  Stack trace:
    at /home/withastro/astro/node_modules/astro/dist/core/routing/manifest/generator.js:30:13
    [...] See full stack trace in the browser, or rerun with --verbose.

The reason I found is because in the getParams fn https://github.com/withastro/astro/blob/25baa4ed0c5f55fa85c2c7e2c15848937ed1dc9b/packages/astro/src/core/render/params-and-props.ts#L69-L74

Here decodeURIComponent is used, which should be used to decode URI Components like search params/hash and it incorrectly decodes the above path as - /products/(select(0)from(select(sleep(15)))v)/*'+(select(0)from(select(sleep(15)))v)+'"+(select(0)from(select(sleep(15)))v)+"*/-9BM4ESb_xARsStc7tdySTQLssA

If you notice here there appears a new / and now this has url 3 levels whereas original route that was matched has 2.

Whereas if we use the decodeURI which is to decode the URI itself, then it is correctly decoded as - /products/(select(0)from(select(sleep(15)))v)%2f*'+(select(0)from(select(sleep(15)))v)+'"+(select(0)from(select(sleep(15)))v)+"*%2f-9BM4ESb_xARsStc7tdySTQLssA

and this also matches the route regex.

What's the expected result?

Astro should ideally register the above as a valid path and pass down the page.

Link to Minimal Reproducible Example

https://codesandbox.io/p/devbox/wonderful-moon-fywty4

Participation

ematipico commented 4 days ago

This has been changed in Astro v5. We recommend looking at the breaking changes and adapting the application accordingly.

https://5-0-0-beta--astro-docs-2.netlify.app/en/guides/upgrade-to/v5/#changed-params-no-longer-decoded

patheticGeek commented 4 days ago

Is there any workaround for now? as this is causing issues in production for us

ematipico commented 4 days ago

Unfortunately, there isn't, and we can't ship this change in v4 because it's a breaking change for users.

ematipico commented 4 days ago

Duplicate of https://github.com/withastro/astro/issues/8516