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

Leaving templateUrl empty in $routeProvider.when() crashes the browser #12176

Open
RoxKilly opened this issue Jun 20, 2015 · 21 comments
Open

Leaving templateUrl empty in $routeProvider.when() crashes the browser #12176

RoxKilly opened this issue Jun 20, 2015 · 21 comments

Comments

@RoxKilly
Copy link

AngularJS 1.4.0
Firefox 38.0.5 (Privacy Mode)

When setting up routing (ngRoute) in the module's .config(), using an empty string as the templateUrl will force the browser into either an infinite loop or a crash.

$routeProvider.when('/custom/route' {templateUrl:'', controller:'', controllerAs:''}) will crash. Replacing templateUrl with a string (even a non-existing file) does not cause the crash.

@caitp
Copy link
Contributor

caitp commented Jun 20, 2015

Can you elaborate on "crash"? If the browser crashes, that's a Firefox bug you should report on bugzilla.mozilla.org. If you just mean it throws, that seems expected

@caitp
Copy link
Contributor

caitp commented Jun 20, 2015

If we're doing some string processing that blocks the browser, then we should fix that, though.

@RoxKilly
Copy link
Author

When I first noticed the problem (by clicking on a link whose routing was set to a templateUrl:'', the entire browser (not just the tab) became unresponsive for about 10 seconds and my CPU usage rose to 100%, then a Firefox alert told me that a script on the page was taking too long and gave me the option to let the script run, or to stop the script. I chose to let it run and eventually the browser crashed (meaning it force-closed and the Mozilla Crash Reporter opened)

In subsequent tests, I can replicate the unresponsiveness and high CPU usage every time, but I no longer get the option to stop the script; the browser just crashes and the Crash Reporter opens.

This only happens when templateUrl is an empty string. I chose to report it here because I suspected either an infinite loop in AngularJS or perhaps a lack of input validation for the function that uses the templateUrl setting to setup the route.

@caitp
Copy link
Contributor

caitp commented Jun 20, 2015

Thanks for the details. And you're sure you can't replicate this on another browser? If we're blocking in an infinite loop somewhere you ought to be able to. I'll take a look when I can unless someone else gets to it first, or if it's been fixed already in 1.4.1

@RoxKilly
Copy link
Author

I have not tried it on another browser

@caitp
Copy link
Contributor

caitp commented Jun 20, 2015

Do you think you could put together a simple reproduction? I've gotten it started at http://plnkr.co/edit/W9ES8v50DFHh3LWUa9N8?p=preview but so far haven't been able to break any browsers I've tried it with

@caitp
Copy link
Contributor

caitp commented Jun 20, 2015

Nevermind, I've repro'ed it =)

@caitp
Copy link
Contributor

caitp commented Jun 20, 2015

http://plnkr.co/edit/iBeXT1Y5I71jEdOB3D4w?p=preview this will break in all browsers --- so we never emit $routeChangeError in this case, but it looks like we should be taking that path since we get the 404 for the empty template string.

This is not a regression, it's been broken since at least v1.2.0

@caitp
Copy link
Contributor

caitp commented Jun 20, 2015

This is pretty bad since it will affect all 404s, not just programming errors --- anyone want to fix this?

@gkalpak
Copy link
Member

gkalpak commented Jun 22, 2015

I can't reproduce the broken-ness. 404 errors seem to be handled correctly (and $routeChangeError seems to be emitted accordingly) - even in @caitp's plnkr.
What do I miss ?

@gkalpak
Copy link
Member

gkalpak commented Jun 22, 2015

