Strip bindings before calling Uri.parse

pkg/polymer/lib/src/build/import_inliner.dart

Index: pkg/polymer/lib/src/build/import_inliner.dart
diff --git a/pkg/polymer/lib/src/build/import_inliner.dart b/pkg/polymer/lib/src/build/import_inliner.dart
index 675db2999d793603aebda86e8799a5b407a7ecd9..8824604fc1294ce665ce759ab264c5825d32b2d5 100644
--- a/pkg/polymer/lib/src/build/import_inliner.dart
+++ b/pkg/polymer/lib/src/build/import_inliner.dart
@@ -262,8 +262,8 @@ String _libraryNameFor(AssetId id, int suffix) {
       '${path.extension(id.path).substring(1)}';
   if (name.startsWith('lib/')) name = name.substring(4);
   name = name.split('/').map((part) {
-    part = part.replaceAll(INVALID_LIB_CHARS_REGEX, '_');
-    if (part.startsWith(NUM_REGEX)) part = '_${part}';
+    part = part.replaceAll(_INVALID_LIB_CHARS_REGEX, '_');
+    if (part.startsWith(_NUM_REGEX)) part = '_${part}';
     return part;
   }).join(".");
   return '${id.package}.${name}_$suffix';
@@ -337,7 +337,7 @@ class _UrlNormalizer extends TreeVisitor {
     if (!isCustomTagName(node.localName)) {
       node.attributes.forEach((name, value) {
         if (_urlAttributes.contains(name)) {
-          if (value != '' && !value.trim().startsWith(_BINDINGS)) {
+          if (value != '' && !value.trim().startsWith(_BINDINGS_REGEX)) {
             node.attributes[name] = _newUrl(value, node.sourceSpan);
             changed = changed || value != node.attributes[name];
           }
@@ -360,7 +360,6 @@ class _UrlNormalizer extends TreeVisitor {
 
   static final _URL = new RegExp(r'url\(([^)]*)\)', multiLine: true);
   static final _QUOTE = new RegExp('["\']', multiLine: true);
-  static final _BINDINGS = new RegExp(r'({{)|(\[\[)');
 
   /// Visit the CSS text and replace any relative URLs so we can inline it.
   // Ported from:
@@ -417,10 +416,12 @@ class _UrlNormalizer extends TreeVisitor {
   }
 
   String _newUrl(String href, Span span) {
-    // Uri.parse blows up on invalid characters (like {{). Encoding the uri
-    // allows it to be parsed, which does the correct thing in the general case.
-    // This uri not used to build the new uri, so it never needs to be decoded.
-    var uri = Uri.parse(Uri.encodeFull(href));
+    // Take all bindings out of the path and replace with [_BINDING_PLACEHOLDER]
+    // so [Uri.parse] will not fail.
+    var bindingMatches = _BINDING_REGEX.allMatches(href).map((m) => m.group(0));
+    href = href.replaceAll(_BINDING_REGEX, _BINDING_PLACEHOLDER);

[Siggi Cherem (dart-lang), 2014/07/31 21:54:22]

I wonder if we need to replace all, or if we should just find the first one. At
that point, we might be OK just splitting it and don't need a placeholder. For
example:

a/b/{{e}}/f/{{g}}/q.dart

->

prefix = a/b/
suffix = {{e}}/f/{{g}}/q.dart

Then later newPath = '${id.path}$suffix'

WDYT?

[jakemac, 2014/08/01 15:20:47]

This seems like a good idea, but I tried it out and there ends up being a lot of
weird edge cases. For instance, a path of just './' turns into '../web'. Also,
the trailing slashes always get removed, so if I just tack on other things I
have to add it back (but only if it had one before). Covering all of these cases
is going to overall make things more complicated unfortunately and I would
likely miss a few.

On 2014/07/31 21:54:22, Siggi Cherem (dart-lang) wrote:

[Siggi Cherem (dart-lang), 2014/08/01 16:02:51]

Interesting, here are a couple other ideas... it might be worth giving them a
shot.

You can do like you are doing with adding a %%BINDING%% but just for the first
one:


a/b/{{e}}/f/{{g}}/q.dart

->
prefix = a/b/%%BINDING%%
suffix = {{e}}/f/{{g}}/q.dart

what do you think?


If that doesn't work, I think we can still get rid of the loop that restores the
bindings (which would get rid of the buildingMatches) by just replacing the
suffix in the resulting path starting from the first binding position.

var uriSafeHref = href.replaceAll(_BINDING_REGEX, _BINDING_PLACEHOLDER);
...
var newPath = id.path;
var bindingIndex = newPath.indexOf(_BINDING_PLACEHOLDER);
if (bindingIndex >= 0) {
  var prefix = newPath.substring(0, bindingIndex);
  var suffix = href.substring(href.indexOf(_BINDING_REGEX));
  var newPath = '$prefix$suffix';
}

[jakemac, 2014/08/01 22:12:35]

Done.

+
+    var uri = Uri.parse(href);
     if (uri.isAbsolute) return href;

[Siggi Cherem (dart-lang), 2014/07/31 21:54:22]

shouldn't this return the original href without the placeholders?
(same below on most of the return statements)

[jakemac, 2014/08/01 15:20:47]

Good catch, I have updated it so I don't write over the original href varibale
any more, instead I am making a new one called uriSafeHref.

On 2014/07/31 21:54:22, Siggi Cherem (dart-lang) wrote:

     if (!uri.scheme.isEmpty) return href;
     if (!uri.host.isEmpty) return href;
@@ -431,12 +432,18 @@ class _UrlNormalizer extends TreeVisitor {
     if (id == null) return href;
     var primaryId = transform.primaryInput.id;
 
-    if (id.path.startsWith('lib/')) {
-      return '${topLevelPath}packages/${id.package}/${id.path.substring(4)}';
+    // Put the original bindings back into the path now that we have parsed it.
+    var newPath = id.path;
+    for (var bindingMatch in bindingMatches) {
+      newPath = newPath.replaceFirst(_BINDING_PLACEHOLDER, bindingMatch);
+    }
+
+    if (newPath.startsWith('lib/')) {
+      return '${topLevelPath}packages/${id.package}/${newPath.substring(4)}';
     }
 
-    if (id.path.startsWith('asset/')) {
-      return '${topLevelPath}assets/${id.package}/${id.path.substring(6)}';
+    if (newPath.startsWith('asset/')) {
+      return '${topLevelPath}assets/${id.package}/${newPath.substring(6)}';
     }
 
     if (primaryId.package != id.package) {
@@ -447,7 +454,7 @@ class _UrlNormalizer extends TreeVisitor {
     }
 
     var builder = path.url;
-    return builder.relative(builder.join('/', id.path),
+    return builder.relative(builder.join('/', newPath),
         from: builder.join('/', builder.dirname(primaryId.path)));
   }
 }
@@ -476,8 +483,12 @@ const _urlAttributes = const [
 const IGNORED_LINKED_STYLE_ATTRS =
     const ['charset', 'href', 'href-lang', 'rel', 'rev'];
 
-/// Global RegExp objects for validating generated library names.
-final INVALID_LIB_CHARS_REGEX = new RegExp('[^a-z0-9_]');
-final NUM_REGEX = new RegExp('[0-9]');
+/// Global RegExp objects.
+final _INVALID_LIB_CHARS_REGEX = new RegExp('[^a-z0-9_]');
+final _NUM_REGEX = new RegExp('[0-9]');
+final _BINDING_REGEX = new RegExp(r'(({{.*}})|(\[\[.*\]\]))');
+
+/// Placeholder for bindings in urls for [Uri.parse].
+final _BINDING_PLACEHOLDER = '%BINDING%';
 
 _getSpan(SourceFile file, AstNode node) => file.span(node.offset, node.end);