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

bug($location): incorrect URL parsing when (1) angular app lives under a subdirectory (2) HTML5 routing is off and (3) there's no trailing slash when accessing app root #2833

Closed
chibicode opened this issue May 30, 2013 · 24 comments

Comments

@chibicode
Copy link

Suppose you turned off HTML5 routing and your app lives at

http://localhost:5000/angular-app/

On version 1.1.5, if you navigate instead to a URL with no trailing slash, e.g.

http://localhost:5000/angular-app

then it redirects to:

http://localhost:5000/angular-app/#angular-app

I tracked down the problem - it's on this line:
https://github.com/angular/angular.js/blob/v1.1.5/src/ng/location.js#L158

  • If there's trailing slash (i.e. http://localhost:5000/angular-app/), beginsWith(appBase, url) returns /. So withoutBaseUrl is set as /.
  • However, if there's no trailing slash (i.e. http://localhost:5000/angular-app), beginsWith(appBase, url) returns an empty string, and it evaluates beginsWith(appBaseNoFile, url), which returns angular-app. So withoutBaseUrl is set as angular-app.

As a result, when HTML5 route is off, this.$$parse behaves differently for http://localhost:5000/angular-app (no trailing slash) and http://localhost:5000/angular-app/ (trailing slash), even though they should return the same thing.

The change is introduced on this commit (cc @mhevery) 58ef323

This is not a problem when (1) HTML5 routing is on (e.g. $locationProvider.html5Mode(true).hashPrefix('!')) and (2) <base href="/angular-app/" is added to <head>.

@michaelnatkin
Copy link

I believe the actual precise location of the bug is in LocationHashbangUrl.$$rewrite. Note that it also accidentally (afaict) returns nil if the condition doesn't match.

@michaelnatkin
Copy link

Possibly a similar bug in LocationHashbangInHtml5Url.$$rewrite - it also looks like it might be able to return nil, or at least there is no default else clause.

@lambdahands
Copy link

I'm having some issues related to this bug as well. I have $locationProvider.html5Mode set to true, but my app runs in a subdirectory, '/foo' for example. My links and routeProvider work fine if I navigate my app without page refreshes, but going to '/foo/bar' through my address bar sets my ngView to blank. The same applies to refreshing the page at any location in my app.

When I turned html5Mode off, I realized my location hashes were way out of wack. Even after setting the base path to '/foo', I would be redirected to '/foo#foo'. I'm not entirely sure how relevant my problem is to this issue, but it seemed to follow similar symptoms.

I've tested my app in stable releases, and no problems are raised, so I'm assuming it has to do with this latest unstable release.

@fessyfoo
Copy link

The issue described above occurs when html5 mode is off. see #2762 and #2799 relating to behavior changes in html5 mode. I think the issue when html5 is on is distinct from the issue when html5 mode is off.

@fessyfoo
Copy link

I worked a little bit on writing tests to illustrate this breakage, basically trying to pathologically enumerate all the initial starting urls possible and how that is expected to behave. I could use feedback on the assumptions being made, see fessyfoo/angular.js@efd44f6 the tests currently produce 5 failures. but at least 4 of them could be my assumptions being wrong. :) 1 of them is the case above, which fails no matter what I assume is correct.

basically consider the following urls:

http://domain.com/base/index.html
http://domain.com/base/index.html#!/a/b?foo=bar
http://domain.com/base/index.html#!?foo=bar
http://domain.com/base/index.html?foo=bar

http://domain.com/base/index.html#!foo

http://domain.com/base/
http://domain.com/base/#!/a/b?foo=bar
http://domain.com/base/#!?foo=bar
http://domain.com/base/?foo=bar

assume that html5mode is off. that base[href] is not set, that hashPrefix is set to '!'
what should the initial values of $location.url(), $location.path(), and $browser.url() be.

for example, the very interesting case of no hash:

http://domain.com/base/index.html 

in v1.1.5 is the $location.absUrl and therefore $browser.url get changed to:

http://domain.com/base/index.html#!/index.html 

some other options are:

  1. assume that because no base[href]/appBase is set we want to convert any path info into the hash portion:

    http://domain.com/base/index.html#!/base/index.html
    
  2. assume that because html5mode is false, we don't need to get any path info from urls without a hash portion, and that the path should be '/'. (we ignore any appBase or base[href])

    http://domain.com/base/index.html#!/
    
  3. like 2, but realize that we want to be stylish and not show a '/' if that's all there is:

    http://domain.com/base/index.html
    

Now consider that appBase was set, (currently via base[href]) how should it be interpreted in the case where html5Mode is off and there is no initial hash? how should it effect interpreting paths embedded in the hash?

Feedback on fessyfoo/angular.js@efd44f6 might allow me to continue and turn it into an actual pull request.

@michaelnatkin
Copy link

