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

Commit b019a48

Browse files
jeffbcrossIgorMinar
authored andcommitted
refactor(location): $location now uses urlUtils, not RegEx
The location service, and other portions of the application, were relying on a complicated regular expression to get parts of a URL. But there is already a private urlUtils provider, which relies on HTMLAnchorElement to provide this information, and is suitable for most cases. In order to make urlUtils more accessible in the absence of DI, its methods were converted to standalone functions available globally. The urlUtils.resolve method was renamed urlResolve, and was refactored to only take 1 argument, url, and not the 2nd "parse" boolean. The method now always returns a parsed url. All places in code which previously wanted a string instead of a parsed url can now get the value from the href property of the returned object. Tests were also added to ensure IPv6 addresses were handled correctly. Closes #3533 Closes #2950 Closes #3249
1 parent 74ef7f1 commit b019a48

10 files changed

+152
-246
lines changed

src/AngularPublic.js

+1-2
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,7 @@ function publishExternalAPI(angular){
127127
$sniffer: $SnifferProvider,
128128
$templateCache: $TemplateCacheProvider,
129129
$timeout: $TimeoutProvider,
130-
$window: $WindowProvider,
131-
$$urlUtils: $$UrlUtilsProvider
130+
$window: $WindowProvider
132131
});
133132
}
134133
]);

