Skip to content

Commit

Permalink
fix(urlUtils): urlUtils doesn't return right path for file:// on win
Browse files Browse the repository at this point in the history
Chrome and other browsers on Windows often
append the drive name to the pathname,
as described in angular#4680. This would cause
the location service to browse to odd
URLs, such as /C:/myfile.html,
when opening apps using file://.

Fixes  angular#4680
  • Loading branch information
ROUL authored and jeffbcross committed Nov 12, 2013
1 parent c5c7538 commit 1ba0144
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 5 deletions.
51 changes: 46 additions & 5 deletions src/ng/urlUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,14 @@
// exactly the behavior needed here. There is little value is mocking these out for this
// service.
var urlParsingNode = document.createElement("a");
/*
Matches paths for file protocol on windows,
such as /C:/foo/bar, and captures only /foo/bar.
*/
var windowsFilePathExp = /^\/?.*?:(\/.*)/;
var originUrl = urlResolve(window.location.href, true);


/**
*
* Implementation Notes for non-IE browsers
Expand All @@ -27,7 +33,7 @@ var originUrl = urlResolve(window.location.href, true);
* browsers. However, the parsed components will not be set if the URL assigned did not specify
* them. (e.g. if you assign a.href = "foo", then a.protocol, a.host, etc. will be empty.) We
* work around that by performing the parsing in a 2nd step by taking a previously normalized
* URL (e.g. by assining to a.href) and assigning it a.href again. This correctly populates the
* URL (e.g. by assigning to a.href) and assigning it a.href again. This correctly populates the
* properties such as protocol, hostname, port, etc.
*
* IE7 does not normalize the URL when assigned to an anchor node. (Apparently, it does, if one
Expand Down Expand Up @@ -62,7 +68,9 @@ var originUrl = urlResolve(window.location.href, true);
*
*/
function urlResolve(url) {
var href = url;
var href = url,
pathname;

if (msie) {
// Normalize before parse. Refer Implementation Notes on why this is
// done in two steps on IE.
Expand All @@ -72,7 +80,34 @@ function urlResolve(url) {

urlParsingNode.setAttribute('href', href);

// $$urlParsingNode provides the UrlUtils interface - http://url.spec.whatwg.org/#urlutils
// ":"" protocol occurs on IE/Win for file://
/* jeffbcross and tbosch say:
Per section 3.3 of http://www.ietf.org/rfc/rfc3986.txt,

This comment has been minimized.

Copy link
@tbosch

tbosch Nov 12, 2013

I would remove the comment for lines 85-93.

the first path segment of a relative path reference
cannot contain a colon, so it is right to assume that
if the first section of the path contains a colon, it
is invalid.
However, to be safe, this check is only obliberating
the first segment if the LAST character of the segment
is a colon.
In Windows, on an anchor node on documents loaded from
the filesystem, the browser will return a pathname
prefixed with the drive name ('/C:/path') when a
pathname without a drive is set:
* a.setAttribute('href', '/foo')
* a.pathname === '/C:/foo' //true
Inside of Angular, we're always using pathnames that
do not include drive names for routing.
*/

pathname = removeWindowsDriveName(urlParsingNode.pathname);
pathname = (pathname.charAt(0) === '/') ? pathname : '/' + pathname;


// urlParsingNode provides the UrlUtils interface - http://url.spec.whatwg.org/#urlutils
return {
href: urlParsingNode.href,
protocol: urlParsingNode.protocol ? urlParsingNode.protocol.replace(/:$/, '') : '',
Expand All @@ -81,9 +116,15 @@ function urlResolve(url) {
hash: urlParsingNode.hash ? urlParsingNode.hash.replace(/^#/, '') : '',
hostname: urlParsingNode.hostname,
port: urlParsingNode.port,
pathname: urlParsingNode.pathname && urlParsingNode.pathname.charAt(0) === '/' ?
urlParsingNode.pathname : '/' + urlParsingNode.pathname
pathname: pathname
};

function removeWindowsDriveName (path) {

This comment has been minimized.

Copy link
@tbosch

tbosch Nov 12, 2013

We don't cover the case when the input url when urlResolve is called already contains the /C:/. In that case, we don't want to remove it afterwards. Cases for the input url where we don't want to remove the /C::

  • file://C:/...
  • /C:/...
  • C:/...
var firstPathSegmentMatch;

firstPathSegmentMatch = windowsFilePathExp.exec(path);
return firstPathSegmentMatch ? firstPathSegmentMatch[1] : path;
}
}


Expand Down
43 changes: 43 additions & 0 deletions test/ng/locationSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,49 @@ describe('$location', function() {
jqLite(document).off('click');
});


describe('File Protocol', function () {
var urlParsingNodePlaceholder;

beforeEach(inject(function ($sniffer) {
if ($sniffer.msie) return;

urlParsingNodePlaceholder = urlParsingNode;

//temporarily overriding the DOM element
//with output from IE, if not in IE
urlParsingNode = {
hash : "#/C:/",
host : "",
hostname : "",
href : "file:///C:/base#!/C:/foo",
pathname : "/C:/foo",
port : "",
protocol : "file:",
search : "",
setAttribute: angular.noop
};
}));


afterEach(inject(function ($sniffer) {
if ($sniffer.msie) return;
//reset urlParsingNode
urlParsingNode = urlParsingNodePlaceholder;
expect(urlParsingNode.pathname).not.toBe('/C:/foo');
}));


it('should not spill over to path() on WIN', function (){
//See issue #4680 for details
url = new LocationHashbangUrl('file:///base', '#!');
url.$$parse('file:///base#!/foo?a=b&c#hash');

expect(url.path()).toBe('/foo');
});
});


describe('NewUrl', function() {
beforeEach(function() {
url = new LocationHtml5Url('http://www.domain.com:9877/');
Expand Down

1 comment on commit 1ba0144

@tbosch
Copy link

@tbosch tbosch commented on 1ba0144 Nov 12, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, besides the comments above.

Please sign in to comment.