Skip to content

Commit

Permalink
fix(urlUtils): made removal of windows drive from path safer
Browse files Browse the repository at this point in the history
Prior to this fix, the urlResolve method would automatically
strip the first segment of a path if the segment ends in a colon.
This was to correct undesired behavior in the $location service
using the file protocol on windows in multiple browsers (see angular#4680).

However, there could be cases where users intentionally 
have first path segments that end in a colon 
(although this conflicts with section 3.3 of rfc3986).

The solution to this problem is an extra check to make sure
the first path segment of the input url does not end with a colon,
to make sure we're only removing undesired path segments.

Fixes angular#4939
  • Loading branch information
jeffbcross committed Nov 13, 2013
1 parent dfe6400 commit 59c0d98
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 19 deletions.
16 changes: 8 additions & 8 deletions src/ng/location.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,21 @@ function encodePath(path) {
return segments.join('/');
}

function parseAbsoluteUrl(absoluteUrl, locationObj) {
var parsedUrl = urlResolve(absoluteUrl);
function parseAbsoluteUrl(absoluteUrl, locationObj, appBase) {
var parsedUrl = urlResolve(absoluteUrl, appBase);

locationObj.$$protocol = parsedUrl.protocol;
locationObj.$$host = parsedUrl.hostname;
locationObj.$$port = int(parsedUrl.port) || DEFAULT_PORTS[parsedUrl.protocol] || null;
}


function parseAppUrl(relativeUrl, locationObj) {
function parseAppUrl(relativeUrl, locationObj, appBase) {
var prefixed = (relativeUrl.charAt(0) !== '/');
if (prefixed) {
relativeUrl = '/' + relativeUrl;
}
var match = urlResolve(relativeUrl);
var match = urlResolve(relativeUrl, appBase);
locationObj.$$path = decodeURIComponent(prefixed && match.pathname.charAt(0) === '/' ?
match.pathname.substring(1) : match.pathname);
locationObj.$$search = parseKeyValue(match.search);
Expand Down Expand Up @@ -91,7 +91,7 @@ function LocationHtml5Url(appBase, basePrefix) {
this.$$html5 = true;
basePrefix = basePrefix || '';
var appBaseNoFile = stripFile(appBase);
parseAbsoluteUrl(appBase, this);
parseAbsoluteUrl(appBase, this, appBase);


/**
Expand All @@ -106,7 +106,7 @@ function LocationHtml5Url(appBase, basePrefix) {
appBaseNoFile);
}

parseAppUrl(pathUrl, this);
parseAppUrl(pathUrl, this, appBase);

if (!this.$$path) {
this.$$path = '/';
Expand Down Expand Up @@ -158,7 +158,7 @@ function LocationHtml5Url(appBase, basePrefix) {
function LocationHashbangUrl(appBase, hashPrefix) {
var appBaseNoFile = stripFile(appBase);

parseAbsoluteUrl(appBase, this);
parseAbsoluteUrl(appBase, this, appBase);


/**
Expand All @@ -178,7 +178,7 @@ function LocationHashbangUrl(appBase, hashPrefix) {
throw $locationMinErr('ihshprfx', 'Invalid url "{0}", missing hash prefix "{1}".', url,
hashPrefix);
}
parseAppUrl(withoutHashUrl, this);
parseAppUrl(withoutHashUrl, this, appBase);
this.$$compose();
};

Expand Down
32 changes: 22 additions & 10 deletions src/ng/urlUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ var originUrl = urlResolve(window.location.href, true);
* | pathname | The pathname, beginning with "/"
*
*/
function urlResolve(url) {
function urlResolve(url, base) {
var href = url,
pathname;

Expand All @@ -92,10 +92,9 @@ function urlResolve(url) {
* do not include drive names for routing.
*/

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


// urlParsingNode provides the UrlUtils interface - http://url.spec.whatwg.org/#urlutils
return {
href: urlParsingNode.href,
Expand All @@ -107,13 +106,6 @@ function urlResolve(url) {
port: urlParsingNode.port,
pathname: pathname
};

function removeWindowsDriveName (path) {
var firstPathSegmentMatch;

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


Expand All @@ -129,3 +121,23 @@ function urlIsSameOrigin(requestUrl) {
return (parsed.protocol === originUrl.protocol &&
parsed.host === originUrl.host);
}

function removeWindowsDriveName (path, url, base) {
var firstPathSegmentMatch;

//Get the relative path from the input URL.
if (url.indexOf(base) === 0) {
url = url.replace(base, '');
}

/*
* The input URL intentionally contains a
* first path segment that ends with a colon.
*/
if (windowsFilePathExp.exec(url)) {
return path;
}

firstPathSegmentMatch = windowsFilePathExp.exec(path);
return firstPathSegmentMatch ? firstPathSegmentMatch[1] : path;
}
9 changes: 8 additions & 1 deletion test/ng/locationSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ describe('$location', function() {
};
}));


afterEach(inject(function ($sniffer) {
if ($sniffer.msie) return;
//reset urlParsingNode
Expand All @@ -50,6 +49,14 @@ describe('$location', function() {

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


it('should include the drive name if it was provided in the input url', function () {
url = new LocationHashbangUrl('file:///base', '#!');
url.$$parse('file:///base#!/C:/foo?a=b&c#hash');

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


Expand Down

1 comment on commit 59c0d98

@tbosch
Copy link

@tbosch tbosch commented on 59c0d98 Nov 13, 2013

Choose a reason for hiding this comment

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

LGTM.

Please sign in to comment.