varabyte / kobweb

A modern framework for full stack web apps in Kotlin, built upon Compose HTML
https://kobweb.varabyte.com
Apache License 2.0
1.53k stars 68 forks source link

Dynamic Routes and Query Parameters Bug #408

Closed AC-Lover closed 3 months ago

AC-Lover commented 9 months ago

Hello, I am new to this, but I think there might be an issue. Please let me know if I'm wrong.

My problem is that when there is a dot in Dynamic routes, it doesn't recognize anything after that dot and gives me a 404 error. For example:

@Page("/sub/{token}")

I intend to dynamically receive a JWT, but I can't due to this issue. The same problem occurs with queryParams.

Thank you for your help.

bitspittle commented 9 months ago

What do you mean by a dot in your dynamic route? Something like

https://mysite.com/sub/a.b.c/

?

bitspittle commented 9 months ago

OK took a minute to try this out. Not only is this broken but it also looks like there's an issue with dynamic slugs in general (e.g. "/sub/{token}") seem to be busted even without a dot in the middle!

Will look into this soon, seems like a bad bug. Thanks for the report!

bitspittle commented 9 months ago

So here's the culprit:

https://github.com/varabyte/kobweb/blob/19da9c4732f36f1b1ffd8db574d63952a967f58f/frontend/kobweb-core/src/jsMain/kotlin/com/varabyte/kobweb/navigation/Router.kt#L266

val extension = pathQueryAndFragment.substringAfterLast('/').substringAfterLast('.', "")

What this means is the route "a/b.c/d/e" is treated as having the file extension "c/d/e". Whoops!

I'm definitely going to fix this to allow dots in the middle of paths (so b.c) is allowed.

@AC-Lover Do you also need to support dots in the final slug as well? That is, do you need to support a/b/stuff.here.with.dot as well? Because if so I need to think about it.

AC-Lover commented 9 months ago

So here's the culprit:

https://github.com/varabyte/kobweb/blob/19da9c4732f36f1b1ffd8db574d63952a967f58f/frontend/kobweb-core/src/jsMain/kotlin/com/varabyte/kobweb/navigation/Router.kt#L266

val extension = pathQueryAndFragment.substringAfterLast('/').substringAfterLast('.', "")

What this means is the route "a/b.c/d/e" is treated as having the file extension "c/d/e". Whoops!

I'm definitely going to fix this to allow dots in the middle of paths (so b.c) is allowed.

@AC-Lover Do you also need to support dots in the final slug as well? That is, do you need to support a/b/stuff.here.with.dot as well? Because if so I need to think about it.

https://regex101.com/r/KXfm0z/1 My requirement is this, please check

bitspittle commented 9 months ago

Apologies as life keeps throwing chores at me these holiday break days but fixing this is my highest priority once I get some time to code. I will put up a 0.15.4-SNAPSHOT you can use when it's ready. I'll ping this bug at that point.

I'm hoping once I actually get some time to code, a fix will be pretty fast, especially now I can repro and know the area of the code I need to fix.

bitspittle commented 9 months ago

FYI initially looking into this and ktor doesn't seem to like being asked to serve a route with a period in it. When I ask ktor for a path associated with a "." slug, I get a 404. Of course, it could be my own configuration of ktor that Kobweb sets up. Investigating.

It dawned on me that you might be able to work around this for now by removing the "." from the token and replacing it with another character, say "_", and then translating it back to a "." right before you send it back to JWT (which I assume happens on your backend).

You might also communicate the token using a JSON message via a request/response body rather than via query parameters, although I'm not 100% sure what your requirements are. (See kobweb create examples/todo for an example of sending data back and forth with JSON payloads)

I'll keep you in the loop as I figure more things out.

bitspittle commented 9 months ago

OK so good news but also a limitation.

That said, I'm giving up on supporting "." inside the slug.

A period inside a slug is valid, but it is generally assumed that when this is done you're trying to declare a file (as far as I can tell). Even ktor seems to think this way -- it tries to figure out the extension to know what sort of content it's going to send over the wire. When it encounters a dot inside a slug and doesn't know what the extension is, it returns an error response.


In your case, I would restrict the token to a query parameter, as in https://yoursite.com/sub/?token=this.has.dots.in.it. If you really don't want to do this for some reason, I would consider base64 encoding the token and then decoding it later. (base64 is limited to alphanumerics)

I've uploaded a snapshot to 0.15.4-SNAPSHOT. Can you try setting your gradle/libs.version.toml file to that version and let me know if you are unblocked?

AC-Lover commented 9 months ago

@bitspittle Thank you very much for your help, but I need the /sub/{JWT} structure and there is no way I can change this structure to another form, because I am actually trying to create a website interface for a tool, and this structure /sub/{JWT} is determined by that tool.

If I understood correctly, the problem is almost solved, right?

bitspittle commented 9 months ago

Sadly the problem is not almost solved because ktor is failing to handle it and it's not at all clear why. In other words, the failure at this point seems to be ktor, not Kobweb.

And unfortunately I do have other priorities at the moment so I don't have much time to jump into something deep in ktor.

What is this tool you're talking about which generates a URL you can't change?

AC-Lover commented 9 months ago

Sadly the problem is not almost solved because ktor is failing to handle it and it's not at all clear why. In other words, the failure at this point seems to be ktor, not Kobweb.

And unfortunately I do have other priorities at the moment so I don't have much time to jump into something deep in ktor.

What is this tool you're talking about which generates a URL you can't change?

I am not in a hurry, and you can solve this problem whenever you are comfortable and I am currently working as quayParameter until the problem is completely solved.

Thank you for your help and follow-up. Have a nice day 🙌

bitspittle commented 3 months ago

I know this is a 6 month old bug, but I made some changes to how I handled path routes with dots in them in a recent release, and I believe this is fixed at this point.

@Page("sub/{token}")
@Composable
fun TestPage() {
    PageLayout("Welcome to Kobweb!") {
        val ctx = rememberPageContext()
        Text("Extracted from URL: \"${ctx.route.params.getValue("token")}\"")
    }
}

dynamic-path-with-dogs

As a result, I'm going to mark this bug closed! But reply again / reopen if you try this out with latest and are still seeing issues. (I believe the fix went into 0.18.1)