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

IE 11 confuses $browser.url(), causing $location to go mad (infinite digest cycle, ...) #14427

Closed
bedag-moo opened this issue Apr 13, 2016 · 2 comments

Comments

@bedag-moo
Copy link
Contributor

Do you want to request a feature or report a bug?

A bug in $browser.url() that causes $location to go mad (infinite digest cycle, spontaneous navigation to previous views, ...).

What is the current behavior?

If $browser.url() is passed a url, it executes the following code:

      // Don't use history API if only the hash changed
      // due to a bug in IE10/IE11 which leads
      // to not firing a `hashchange` nor `popstate` event
      // in some cases (see #9143).
      if ($sniffer.history && (!sameBase || !sameState)) {
        history[replace ? 'replaceState' : 'pushState'](state, '', url);
        cacheState();
        // Do the assignment again so that those two variables are referentially identical.
        lastHistoryState = cachedState;
      } else {
        if (!sameBase || pendingLocation) {
          pendingLocation = url;
        }
        if (replace) {
          location.replace(url);
        } else if (!sameBase) {
          location.href = url;
        } else {
          location.hash = getHash(url);
        }
        if (location.href !== url) {
          pendingLocation = url;
        }

If sameBase and sameState are both true, and the caller has set replace, this invokes location.href=url. If url contains a substring that can be interpreted as a HTML entity reference, Internet Explorer 11 often resolves that entity reference (in our case, the url parameter &not-before-policy=0 was replaced by ¬-before-policy=0. Therefore, location.href !== url, and pendingLocation is set.

As a consequence, querying $browser.url(), which is implemented by

    // getter
    } else {
      // - pendingLocation is needed as browsers don't allow to read out
      //   the new location.href if a reload happened or if there is a bug like in iOS 9 (see
      //   https://openradar.appspot.com/22186109).
      // - the replacement is a workaround for https://bugzilla.mozilla.org/show_bug.cgi?id=407172
      return pendingLocation || location.href.replace(/%27/g,"'");
    }

yields the original url, which is probably fine.

However, future calls to $browser.url() to actually change the url satisfy the condition

  if ($sniffer.history && (!sameBase || !sameState)) {

therefore do not update pendingLocation, and $browser.url() continues to return the old url.

This is turn horribly confuses $location, because it activates the watch:

    $rootScope.$watch(function $locationWatch() {
      var oldUrl = trimEmptyHash($browser.url());
      var newUrl = trimEmptyHash($location.absUrl());
      var oldState = $browser.state();
      var currentReplace = $location.$$replace;
      var urlOrStateChanged = oldUrl !== newUrl ||
        ($location.$$html5 && $sniffer.history && oldState !== $location.$$state);

      if (initializing || urlOrStateChanged) {
        initializing = false;

        $rootScope.$evalAsync(function() {
          var newUrl = $location.absUrl();
          var defaultPrevented = $rootScope.$broadcast('$locationChangeStart', newUrl, oldUrl,
              $location.$$state, oldState).defaultPrevented;

          // if the location was changed by a `$locationChangeStart` handler then stop
          // processing this location change
          if ($location.absUrl() !== newUrl) return;

          if (defaultPrevented) {
            $location.$$parse(oldUrl);
            $location.$$state = oldState;
          } else {
            if (urlOrStateChanged) {
              setBrowserUrlWithFallback(newUrl, currentReplace,
                                        oldState === $location.$$state ? null : $location.$$state);
            }
            afterLocationChange(oldUrl, oldState);
          }
        });
      }

which always schedules an asyncTask to $broadcast the corresponding events, causing an infinite digest cycle.

Moreover, when $browser invokes onUrlChange in response to a browser event, it passes $browser.url(), which causes $location to spontaneously navigate back to the original url rather than the url the browser displays.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://plnkr.co or similar (template: http://plnkr.co/edit/tpl:yBpEi4).

In IE 11:

$browser.url('oauthCallback#state=xxx&not-before-policy=0');
$browser.url('oauthCallback#state=xxx&not-before-policy=0');
$browser.url('someOtherPage');
$browser.url(); // should return 'someOtherPage', but returns  `oauthCallback...`

Note that IE 11 performing HTML escaping in URLs is nondeterministic. On some environments, it occurs nearly every time, in others hardly ever. To get a deterministic reproducer, you'd have to mock location.

What is the expected behavior?

Obviously, invoking the setter of $browser.url() should cause the getter to report the new value.

What is the motivation / use case for changing the behavior?

Our users rather dislike infinite digest loops.

Which versions of Angular, and which browser / OS are affected by this issue? Did this work in previous versions of Angular? Please also test with the latest stable and snapshot (https://code.angularjs.org/snapshot/) versions.

We tested with angular 1.5.0, but the code in master looks unchanged. We suspect the regression was introduced by 8d39bd8

Other information (e.g. stacktraces, related issues, suggestions how to fix)

@bedag-moo bedag-moo changed the title $browser.url() IE 11 confuses $browser.url(), causing $location to go mad (infinite digest cycle, ...) Apr 13, 2016
@gkalpak gkalpak added this to the Backlog milestone Apr 14, 2016
@gkalpak
Copy link
Member

gkalpak commented Apr 14, 2016

So, IE11 is escaping &not even if it isn't an HTML entity (missing trailing ;) and does so undeterministically ? Nice !

@petebacondarwin
Copy link
Contributor

@bedag-moo really nice issue report. Thank you!

petebacondarwin pushed a commit that referenced this issue May 20, 2016
While $location expects that $browser stores the URL unchanged, "some browsers" transform the URL
when setting or defer the acutal update. To work around this, $browser.url() kept the unchanged
URL in pendingLocation.

However, it failed to update pendingLocation in all code paths, causing
$browser.url() to sometimes incorrectly report previous URLs, which horribly confused $location.

This fix ensures that pendingLocation is always updated if set, causing url() to report the
current url.

Fixes #14427
Closes #14499
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.

3 participants