quarkus-quinoa icon indicating copy to clipboard operation
quarkus-quinoa copied to clipboard

SvelteKit live dev with dynamic routing fails

Open devpikachu opened this issue 1 year ago • 20 comments

Describe the bug

Having added a SvelteKit 2 app (running with Vite 5) and then a dynamic route to it (routes/activate/[activationToken]/+page.svelte) results in a 500 error being displayed in browser. image

Inspecting the Network tab, it seems like there's 400 errors being thrown when Svelte tries to load the code for the dynamic route: image

Looking at the CLI output of Quarkus, it seems like Quinoa is not intercepting these 2 calls at all, which leads me to believe it is Quinoa returning these 400 responses. As a note, I set the logging level to TRACE and found no indication of these 2 calls being handled by the server.

Having tried running Vite directly (pnpm run dev), the page can be accessed as normal and it works just fine.

I've already did the config change as per #574 . image

Calling Quarkus:

curl -v 'http://localhost:8080/src/routes/activate/\[activationToken\]/+page.ts'
* processing: http://localhost:8080/src/routes/activate/[activationToken]/+page.ts
*   Trying [::1]:8080...
* connect to ::1 port 8080 failed: Connection refused
*   Trying 127.0.0.1:8080...
* Connected to localhost (127.0.0.1) port 8080
> GET /src/routes/activate/[activationToken]/+page.ts HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/8.2.1
> Accept: */*
> 
< HTTP/1.1 400 Bad Request
< content-length: 0
< 
* Connection #0 to host localhost left intact

Calling Vite directly:

curl -v 'http://localhost:5173/src/routes/activate/\[activationToken\]/+page.ts'
* processing: http://localhost:5173/src/routes/activate/[activationToken]/+page.ts
*   Trying [::1]:5173...
* Connected to localhost (::1) port 5173
> GET /src/routes/activate/[activationToken]/+page.ts HTTP/1.1
> Host: localhost:5173
> User-Agent: curl/8.2.1
> Accept: */*
> 
< HTTP/1.1 200 OK
< Access-Control-Allow-Origin: *
< Content-Type: application/javascript
< Cache-Control: no-cache
< Etag: W/"16a-ntZf16MXhkhvJbKqOZZm+fpwNDQ"
< Date: Sat, 16 Dec 2023 15:46:57 GMT
< Connection: keep-alive
< Keep-Alive: timeout=5
< Content-Length: 1037
< 
import { redirect } from "/node_modules/.pnpm/@[email protected]_@[email protected][email protected][email protected]/node_modules/@sveltejs/kit/src/exports/index.js?v=1905be8b";
export const load = async ({ fetch, params }) => {
  const regex = /^[0-9a-fA-F]{32}$/;
  if (!regex.test(params.activationToken)) {
    throw redirect(301, "/login");
  }
};

//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbIitwYWdlLnRzIl0sInNvdXJjZXNDb250ZW50IjpbImltcG9ydCB7IHJlZGlyZWN0IH0gZnJvbSAnQHN2ZWx0ZWpzL2tpdCc7XG5cbmV4cG9ydCBjb25zdCBsb2FkID0gYXN5bmMgKHsgZmV0Y2gsIHBhcmFtcyB9KSA9PiB7XG5cdGNvbnN0IHJlZ2V4ID0gL15bMC05YS1mQS1GXXszMn0kLztcblxuXHRpZiAoIXJlZ2V4LnRlc3QocGFyYW1zLmFjdGl2YXRpb25Ub2tlbikpIHtcblx0XHR0aHJvdyByZWRpcmVjdCgzMDEsICcvbG9naW4nKTtcblx0fVxufTtcbiJdLCJtYXBwaW5ncyI6IkFBQUEsU0FBUyxnQkFBZ0I7QUFFbEIsYUFBTSxPQUFPLE9BQU8sRUFBRSxPQUFPLE9BQU8sTUFBTTtBQUNoRCxRQUFNLFFBQVE7QUFFZCxNQUFJLENBQUMsTUFBTSxLQUFLLE9BQU8sZUFBZSxHQUFHO0FBQ3hDLFVBQU0sU0FBUyxLQUFLLFFBQVE7QUFBQSxFQUM3QjtBQUNEOyIsIm5* Connection #0 to host localhost left intact
hbWVzIjpbXX0=

Currently the only workaround to this is setting quarkus.quinoa.dev-server to false and running Vite separately, which defeats the purpose of Quinoa's DX during development.

Quinoa version

2.2.1

Quarkus version

3.6.3

Build / Runtime

Vite

Package Manager

PNPM

