[Bug]: generateRoute() removing the last asterisk if it's part of the param string #9051
Comments
|
I'm not 100% sure this is a valid bug. Yes, it's not doing what you're expecting, but that's also not a valid path, as |
|
@timdorr thanks for your reply. Actually we used encodeURIComponent but this does not URL encode Should we force URL encode this as a "unique" condition in our code as |
|
Sorry, I shouldn't have said asterisks are encoded. They are not according to the spec (section 2.2). You can also use |
|
We are in fact encoding, I just didn't show that in my initial post, we do use Unfortunately the asterisk at the end for some reason gets stripped, so there's only "ugly" solutions such as
I guess it's only me running into this, so it seems will have to take one of the dirty approaches for now. |
generatePath was doing multiple passes on the `path` using string replace, the first two passes were applying parameters, the third pass was doing a cleanup and the fourth path was applying the `splat`.
It was possible to get incorrect results while applying `splat` when the last parameter value ended with `*`:
```ts
const path = generatePath("/route/:name", {
name: encodeURIComponent("includes *asterisk at the end*"),
})
```
```
Expected: "/route/includes *asterisk at the end*"
Received: "/route/includes *asterisk at the end"
```
results of the first two passes return the value of `/route/*asterisk at the end*` which was later treated as path with the splat resulting in the last asterisk removed.
it was also possible to inject the splat value unintentionally
```ts
generatePath("/courses/:name", { name: "foo*", "*": "splat_should_not_be_added" })
```
```
Expected: "/courses/foo*"
Received: "/courses/foosplat_should_not_be_added"
```
A safer option, instead of mutating a global path multiple times, is to split the path into segments, process each segment in isolation and then join them back together.
fixes remix-run#9051
|
@trainoasis FYI, I've got a PR fixing the issue. |
|
This issue has been automatically marked stale because we haven't received a response from the original author in a while 🙈. This automation helps keep the issue tracker clean from issues that are not actionable. Please reach out if you have more information for us or you think this issue shouldn't be closed! 🙂 If you don't do so within 7 days, this issue will be automatically closed. |
|
This was released in 6.9.0 |
What version of React Router are you using?
6.3.0
Steps to Reproduce
Expected Behavior
I would expect for the above to return
/route/includes *asterisk at the end*Actual Behavior
It returns
/route/includes *asterisk at the endThe text was updated successfully, but these errors were encountered: