Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Angular terminates when parsing cookies and decodeURIComponent throws error. #9211

Closed
robertbeuligmann opened this issue Sep 22, 2014 · 11 comments

Comments

@robertbeuligmann
Copy link

Tested with v1.3.0-rc.2

As far as I can tell, when angular goes to make a web request it parses the cookies that are set on the local machine. If any cookie causes decodeURIComponent to throw an error Angular will terminate. If this happens during the routing phase the view is left blank.

Origin: /src/ng/browser.js line 327
Lib: angular.js line 4832

Plunker: http://plnkr.co/edit/ZLg6SNrCteqGRrPhThWm?p=info

To reproduce, create a cookie "anyname=%abcd", use example app below.

test.html

    angular.module('testapp', ['ngRoute']);
    angular.module('testapp').config(['$routeProvider', function ($routeProvider) {
        $routeProvider
        .when('/', {
            templateUrl: 'testtemplate.html',
        })
        .otherwise({ redirectTo: '/' });
    }]);

testtemplate.html

    <b>{{1 +1}}</b>
@robertbeuligmann robertbeuligmann changed the title Angular Fails to start, when parsing cookies and decodeURIComponent throws error. Angular fails to start, when parsing cookies and decodeURIComponent throws error. Sep 22, 2014
@btford
Copy link
Contributor

btford commented Sep 22, 2014

Can you please put your reproduction into a jsbin, plnkr.co, or jsfiddle?

This saves us a lot of time. Thanks!

@btford btford added this to the Backlog milestone Sep 22, 2014
@robertbeuligmann
Copy link
Author

Created Plunker
http://plnkr.co/edit/ZLg6SNrCteqGRrPhThWm?p=info

@robertbeuligmann robertbeuligmann changed the title Angular fails to start, when parsing cookies and decodeURIComponent throws error. Angular terminates when parsing cookies and decodeURIComponent throws error. Sep 22, 2014
@robertbeuligmann
Copy link
Author

I do not know if this is really related to ngCookies as the error is in /src/ng/browser.js. As shown in the plunk, ngCookies is not required for this error to occur. This error is in the core angular.js.

@caitp
Copy link
Contributor

caitp commented Sep 22, 2014

This appears to be a native error, which breaks the app. The cookie value is not something decodeURIComponent() can deal with (for whatever reason, because of this failure, the template never gets requested)

@robertbeuligmann
Copy link
Author

Should it assume that a cookie value can be decoded using decodeURIComponent?

@caitp
Copy link
Contributor

caitp commented Sep 22, 2014

I believe that would make the most sense, would it not? (I don't know really, cookies are weird, lets use localStorage instead :c)

@robertbeuligmann
Copy link
Author

Seeing as the cookie value is not exclusively a URI I do not see how it would be a safe assumption to make. The original spec says no encoding is assumed, while the newer spec is still pretty vague about what can be there. The problem remains that a value that does not work with decodeURIComponent breaks the app.

References:
Modern Cookies: http://tools.ietf.org/html/rfc6265
Original Netscape Cookies: http://curl.haxx.se/rfc/cookie_spec.html

@caitp
Copy link
Contributor

caitp commented Sep 22, 2014

there's going to be some reason why we're doing it, otherwise we wouldn't be doing it!

I have no idea what that reason is or was, however. It's possible that we can throw it out.

@shahata
Copy link
Contributor

shahata commented Sep 22, 2014

It makes sense that Angular is doing this since some characters (= and ; for instance) cannot appear in cookie name and value, so when cookies are written using Angular, it needs to encode the name and value in order to make sure that those characters are not serialized as is (and obviously it needs to decode them when reading). The problem is that no one makes any promises that other cookie writers will also choose to use this encoding and so decoding a cookie written by someone else might result with the wrong value or even fail.

@shahata
Copy link
Contributor

shahata commented Sep 23, 2014

So I think there is no good solution here and we'll end up doing something ugly no matter what. One option is to revert back to unescape instead of decodeURIComponent (this was introduced in #8125), but I think it would be better to protect the decoding with try/catch and return the original string as fallback. I'll send a PR soon.

@IcanDivideBy0
Copy link

Created a decorator to quick fix this since it totally break angular
https://gist.github.com/IcanDivideBy0/987517c70533d045ffe2

@shahata shahata closed this as completed in 9c99590 Oct 9, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants