auto_route_library icon indicating copy to clipboard operation
auto_route_library copied to clipboard

Severe bug: Query parameters are not being encoded

Open quaaantumdev opened this issue 3 months ago • 2 comments

The query parameters are not encoded properly or are decoded incorrectly somewhere in the process. I am pretty sure it has something to do with the use of the UrlState(..).url getter which uses Uri.decodeFull(...) internally.

Let's say you have three pages, FirstPage SecondPage and RandomPage:

// costructor of the FirstPage
// let's say the route is: /first-page?redirectTo=...
FirstPage({
  @QueryParam('redirectTo') this.redirectTo,
});

// costructor of the SecondPage
// let's say the route is: /second-page?redirectTo=...
SecondPage({
  @QueryParam('redirectTo') this.redirectTo,
});

// costructor of the RandomPage
// let's say the route is: /random-page
RandomPage(
  // no parameters
);

The FirstPage and SecondPage both provide a redirectTo parameter. Let's say this may carry an URL which describes where we want to go, once we finished whatever we wanted to do on these pages. This is fairly for various proceedures such as for login flows.

Now we'll start at the RandomPage and navigate to the FirstPage: Now let's navigate to the first page:

// somewhere in the RandomPage ...
// the current url is /random-page
final router = AutoRouter.of(context);
router.replace(FirstRoute(redirectTo: router.currentUrl));
// will go to:
//   /first-page?redirectTo=/random-page
// which is technically acceptable by the RFC standard but should probably encode the slash:
//   /first-page?redirectTo=%2Frandom-page

From the new FirstPage we'll now go to the SecondPage, providing our own url als redirectUrl (again)

// somewhere in the FirstPagePage ...
// the current url now is /first-page?redirectTo=/random/page
final router = AutoRouter.of(context);
router.replace(SecondRoute(redirectTo: router.currentUrl));
// should go to:
//   /second-page?redirectTo=%2Ffirst-page%3FredirectTo%3D%252Frandom-page
// or at least (encoding the questionmark and equals): 
//   /second-page?redirectTo=/first-page%3FredirectTo%3D/randomPage
// but will actually go to:
//   /first-page?redirectTo=/first-page?redirectTo=/random-page

so the final url created contains two question marks, an incorrect = sign

/first-page?redirectTo=/first-page?redirectTo=/random-page

This obviously goes very wrong in web as this is the url being set in the browser. Somewhere in the process, one of the "redirectTo" arguments simply gets dropped as it's effectively overwritten. But this is not simply overwriting a query parameter, this is modifying some part of the content of a queryparamter, it just happens to be that that string was a url by itself which resulted in modifying some part of it.

I investigated a bit and I am pretty sure it has something to with the use of the UrlState(..).url getter which uses Uri.decodeFull(...) internally. decodeFull has a great name but is not the standard functionality it sounds like. I am pretty sure something else should be used there. I also found the issue and pull request that introduced the use of decodeFull, see: this: https://github.com/Milad-Akarie/auto_route_library/issues/1119

quaaantumdev avatar Apr 16 '24 09:04 quaaantumdev