Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

string parameters are url encoding issue #534

Closed
paolosanchi opened this issue Feb 21, 2020 · 5 comments
Closed

string parameters are url encoding issue #534

paolosanchi opened this issue Feb 21, 2020 · 5 comments
Labels

Comments

@paolosanchi
Copy link

paolosanchi commented Feb 21, 2020

The value of any parameter is urlencoded to the route, but it isn't urldecoded when converted back to parameter, this causes the engine detect it as changed and it causes a navigation.

format()

      // If the parameter type is "raw", then do not encodeURIComponent
      if (param.raw) return acc + encoded;
      // Encode the value
      return acc + encodeURIComponent(<string>encoded);

exec()

    for (let i = 0; i < nPathSegments; i++) {
      const param: Param = pathParams[i];
      let value: any | any[] = match[i + 1];

      // if the param value matches a pre-replace pair, replace the value before decoding.
      for (let j = 0; j < param.replace.length; j++) {
        if (param.replace[j].from === value) value = param.replace[j].to;
      }
      if (value && param.array === true) value = decodePathArray(value);
      if (isDefined(value)) value = param.type.decode(value);
      values[param.id] = param.value(value);
    }
    searchParams.forEach(param => {
      let value = search[param.id];
      for (let j = 0; j < param.replace.length; j++) {
        if (param.replace[j].from === value) value = param.replace[j].to;
      }
      if (isDefined(value)) value = param.type.decode(value);
      values[param.id] = param.value(value);
    });

I think this would be solved change the exec() to something like this:

    for (let i = 0; i < nPathSegments; i++) {
      const param: Param = pathParams[i];
      let value: any | any[] = match[i + 1];

      // if the param value matches a pre-replace pair, replace the value before decoding.
      for (let j = 0; j < param.replace.length; j++) {
        if (param.replace[j].from === value) value = param.replace[j].to;
      }
      if (value && param.array === true) value = decodePathArray(value);
      if (isDefined(value)){
		value = param.type.decode(value);
		if(typeof value === 'string' && !param.raw){
			value = decodeURIComponent(value);
		}
	  }
      values[param.id] = param.value(value);
    }
    searchParams.forEach(param => {
      let value = search[param.id];
      for (let j = 0; j < param.replace.length; j++) {
        if (param.replace[j].from === value) value = param.replace[j].to;
      }
      if (isDefined(value)){
		value = param.type.decode(value);
		if(typeof value === 'string' && !param.raw){
			value = decodeURIComponent(value);
		}
	  }
      values[param.id] = param.value(value);
    });
@thekip
Copy link

thekip commented Apr 8, 2020

Same issue here. Interesting that with AngularJS bindings I didn't encountered this issue, but when I switched to Angular (current one) it appeared (they both should use the same version of core and the same UrlMatcher).

Related issue in ui-router/angular repo ui-router/angular#340

@thekip
Copy link

thekip commented Apr 8, 2020

@christopherthielen FYI

@paolosanchi
Copy link
Author

Same issue here. Interesting that with AngularJS bindings I didn't encountered this issue, but when I switched to Angular (current one) it appeared (they both should use the same version of core and the same UrlMatcher).

Related issue in ui-router/angular repo ui-router/angular#340

I work with Angular (not AngularJs) so i didn't aware of this, Maybe angularjs does something special to urls itself, like to urldecode automatically.

@stale
Copy link

stale bot commented Oct 7, 2020

This issue has been automatically marked as stale because it has not had
recent activity. It will be closed if no further activity occurs.

This does not mean that the issue is invalid. Valid issues
may be reopened.

Thank you for your contributions.

@christopherthielen
Copy link
Member

FYI this was fixed in core@6.0.6 #618

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants