angular: Using parenthesis in parameters throws an exception in RC4

I’m submitting a … (check one with “x”)

[x] bug report
[ ] feature request
[ ] support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior When trying to access a route using parameters that contain parenthesis, the application throws an exception: Error: Uncaught (in promise): Error: Cannot match any routes: ‘724,692’

When accessing http://localhost:5000/abajur;f=(724,692), the application throws the above mentioned exception.

On the other hand, http://localhost:5000/abajur;f=724,692 runs as expected.

Expected/desired behavior Route parameters should be allowed to contain parenthesis.

Reproduction of the problem If the current behavior is a bug or you can illustrate your feature request better with an example, please provide the steps to reproduce and if possible a minimal demo of the problem via https://plnkr.co or similar (you can use this template as a starting point: http://plnkr.co/edit/tpl:AvJOMERrnz94ekVua0u5).

What is the expected behavior?

What is the motivation / use case for changing the behavior? As per RFC 3986 , (see Section 2: Characters) may contain any of the following characters: ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._~:/?#[]@!$&'()*+,;=. Any other character needs to be encoded with the percent-encoding (%hh).

Please tell us about your environment:

  • Angular version: 2.0.0-rc. and “@angular/router”: “3.0.0-beta.2”,
  • Browser: [all | Chrome 51 ]
  • Language: [all | TypeScript 1.8 | ES6/7 | ES5 | Dart]

About this issue

  • Original URL
  • State: closed
  • Created 8 years ago
  • Reactions: 14
  • Comments: 37 (10 by maintainers)

Commits related to this issue

Most upvoted comments

In order to allow parentheses in the url, I defined my own custom URL serializer (also see suggestion here):

import { DefaultUrlSerializer, UrlSerializer, UrlTree } from '@angular/router';

export default class CustomUrlSerializer implements UrlSerializer {
    private _defaultUrlSerializer: DefaultUrlSerializer = new DefaultUrlSerializer();

    parse(url: string): UrlTree {
       // Encode parentheses
       url = url.replace(/\(/g, '%28').replace(/\)/g, '%29');
       // Use the default serializer.
       return this._defaultUrlSerializer.parse(url)
    }

    serialize(tree: UrlTree): string {
       return this._defaultUrlSerializer.serialize(tree).replace(/%28/g, '(').replace(/%29/g, ')');
    }
}

and I included CustomUrlSerializer in my providers array (within AppModule):

providers: [
    { provide: UrlSerializer, useClass: CustomUrlSerializer },
    ...
]

‘While these changes are not considered breaking because applications should be decoding URLs and key/value pairs, it is possible that some unit tests will break if comparing hard-coded URLs in tests since that hard coded string will represent the old encoding. Therefore we are releasing this fix in the upcoming Angular v6 rather than adding it to a patch for v5.

Wondering why this statement was made to hold off until v6, but it was included in 5.2.8? Was this a mistake or am I understanding this wrong?

Our app, which uses oidc-client-js, began failing yesterday until we reverted to 5.2.7 - in looking this morning at other comments/references for https://github.com/angular/angular/pull/22337 it appears it’s breaking for others as well - of note is https://github.com/okta/okta-oidc-js/issues/140

I’ve tried a really wild solution as I read the comment from @dchacke.

As a workaround what about patching encodeURIComponent?

const encode = encodeURIComponent;
window['encodeURIComponent'] = (component: string) => {
  return encode(component).replace(/[!'()*]/g, (c) => {
    // Also encode !, ', (, ), and *
    return '%' + c.charCodeAt(0).toString(16);
  });
}

It seems working for me, but I cannot predict the side effects of patching a global function.

I am having the same problem, and it’s also a security issue in the case of user supplied data as it will break routes, so I’m thinking this should be treated with a bit more urgency. Any progress on this?

As a workaround, and to speed things up, I think that one could change the encode function in modules/@angular/router/src/url_tree.ts:344 to:

export function encode(s: string): string {
  return encodeURIComponent(s).replace(/[!'()*]/g, (c) => {
    // Also encode !, ', (, ), and *
    return '%' + c.charCodeAt(0).toString(16);
  });
}

I hope this is the right place in the code! It may also be needed for query params, and even other places. Proper decoding for params may also be needed when params are injected into a component.

Essentially, you want to be looking for

<a [routerLink]="['foo', '(bar)']"></a>

to be turned into

<a href="/foo/%2528bar%2529"></a>

The reasoning for this is explained here (see last paragraph in the section called “Description”).

I was going to submit a PR for this, but this legal nonsense ruined it for me.

For anyone needing a workaround, use the following pipe:

import { Pipe, PipeTransform } from '@angular/core';

@Pipe({
  name: 'encodeURIComponent'
})

export class EncodeUriComponentPipe implements PipeTransform {
  transform(uriComponent: string): string {
    return encodeURIComponent(uriComponent).replace(/[!'()*]/g, (c) => {
      // Also encode !, ', (, ), and *
      return '%' + c.charCodeAt(0).toString(16);
    });
  }
}

and then

<a [routerLink]="['foo', ('(bar)' | encodeURIComponent)]">your link</a>

That fixes it for me, but it would be nice to see this adopted in Angular (if this is the correct solution, that is, as I may well be wrong).

With this routing pattern: path: 'items/:paramA/:paramB' and this url: localhost:4200/items/BA(p9)677/Something, I get Error: Cannot match any routes. URL Segment: 'p9'. Even if I manually decode it to localhost:4200/items/BA%28p9%29677/Something, I get the same error.

So this is fixed and will be released with the v6 release? Is it in any v6 betas/rcs?

Yes, thanks @wulfsberg - Jason is working to land it again.

Notes/fix plan:

  1. The URL in in the original issue above appears it should not work. The issue states that http://localhost:5000/abajur;f=(724,692) fails to match a route. However, http://localhost:5000/abajur;f= is a valid URL. Therefore if there’s something in parenthesis it would get parsed as another route. What if the URL were instead http://localhost:5000/abajur;f=(main:abc)? This would parse into two top-level UrlSegmentGroups.

Based on this, can we agree the URL above is invalid? And that a valid version of it would be: http://localhost:5000/abajur;f=(main:abc)

  1. Parentheses (not encoded) in URL parameters and/or fragments should probably not cause errors (this should match current behavior).

  2. Serializing a URL where params contain parens should serialize to the encoded version so refreshing and deep linking won’t cause errors.

This is really a show stopper when attempting to utilize the URI for state, e.g. example.com/(sessionid) => www.example.com/ Cannot match any routes: ‘sessionid’ (it removes the parenthesis from the error). Any ETA would be very helpful to at least notify my manager of this issue.

Looking into it. Stay tuned.