Agree. Assuming appbase == base isn't good at least in my app as it implies that all relative urls under that base are Ajax urls handled by angular and should be rewritten in nonhtml5 mode. That isn't the case for me. What is like to see is that only urls that match an angular route are rewritten.

Sent from my iPhone

On Jun 27, 2013, at 7:23 AM, fessyfoo notifications@github.com wrote:

The issue described above occurs when html5 mode is off. see #2762 and #2799 relating to behavior changes in html5 mode. I think the issue when html5 is on is distinct from the issue when html5 mode is off.


Reply to this email directly or view it on GitHub.

@jssebastian
Copy link

I think option 3 is definitely the "right" way to go. If a user does not have html5 mode enabled, I do not see how implicitly forcing unrequested URL rewriting on him is a feature.

If a user wants any of the above URL rewriting behavior a simple route definition with redirectTo will achieve that in 1 line of code.

Changing the base_href to avoid this behavior has other undesirable side effects in non-html5-mode apps (unwanted relative URL changes).

@jssebastian
Copy link

To elaborate on this a bit further: I think angular should not assume that everyone is doing single-page apps: an angular app may have to seamlessly share a URL space with non-angular, legacy code.

@fessyfoo
Copy link

I think I was just beginning to understand/explore what it would mean for the location code to have an idea of an application base. which is what it seems was being considered when the breaking changes were made,

for example

like 1, we assume that with no appbase set then appbase is '/'. but we are stylish. so looks like 3, but note $location.path()

    http://domain.com/base/index.html
    $location.path() == "/base/index.html"

showing how those assumptions, stylish, and appbase of '/' work with a path:

    http://domain.com/base/index.html#!/a/b

then

    $location.path() == "/base/index.html/a/b"

I think this matches the way html5mode makes it's assumptions. Though I believe most of us with html5mode off would prefer number 3. So I'd like to re-describe number 3 as, when html5mode is off, the default appbase is the current urls' path. (/base/index.html) so:

http://domain.com/base/index.html#!/a/b  
$location.path() == "/a/b"

http://domain.com/base/index.html 
$location.path() == "/"

If appbase were configured as something else, like /base then the same logic would give:

http://domain.com/base/#!/index.html/a/b  
$location.path() == "/index.html/a/b"

http://domain.com/base/index.html 
$location.path() == "/index.html"

http://domain.com/base/
$location.path() == "/"

// and an interesting case:
http://domain.com/base
$location.path() == "/"

this allows the idea of an appbase value to be involved, the same as it is for html5mode on. and it's just a matter of picking the correct default setting.

Note this definition is close to what's happening in v1.1.5 the assumption is that the appbase is the path portion of the url without any file portion.

Heh. I should just fix this in some way and submit the PR. 😁 problem for me is I switched to html5mode. and have other things I should be thinking about.

@michaelnatkin
Copy link

To elaborate on this a bit further: I think angular should not assume
that everyone is doing single-page apps: an angular app may have to
seamlessly share a URL space with non-angular, legacy code.

Precisely.


Michael Natkin
CTO
http://ChefSteps.com http://chefsteps.com/

On Fri, Jun 28, 2013 at 11:27 AM, jssebastian notifications@github.51.alwrote:

To elaborate on this a bit further: I think angular should not assume that
everyone is doing single-page apps: an angular app may have to seamlessly
share a URL space with non-angular, legacy code.


Reply to this email directly or view it on GitHubhttps://github.com//issues/2833#issuecomment-20205653
.

@jssebastian
Copy link

@fessyfoo: I don't think the concept of appBase is really applicable to non-html5 mode. In your example above, the only value for appBase that makes sense is the full path http://domain.com/base/index.html.

Your example with e.g. appBase value of http://domain.com/ makes no sense to me, because $location.path is a getter/setter. If for url http://domain.com/base/index.html#/a/b", $location.path is /base/index.html/a/b, what happens when I set $location.path("/notbase")?

@fessyfoo
Copy link

@jssebastian I agree mostly. I think any other appBase than the full path is confusing. and I think it should be the default. and that fix is probably the most expedient. it similar to the fix on the html5mode on side to assume '/' aef0980.

for your example: $location.path("/notbase") would cause the browser to a round trip to the server. because html5mode is off and there was more than just a hash change. the server could chose to serve the same angular application from that url as well. and when it loaded any route for "/notbase" would match. Someone might want to have things work that way.

anyhow the current code is trying to make use of an appbase so the least change would be to make some concept of that work in a sane way. It's a bigger change to remove the concept entirely. and I'm not an angular dev. 😁 I was mostly concerned with getting tests in place that would prevent this type of regression.

@fessyfoo
Copy link

I simplified my test cases so they're easier to read and configured the results so they pass against v1.1.4. I left out any specs with base[href] set and will ignore that issue for now. ( though v1.1.4 I believe ignores base[href] ) I'll add these specs to master and try to fix what broke. Here they are in case anyone wants to think about the assertions they make, keep in mind this is what passes on v1.1.4