src/ng/compile.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -276,9 +276,9 @@ function $CompileProvider($provide) {
276276

277277
this.$get = [
278278
'$injector', '$interpolate', '$exceptionHandler', '$http', '$templateCache', '$parse',
279-
'$controller', '$rootScope', '$document', '$sce', '$$urlUtils', '$animate',
279+
'$controller', '$rootScope', '$document', '$sce', '$animate',
280280
function($injector, $interpolate, $exceptionHandler, $http, $templateCache, $parse,
281-
$controller, $rootScope, $document, $sce, $$urlUtils, $animate) {
281+
$controller, $rootScope, $document, $sce, $animate) {
282282

283283
var Attributes = function(element, attr) {
284284
this.$$element = element;
@@ -370,9 +370,9 @@ function $CompileProvider($provide) {
370370
// sanitize a[href] and img[src] values
371371
if ((nodeName === 'A' && key === 'href') ||
372372
(nodeName === 'IMG' && key === 'src')) {
373-
// NOTE: $$urlUtils.resolve() doesn't support IE < 8 so we don't sanitize for that case.
373+
// NOTE: urlResolve() doesn't support IE < 8 so we don't sanitize for that case.
374374
if (!msie || msie >= 8 ) {
375-
normalizedVal = $$urlUtils.resolve(value);
375+
normalizedVal = urlResolve(value).href;
376376
if (normalizedVal !== '') {
377377
if ((key === 'href' && !normalizedVal.match(aHrefSanitizationWhitelist)) ||
378378
(key === 'src' && !normalizedVal.match(imgSrcSanitizationWhitelist))) {

src/ng/http.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,8 @@ function $HttpProvider() {
132132
*/
133133
var responseInterceptorFactories = this.responseInterceptors = [];
134134

135-
this.$get = ['$httpBackend', '$browser', '$cacheFactory', '$rootScope', '$q', '$injector', '$$urlUtils',
136-
function($httpBackend, $browser, $cacheFactory, $rootScope, $q, $injector, $$urlUtils) {
135+
this.$get = ['$httpBackend', '$browser', '$cacheFactory', '$rootScope', '$q', '$injector',
136+
function($httpBackend, $browser, $cacheFactory, $rootScope, $q, $injector) {
137137

138138
var defaultCache = $cacheFactory('$http');
139139

@@ -649,7 +649,7 @@ function $HttpProvider() {
649649
config.headers = headers;
650650
config.method = uppercase(config.method);
651651

652-
var xsrfValue = $$urlUtils.isSameOrigin(config.url)
652+
var xsrfValue = urlIsSameOrigin(config.url)
653653
? $browser.cookies()[config.xsrfCookieName || defaults.xsrfCookieName]
654654
: undefined;
655655
if (xsrfValue) {

src/ng/httpBackend.js

+1-2
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,7 @@ function createHttpBackend($browser, XHR, $browserDefer, callbacks, rawDocument,
102102
}
103103

104104
function completeRequest(callback, status, response, headersString) {
105-
// URL_MATCH is defined in src/service/location.js
106-
var protocol = (url.match(SERVER_MATCH) || ['', locationProtocol])[1];
105+
var protocol = locationProtocol || urlResolve(url).protocol;
107106

108107
// cancel timeout and subsequent timeout promise resolution
109108
timeoutId && $browserDefer.cancel(timeoutId);

src/ng/location.js

+29-27
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
'use strict';
22

3-
var SERVER_MATCH = /^([^:]+):\/\/(\w+:{0,1}\w*@)?(\{?[\w\.-]*\}?)(:([0-9]+))?(\/[^\?#]*)?(\?([^#]*))?(#(.*))?$/,
4-
PATH_MATCH = /^([^\?#]*)(\?([^#]*))?(#(.*))?$/,
3+
var PATH_MATCH = /^([^\?#]*)(\?([^#]*))?(#(.*))?$/,
54
DEFAULT_PORTS = {'http': 80, 'https': 443, 'ftp': 21};
65
var $locationMinErr = minErr('$location');
76

@@ -23,39 +22,40 @@ function encodePath(path) {
2322
return segments.join('/');
2423
}
2524

26-
function matchUrl(url, obj) {
27-
var match = SERVER_MATCH.exec(url);
25+
function parseAbsoluteUrl(absoluteUrl, locationObj) {
26+
var parsedUrl = urlResolve(absoluteUrl);
2827

29-
obj.$$protocol = match[1];
30-
obj.$$host = match[3];
31-
obj.$$port = int(match[5]) || DEFAULT_PORTS[match[1]] || null;
28+
locationObj.$$protocol = parsedUrl.protocol;
29+
locationObj.$$host = parsedUrl.hostname;
30+
locationObj.$$port = int(parsedUrl.port) || DEFAULT_PORTS[parsedUrl.protocol] || null;
3231
}
3332

34-
function matchAppUrl(url, obj) {
35-
var match = PATH_MATCH.exec(url);
3633

37-
obj.$$path = decodeURIComponent(match[1]);
38-
obj.$$search = parseKeyValue(match[3]);
39-
obj.$$hash = decodeURIComponent(match[5] || '');
34+
function parseAppUrl(relativeUrl, locationObj) {
35+
var prefixed = (relativeUrl.charAt(0) !== '/');
36+
if (prefixed) {
37+
relativeUrl = '/' + relativeUrl;
38+
}
39+
var match = urlResolve(relativeUrl);
40+
locationObj.$$path = decodeURIComponent(prefixed && match.pathname.charAt(0) === '/' ? match.pathname.substring(1) : match.pathname);
41+
locationObj.$$search = parseKeyValue(match.search);
42+
locationObj.$$hash = decodeURIComponent(match.hash);
4043

4144
// make sure path starts with '/';
42-
if (obj.$$path && obj.$$path.charAt(0) != '/') obj.$$path = '/' + obj.$$path;
45+
if (locationObj.$$path && locationObj.$$path.charAt(0) != '/') locationObj.$$path = '/' + locationObj.$$path;
4346
}
4447

4548

46-
function composeProtocolHostPort(protocol, host, port) {
47-
return protocol + '://' + host + (port == DEFAULT_PORTS[protocol] ? '' : ':' + port);
48-
}
49-
5049
/**
5150
*
5251
* @param {string} begin
5352
* @param {string} whole
54-
* @param {string} otherwise
55-
* @returns {string} returns text from whole after begin or otherwise if it does not begin with expected string.
53+
* @returns {string} returns text from whole after begin or undefined if it does not begin with expected string.
5654
*/
57-
function beginsWith(begin, whole, otherwise) {
58-
return whole.indexOf(begin) == 0 ? whole.substr(begin.length) : otherwise;
55+
function beginsWith(begin, whole) {
56+
if (whole.indexOf(begin) == 0) {
57+
return whole.substr(begin.length);
58+
}
5959
}
6060

6161

@@ -87,20 +87,22 @@ function LocationHtml5Url(appBase, basePrefix) {
8787
this.$$html5 = true;
8888
basePrefix = basePrefix || '';
8989
var appBaseNoFile = stripFile(appBase);
90+
parseAbsoluteUrl(appBase, this);
91+
92+
9093
/**
9194
* Parse given html5 (regular) url string into properties
9295
* @param {string} newAbsoluteUrl HTML5 url
9396
* @private
9497
*/
9598
this.$$parse = function(url) {
96-
var parsed = {}
97-
matchUrl(url, parsed);
9899
var pathUrl = beginsWith(appBaseNoFile, url);
99100
if (!isString(pathUrl)) {
100101
throw $locationMinErr('ipthprfx', 'Invalid url "{0}", missing path prefix "{1}".', url, appBaseNoFile);
101102
}
102-
matchAppUrl(pathUrl, parsed);
103-
extend(this, parsed);
103+
104+
parseAppUrl(pathUrl, this);
105+
104106
if (!this.$$path) {
105107
this.$$path = '/';
106108
}
@@ -151,7 +153,7 @@ function LocationHtml5Url(appBase, basePrefix) {
151153
function LocationHashbangUrl(appBase, hashPrefix) {
152154
var appBaseNoFile = stripFile(appBase);
153155

154-
matchUrl(appBase, this);
156+
parseAbsoluteUrl(appBase, this);
155157

156158

157159
/**
@@ -170,7 +172,7 @@ function LocationHashbangUrl(appBase, hashPrefix) {
170172
if (!isString(withoutHashUrl)) {
171173
throw $locationMinErr('ihshprfx', 'Invalid url "{0}", missing hash prefix "{1}".', url, hashPrefix);
172174
}
173-
matchAppUrl(withoutHashUrl, this);
175+
parseAppUrl(withoutHashUrl, this);
174176
this.$$compose();
175177
};
176178

src/ng/sce.js

+5-5
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ function adjustMatchers(matchers) {
123123
* // The blacklist overrides the whitelist so the open redirect here is blocked.
124124
* $sceDelegateProvider.resourceUrlBlacklist([
125125
* 'http://myapp.example.com/clickThru**']);
126-
* });
126+
* });
127127
* </pre>
128128
*/
129129

@@ -199,8 +199,8 @@ function $SceDelegateProvider() {
199199
return resourceUrlBlacklist;
200200
};
201201

202-
this.$get = ['$log', '$document', '$injector', '$$urlUtils', function(
203-
$log, $document, $injector, $$urlUtils) {
202+
this.$get = ['$log', '$document', '$injector', function(
203+
$log, $document, $injector) {
204204

205205
var htmlSanitizer = function htmlSanitizer(html) {
206206
throw $sceMinErr('unsafe', 'Attempting to use an unsafe value in a safe context.');
@@ -213,15 +213,15 @@ function $SceDelegateProvider() {
213213

214214
function matchUrl(matcher, parsedUrl) {
215215
if (matcher === 'self') {
216-
return $$urlUtils.isSameOrigin(parsedUrl);
216+
return urlIsSameOrigin(parsedUrl);
217217
} else {
218218
// definitely a regex. See adjustMatchers()
219219
return !!matcher.exec(parsedUrl.href);
220220
}
221221
}
222222

223223
function isResourceUrlAllowedByPolicy(url) {
224-
var parsedUrl = $$urlUtils.resolve(url.toString(), true);
224+
var parsedUrl = urlResolve(url.toString());
225225
var i, n, allowed = false;
226226
// Ensure that at least one item from the whitelist allows this url.
227227
for (i = 0, n = resourceUrlWhitelist.length; i < n; i++) {

0 commit comments

Comments
 (0)