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

LocationHashbangUrl.$$compose() creates nonsensical routes out of thin air #3141

Closed
ghost opened this issue Jul 5, 2013 · 9 comments
Closed

Comments

@ghost
Copy link

ghost commented Jul 5, 2013

In short, in 1.1.5, if your initial route is basically no route at all (pushState disabled and no hash present), Angular makes one up, redirects to it, and destroys window.location.

The first thing LocationHashbangUrl's constructor does is create an appBaseNoFile variable and assign it the output of stripFile(appBase).

If our appBase was something like
http://localhost:3000/some/resource/path

then appBaseNoFile is going to contain http://localhost:3000/some/resource/.

Later on, $$parse does this bit:

var withoutBaseUrl = beginsWith(appBase, url) || beginsWith(appBaseNoFile, url);

and withoutBaseUrl now contains path.

Shortly thereafter, it essentially copies this value to another variable: withoutHashUrl

It then calls matchAppUrl passing it this withoutHashUrl which remains unchanged.

Eventually we get to $$compose which decides its $$url and $$path variables should contain /path and suddenly $$absUrl contains the completely invalid route

http://localhost:3000/some/resource/path#/path

Which the window's location is set to by the location provider, causing all kinds of problems.

If Angular absolutely must create a route in order to function, it should at least be a valid one, like /#/ rather than the last path component repeated for no apparent reason.

@ghost
Copy link
Author

ghost commented Jul 5, 2013

Probably related to #2833

@martinstein
Copy link

also related/same issue as #2860

@busticated
Copy link

trying to follow the thread amongst all these related issues is tough... but fwiw, in my app when using v1.1.5 these two routes are treated the same when entered directly into the browser's address bar:

<site.com>/foo

<site.com>/foo/bar/baz/qux/hamsammich/potato/foo

likewise and more problematically, this route:

<site.com>/foo/edit/:id

ends up being interpreted as:

<site.com>/:id

clicking down into the above url works... refreshing the browser or using a bookmark does not.

downgrading to v1.1.4 fixed it for me.

@ghost
Copy link
Author

ghost commented Jul 10, 2013

I gave up and went to 1.0.7 or whatever's "stable." This is just too horribly broken to mess with.

@kanwei
Copy link
Contributor

kanwei commented Jul 19, 2013

I fixed this in my custom repo by adding the last line as follows:

function LocationHashbangUrl(appBase, hashPrefix) {
  var appBaseNoFile = stripFile(appBase);
  appBaseNoFile = appBase;

It doesn't seem like we should be using appBaseNoFile for LocationHashbangUrl.

@adnansorg
Copy link

Since I use html5mode set to true with modern browsers and false for IE9 or less, I had to modify the function LocationHashbangUrl(appBase, hashPrefix) as well.

    function LocationHashbangUrl(appBase, hashPrefix) {
        var appBaseNoFile = stripFile(appBase);
        ...

I changed the stripFile(appBase) function to reference the <base href="/rootPath" /> tag.

function stripFile(url) {
    return url.substr(0, document.getElementsByTagName('base')[0].href.length);
}

and in the LocationHashbangUrl function, I use this new variable to create the $$absUrl

...
this.$$absUrl = appBaseNoFile + (this.$$url ? hashPrefix + this.$$url : '');
...

When using html5mode(true) everything works normally on modern browsers, but on IE9 or less, html5mode(false) is switched on and the hashbang url can be created when inputing non-hashbang urls. For example:

With <base href="/rootPath" /> and html5mode(true)

http://www.domain.com/rootPath/property/123/items/3 works ok in modern browsers as normal.

When loading the above link in IE9 or less, the url becomes: http://www.domain.com/rootPath#/property/123/items/3 and immediately links to the relevant page needed.

I hope this helps someone who is also looking to fix this.

@jeffbcross
Copy link
Contributor

@oncomeme is this issue still biting you in 1.2.x? We refactored the underlying url parsing logic in 1.2. I'm closing this issue, but feel free to reopen. If you do need to reopen, can you provide a plunker demonstrating the issue?

@ghost
Copy link
Author

ghost commented Mar 10, 2014

No idea. Can't upgrade any time soon.

@kanwei
Copy link
Contributor

kanwei commented Mar 10, 2014

Yes, this is confirmed to be fixed

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

No branches or pull requests

5 participants