Closed perbergland closed 2 years ago
Hello @perbergland ,
Sadly this is a flaw of this package inherited from original FlowRouter. Although that will be an easy fix, I'm not sure if it can be done without breaking current integrations.
Saying that this will require major version bump. I already have plans for v4 to keep current API and write new codebase from the scratch for top performance and compatibility (for example now we can not even update dependencies to the latest releases). Before v4 is released you have next options:
/packages
directory;qs.parse
directly as you suggested in OP;Hope that helps. Let's keep this issue open until it fixed by further releases.
Ah OK that’s what I thought - "bug compatibility" is a bitch…
Could you point to the code that would need to change and then I’ll try to look into introducing some kind of configuration?
Caveat: I’ve never modified/published either an npm or meteor package before :)
Hello @perbergland
I’ve never modified/published either an npm or meteor package before
Hope you're excited to make a first step. Please, get familiar with our simple contribution guidelines.
Basically you want to fork this branch (as it's a fork of original Flow-Router itself) to your own so called "feature branch". After you finish modifications you going to submit a pull request (PR).
Before you submit a PR please make sure nothing is broken by running tests.
This is the piece if code where we want to set the new rule/option. My suggestion is to introduce flag avoidDoubleURIEncoding
(you're welcome to suggest better name) and use it in this if(){}else{}
condition:
if (!this. avoidDoubleURIEncoding && this._specialChars.includes(paramArr[i])){
_param += encodeURIComponent(encodeURIComponent(paramArr[i]));
} else {
try {
_param += encodeURIComponent(paramArr[i]);
} catch (e) {
_param += paramArr[i];
}
}
@dr-dimitru and @perbergland, I also came across this bug and took it upon myself to follow the above recommendation and create an opt-in fix for it. Unfortunately, the above code didn't fix the issue, but the PR I submitted fixes it and passes all the tests. To use it, you would just apply:
FlowRouter.decodeQueryParamsOnce = true;
It's been over a year since this issue was first discussed, but hopefully this helps someone else struggling with the same issue!
🎉🎉🎉🎉🙏🙏
The use case is for a roundtrip to a login page that then is supposed to take you back to the original page including its query parameters.
You will then have a returnUrl query parameter which has doubly-encoded query parameters which are not handled properly because flow-router seems to decode the query parameters twice when it is vital that they are only decoded once.
Example:
Original url: http://localhost:3000/projects?param1=abc&redirectUrl=https%3A%2F%2Fwww.company.com%2Fabc123
Redirected to http://localhost:3000/login?returnUrl=%2Fprojects%3Fparam1%3Dabc%26redirectUrl%3Dhttps%253A%252F%252Fwww.company.com%252Fabc123
The queryParams provided by FlowRouter to the route registered for /login will now be {returnUrl: "/projects?param1=abc", redirectUrl: "https://www.company.com/abc123"}
i.e. the entire redirectUrl part of the returnUrl is lost.
The hack fix is to use qs directly instead of the queryParam provided by FlowRouter
qs.parse(window.location.search, { ignoreQueryPrefix: true, });
which gives the correct result:
{returnUrl: "/projects?param1=abc&redirectUrl=https%3A%2F%2Fwww.company.com%2Fabc123"}
This happens when using kadira:flowrouter and flowrouter-extra@3.7.4, regardless of using the latest qs version (6.9.3) or some earlier version like 6.5.1.
This is for Meteor 1.10.1