fix(relative_url_resolver): Fix relative URI resolution for html.

Certain values used in href, src, and action attributes inside of HTML will be rewritten
incorrectly. Such as javascript: values, empty strings, #, and data: values. Add improved
calculation if URIs are relative or not.

Author: TedSander

lib/core_dom/resource_url_resolver.dart

@@ -8,7 +8,6 @@
 library angular.core_dom.resource_url_resolver;
 
 import 'dart:html';
-import 'dart:js' as js;
 
 import 'package:di/di.dart';
 import 'package:di/annotations.dart';
@@ -29,12 +28,14 @@ class ResourceUrlResolver {
   static final RegExp quotes = new RegExp("[\"\']");
 
   // Reconstruct the Uri without the http or https restriction due to Uri.base.origin
-  final _baseUri = _getBaseUri();
+  final String _baseUri;
 
   final TypeToUriMapper _uriMapper;
   final ResourceResolverConfig _config;
 
-  ResourceUrlResolver(this._uriMapper, this._config);
+  ResourceUrlResolver(this._uriMapper, this._config): _baseUri = _getBaseUri();
+
+  ResourceUrlResolver.forTests(this._uriMapper, this._config, this._baseUri);
 
   static final NodeTreeSanitizer _nullTreeSanitizer = new _NullTreeSanitizer();
   static final docForParsing = document.implementation.createHtmlDocument('');
@@ -139,34 +140,42 @@ class ResourceUrlResolver {
   /// URIs, while [uri] is assumed to use 'packages/' syntax for
   /// package-relative URIs. Resulting URIs will use 'packages/' to indicate
   /// package-relative URIs.
-  String combine(Uri baseUri, String uri) {
+  String combine(Uri baseUri, String path) {
     if (!_config.useRelativeUrls) {
-       return uri;
+       return path;
     }
 
-    if (uri == null) {
-      uri = baseUri.path;
+    Uri resolved;
+    if (path == null) {
+      resolved = baseUri;
     } else {
+      Uri uri = Uri.parse(path);
       // if it's absolute but not package-relative, then just use that
       // The "packages/" test is just for backward compatibility.  It's ok to
       // not resolve them, even through they're relative URLs, because in a Dart
       // application, "packages/" is managed by pub which creates a symlinked
       // hierarchy and they should all resolve to the same file at any level
       // that a "packages/" exists.
-      if (uri.startsWith("/") || uri.startsWith('packages/')) {
-        return uri;
+      if (uri.path.startsWith('/') ||
+          uri.path.startsWith('packages/') ||
+          uri.path.trim() == '' || // Covers both empty strings and # fragments
+          uri.isAbsolute) {
+        return _uriToPath(uri);
       }
+      // Not an absolute uri. Resolve it to the base.
+      resolved = baseUri.resolve(path);
     }
-    // If it's not absolute, then resolve it first
-    Uri resolved = baseUri.resolve(uri);
-
-    // If it's package-relative, tack on [packageRoot].
-    if (resolved.scheme == 'package') {
-      return '${_config.packageRoot}${resolved.path}';
-    } else if (resolved.isAbsolute && resolved.toString().startsWith(_baseUri)) {
-      return resolved.path;
-    } else {
-      return resolved.toString();
+
+    return _uriToPath(resolved);
+  }
+
+  String _uriToPath(Uri uri) {
+    if (uri.scheme == 'package') {
+      return '${_config.packageRoot}${uri.path}';
+    } else if (uri.isAbsolute && uri.toString().startsWith(_baseUri)) {
+      return uri.path;
+    }  else {
+      return uri.toString();
     }
   }
 

test/core_dom/resource_url_resolver_spec.dart

@@ -241,6 +241,60 @@ void main() {
     _run_resolver(useRelativeUrls: true);
     _run_resolver(useRelativeUrls: false);
   });
+
+  describe('url_resolver uri rewrite', () {
+    ResourceUrlResolver resolver;
+
+    beforeEach((){
+      var config = new ResourceResolverConfig.resolveRelativeUrls(true);
+      var typeMapper = new DynamicTypeToUriMapper();
+      resolver = new ResourceUrlResolver
+          .forTests(typeMapper, config, 'http://localhost');
+    });
+
+    it('should not rewrite absolute paths, empty paths and custom schemas',
+        () {
+      Uri baseUri = new Uri();
+      expect(resolver.combine(baseUri, "/test")).toEqual("/test");
+      expect(resolver.combine(baseUri, "#")).toEqual("#");
+      expect(resolver.combine(baseUri, "#abc")).toEqual("#abc");
+      expect(resolver.combine(baseUri, "")).toEqual("");
+      expect(resolver.combine(baseUri, "javascript:void()"))
+          .toEqual("javascript:void()");
+      expect(resolver.combine(baseUri, "data:uriloijoi"))
+          .toEqual("data:uriloijoi");
+    });
+
+    it('should rewrite package relative base uris', () {
+      Uri baseUri = Uri.parse("/packages/a.b.c/comp.dart");
+      expect(resolver.combine(baseUri, "test.jpg"))
+          .toEqual("/packages/a.b.c/test.jpg");
+      expect(resolver.combine(baseUri, "test/test.jpg"))
+          .toEqual("/packages/a.b.c/test/test.jpg");
+    });
+
+    it('should rewrite current base uri to absolute without scheme', () {
+      Uri baseUri = Uri.parse("http://localhost/test/test2/test.jgp");
+      expect(resolver.combine(baseUri, 'test4.jpg'))
+          .toEqual('/test/test2/test4.jpg');
+    });
+
+    it('should rewrite external URIs to contain full scheme', () {
+      Uri baseUri = Uri.parse("http://foo.com/test/test.jgp");
+      expect(resolver.combine(baseUri, 'test4.jpg'))
+          .toEqual('http://foo.com/test/test4.jpg');
+    });
+
+    it('should not remove hash from URI', () {
+      Uri baseUri = Uri.parse("/packages/a.b.c/comp.dart");
+      expect(resolver.combine(baseUri, "test.jpg#1234"))
+          .toEqual("/packages/a.b.c/test.jpg#1234");
+
+      Uri externalUri = Uri.parse("http://foo.com/test/test.jgp");
+      expect(resolver.combine(externalUri, 'test4.jpg#1234'))
+          .toEqual('http://foo.com/test/test4.jpg#1234');
+    });
+  });
 }
 
 class NullSanitizer implements NodeValidator {