Update:  tests removed.  see plunkr links in next post.

@fessyfoo
Copy link

fessyfoo commented Jul 4, 2013

I tried to fix things, but i hit issues where html5mode off is connected to html5mode on with fallback to hashbang urls, and so it's more complicated. I need to get an understanding of how the whole thing is supposed to work.

I've asked for help on the angular mailing list.

Here's my assumptions coded in jasmine specs in angular, hopefully it's more readable that way.

with angular 1.1.5 I get 27 out of 36 failures: http://plnkr.co/edit/FepZHtZUWDvyqllzklSi?p=preview

same tests with angular 1.1.4 I get 19 out of 36 failures: http://plnkr.co/edit/DqkASF?p=preview

and then again with angular 1.0.7, I get 19 out of 36 failures: http://plnkr.co/edit/zX5JeJymYDOTqTEAPp6b?p=preview

@jssebastian
Copy link

@fessyfoo: i've not used html5 mode yet, so I can't really comment on the test failures that occur in html5 mode. But the non-html5 mode failures that I see in 1.0.7 and 1.1.4 seem related to your tests expecting that "/" be added to the path.

This may be the right thing to do, I don't know. However, I think there are two issues that should be separated.

One is fixing the bug in 1.1.5 that redirects index.html to index.html#/index.html. This is just a bug, for which several people have provided reproducible test cases so hopefully we can get it be fixed.

The other is your proposal of rationalizing paths, which sounds interesting but requires someone with more understanding of html5 mode than me to comment on whether it's a good idea.

I think we should break this up into 2 different stories (maybe re-open one of the existing ones). What do you think?

@fessyfoo
Copy link

fessyfoo commented Jul 5, 2013

I agree with you it would be nice to just fix the bug in 1.1.5 that redirects index.html to index.html#/index.html. I had a simple fix. and this broke existing tests in angular related to html5 fallback to hashbang mode. so I needed to have an understanding of how that worked to make the fix without breaking them.

I'm not trying to propose anything about rationalizing paths. I'm trying to state my understanding of the code and the documentation about html5mode and html5 fallback ( http://docs.angularjs.org/guide/dev_guide.services.$location ) The latest code has tried to make html5 fallback mode and html5mode off, one and the same. However, I think they have different concerns.

I think fixing html5mode off, requires not breaking html5 fallback mode, which connects things to html5mode on. but this issue is still really about fixing the most obvious bit about html5mode off turning index.html to index.html#/index.html. and ideally there wouldn't be a second issue unless I've really discovered that html5mode on and html5 fallback mode are both broken as well.

sorry if I appear to be cluttering the issue. :(

@jssebastian
Copy link

@fessyfoo: thanks for the clarification. I see your point.

@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. Could an angular core dev please comment on this? @IgorMinar

@jssebastian
Copy link

@kanwei: thanks, I had been trying to do something like that in my local repo with no success so far, and had reverted to 1.1.4.

I guess we have to see if this patch has side-effects on html5-fallback.

@donutcho
Copy link

@kanwei Thank you! very simple solution works for me. (me also added for local v1.1.5 source only 1 line:)

@beppler
Copy link

beppler commented Aug 16, 2013

@kanwei: it worked for me too (1.1.5).

shuhaowu added a commit to shuhaowu/projecto that referenced this issue Feb 16, 2014
@abratashov
Copy link

@Narretz Narretz added this to the Backlog milestone Jul 1, 2014
@Narretz
Copy link
Contributor

Narretz commented Oct 18, 2016

Location has seen some major changes since 1.1.5, so this probably has been solved. If somebody encounter this problem again, please open a new issue.

@Narretz Narretz closed this as completed Oct 18, 2016
@varunprakas
Copy link

I am facing some issues when i enabled html5mode. It's working fine without nested urls but when i try to refresh a page url contains nested states. it's showing some error in the console. when i check in the console the file path is wrong. but in normal url i am not facing any issues. i am giving my code details below

  1. app.js
$locationProvider.html5Mode({
      enabled: true,
      requireBase: false
    }); 
	.state('/contactus', {
    url: '/contactus',
    template: '<ui-view></ui-view>',
     abstract: true,
    authenticate: true,}
	
	state('/contactus.contactuspage', {
    url: '/contactuspage',
    templateUrl: 'views/contactus-planning.html',
    controller: 'contactusCtrl'}

  1. index.html

<base href="/alp/member/">

  1. server
  <Directory /var/alp/app/member>
    #for angular pretty URL
    RewriteEngine on
 
    # Don't rewrite files or directories
    RewriteCond %{REQUEST_FILENAME} -f [OR]
    RewriteCond %{REQUEST_FILENAME} -d
    RewriteRule ^ - [L]
 
    # Rewrite everything else to index.html to allow html5 state links
    RewriteRule ^ /alp/member/index.html [L]
  </Directory>

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

No branches or pull requests