Steps to reproduce the behavior

  1. Create a Quarkus & Quinoa project
  2. Add a SvelteKit project to it and follow docs for disabling SSR
  3. Add a dynamic route (routes/[slug]/+page.svelte)
  4. Attempt to access the route (http://localhost:8080/test-slug)

Expected behavior

The page is loaded as normal

devpikachu avatar Dec 16 '23 15:12 devpikachu

Upon further investigation I've found the root cause, and it seems like this is a limitation within Java itself.

The 400 status is set here: https://github.com/quarkusio/quarkus/blob/7cf3e4e8f484aefed9ea97b08ebb2164093baa4e/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/VertxHttpRecorder.java#L122

if (!uriValid(httpServerRequest)) {
    httpServerRequest.response().setStatusCode(400).end();
    return;
}

The uriValid method only constructs a new URI object:

private boolean uriValid(HttpServerRequest httpServerRequest) {
    if (DISABLE_URI_VALIDATION) {
        return true;
    }
    try {
        // we simply need to know if the URI is valid
        new URI(httpServerRequest.uri());
        return true;
    } catch (URISyntaxException e) {
        return false;
    }
}

This can be disabled by setting the vertx.disableURIValidation JVM system property to true, but that's a really bad idea to do in production.

Having said that, I'll do some more tests to see if the production build requires this kind of path - if not, a workaround could be to disable URI validation in dev only.

devpikachu avatar Dec 16 '23 16:12 devpikachu

I can confirm that setting vertx.disableURIValidation to true mitigates this issue in development environments.

quarkus dev -Dvertx.disableURIValidation=true

image

devpikachu avatar Dec 16 '23 17:12 devpikachu

Agreed this is not a good long term solution let's see what @ia3andy says.

melloware avatar Dec 16 '23 18:12 melloware

This is a tricky one 🤪, my take on this is:

  • If this url is invalid (this needs to be checked), then it's a bad practice from Vite to actually use such a URL and we should investigate ways to provide a fix in Vite itself
  • If this url is valid we should patch vertx to allow it

As a workaround this could maybe be set as a %dev setting in the application.properties. @geoand are the vertx option configurable through the application.properties (if not maybe we should provide a way)?

ia3andy avatar Dec 18 '23 09:12 ia3andy

cc @vietj

ia3andy avatar Dec 18 '23 09:12 ia3andy

If this url is valid we should patch vertx to allow it

It's Vertx that is failing, it's a Quarkus specific check that utilizes the code pasted in OP.

If there is some Vertx utility to check the validity of a URI, let's by all means use that.

geoand avatar Dec 18 '23 09:12 geoand

Ok, the URL is not valid in the RFC: "This URL pattern is commonly used in modern JavaScript frameworks for dynamic routing. In this case, "[activationToken]" is a dynamic segment, meaning it can change based on the data passed to the route. The "+" symbol before "page.svelte" is a special syntax used in Vite to indicate that this is a nested route. However, please note that this URL pattern is specific to the development environment and the routing library you are using. It may not be directly usable as a URL in a production environment or with different routing libraries."

So... the best solution would be to allow it in dev only, or through QUinoa, I wonder if I can set this for the request.

ia3andy avatar Dec 18 '23 10:12 ia3andy

I think we should use a quarkus configuration instead of a system property in Quarkus: https://github.com/quarkusio/quarkus/blob/main/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/VertxHttpRecorder.java#L117

This would solve the issue as we could enable this only in dev mode.

Also we could keep backward compat on this with the system props: disabled if Boolean.getBoolean("vertx.disableURIValidation") || fromConfig(quarkus.http.disableURIValidation)

@geoand agreed?

ia3andy avatar Dec 18 '23 10:12 ia3andy

That was really meant to be a hidden flag, but I guess since we now have a valid use case, we can make it a real configuration option

geoand avatar Dec 18 '23 10:12 geoand

I'll create an issue on Quarkus..

@devpikachu so this means:

  • Until the fix is available in Quarkus core, you can use the quarkus dev -Dvertx.disableURIValidation=true as a workaround
  • Then you will be able to use the application.properties with %dev.quarkus.http.disableURIValidation=true (or similar)

ia3andy avatar Dec 18 '23 10:12 ia3andy

Super, thanks for the quick investigation @ia3andy .

FYI, I've also opened an issue on Quarkus' side, more-so for the decoding failing silently rather than throwing to output: https://github.com/quarkusio/quarkus/issues/37789

devpikachu avatar Dec 18 '23 10:12 devpikachu

Here is the underlying issue: https://github.com/quarkusio/quarkus/issues/37804

ia3andy avatar Dec 18 '23 10:12 ia3andy

I'll take a look

geoand avatar Dec 18 '23 10:12 geoand

@all-contributors add @devpikachu for bug

melloware avatar Dec 19 '23 13:12 melloware

@melloware

I've put up a pull request to add @devpikachu! :tada:

allcontributors[bot] avatar Dec 19 '23 13:12 allcontributors[bot]

From Quarkus Team "disableURIValidation is not exposed for a reason: you want to validate URI. It's an attack vector."

melloware avatar Jan 22 '24 12:01 melloware

I guess this should then be documented on how to get around it in dev, as it is only a concern there.

treo avatar Feb 17 '24 07:02 treo

@treo is this ticket safe to close now that there are documented workarounds?

melloware avatar Feb 17 '24 12:02 melloware

At the moment this is only documented in this particular issue. https://github.com/quarkiverse/quarkus-quinoa/pull/640 was for an unrelated problem.

I'm not sure where exactly this particular flag needs to be documented, as I think it isn't necessarily a SvelteKit only thing.

treo avatar Feb 17 '24 14:02 treo

The Quarkus Team has decided not to take action on this as its considered too risky from a security perspective.

melloware avatar Mar 11 '24 11:03 melloware