It seems the issue arises when templateUrl: '' loads the same page it is called from (which obviously contains an ngView (which tries to load the same page etc)). (At least that's the only error I could reproduce.)

If we are talking about this error, then it's basically a nested ngView error (reproducible with any template that contains an ngView). A possible fix could be detecting a parent ngView inside the ngView directive, in which case we could emit an error and stop further processing.

@Narretz Narretz added this to the Backlog milestone Jun 22, 2015
@gdangelo
Copy link

I can reproduce the bug with the @caitp's plnkr or on my own by defining a route with an empty templateUrl option.

Looking at the source code, we can see that the commitRoute() function handles the route changes. And, some checks are done on the templateUrl using the angular.isDefined(templateUrl) function.

According to the doc this function determines if a reference is defined. In our case, with an empty templateUrl as parameter, the angular.isDefined('') returns true.
Hence, a simple fix could be adding a more complete check on the templateUrl to prevent an empty value to crash browsers.

However, the behavior in this case could be either to redirect to the URL displaying an empty html page without crash, or either to redirect anywhere else (default could be '/'). Throwing an error could also be done to trigger the $rootScope.$broadcast('$routeChangeError', ...).

Need feedbacks.

@gkalpak
Copy link
Member

gkalpak commented Jun 26, 2015

As already mentioned, the problem I can see is that using templateUrl: '' will load the page itself again, which (obviously) contains an ngView and tries to load a view (because of the ngView directive) and we have a nice infinite loop.

The cause of the problem (afaict) is not related to routing, but to nested ngView's creating infinite loops. IMO, this is what we should fix.

@gkalpak
Copy link
Member

gkalpak commented Jun 26, 2015

BTW, here is a quick-and-dirty fix (decorating the ngView directives), that "fixes" the issue (as a POC).
The same decoration "fixes" @caitp's demo as well (updated plnkr).

@gdangelo
Copy link

Indeed, the update() function inside the ngView directive is called infinitely when the templateUrl is empty.

This could be fixed by adding a check before attaching the content view to the DOM inside the update() function. Hence, the $transclude() function will not be triggered in this case.

@gkalpak, what do you think?

@gkalpak
Copy link
Member

gkalpak commented Jun 29, 2015

@gdangelo, imo the issue should be caught and prevented before reaching the update() function (to avoid the unnecessary overhead of other operations and listeners).

That said, it is pretty difficult to have a reliable solution (taking into account corner-cases such as multiple apps on the page and ng-non-bindable regions).

@vinkaga
Copy link

vinkaga commented Sep 4, 2015

I ran into this issue as well. The following code reproduces this problem reliably in all browsers (but not in codepen or plunkr). It is clearly an infinite loop issue as can be seen by repeated "Title" on page and repeated 'in modalCtrl' in console.

<!DOCTYPE html>
<html>
<head>
    <meta charset="utf-8">
    <meta name="fragment" content="!">
    <base href="/">
</head>

<body ng-app="Angular">

<h1>Title</h1>
<ng-view class="app-content"></ng-view>

<script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.4.5/angular.min.js"></script>
<script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.4.5/angular-route.min.js"></script>
<script>
    angular.module('g.modal', [
        'ngRoute',
    ]).controller('modalCtrl', [function() {
        console.log('in modalCtrl');
    }]);

    angular.module('Angular', [
        'ngRoute',
        'g.modal',
    ]).config(['$routeProvider', '$locationProvider', function($routeProvider, $locationProvider) {
        $routeProvider.when('/', {templateUrl: '',  controller: 'modalCtrl'});
        $locationProvider.html5Mode(true);
        $locationProvider.hashPrefix('!');
    }]);

</script>

</body>
</html>

@petebacondarwin
Copy link
Contributor

In what circumstances is an empty string a valid templateUrl? Surely that is a programming error in the application and Angular should throw an error, right?

@vinkaga
Copy link

vinkaga commented Sep 8, 2015

@petebacondarwin, yes it's due to programming bug - something that costed me more than 2 days to figure out. The same symptom occurs if the templateUrl points to a non-existent template.

@Narretz Narretz modified the milestones: Backlog, 1.7.x May 3, 2018
@Narretz Narretz self-assigned this May 3, 2018
@petebacondarwin petebacondarwin self-assigned this May 14, 2018
@petebacondarwin
Copy link
Contributor

Not sure if throwing errors would be BC at this stage? But I guess if we know it would always infinitely loop then it doesn't matter. The question is whether that is always the case?

@Narretz
Copy link
Contributor

Narretz commented May 14, 2018

I guess you could have a setup where an interceptor changes the url into something else? In that case an empty string would be okay.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.