Emulate Ruby Sass' url() parsing semantics

We've had countless bugs and regressions with parsing url(). This patch is complete refactor of our url() parsing semantics to 100% match that of Ruby Sass. Fixes sass#674 Spec sass/sass-spec#539
xzyfer · Oct 13, 2015 · 2e15e46 · 2e15e46
1 parent fbb6fb6
commit 2e15e46
Show file tree

Hide file tree

Showing 8 changed files with 103 additions and 22 deletions.
diff --git a/src/constants.cpp b/src/constants.cpp
@@ -141,6 +141,7 @@ namespace Sass {
 
     // constants for uri parsing (RFC 3986 Appendix A.)
     extern const char uri_chars[]  = ":;/?!%&#@|[]{}'`^\"*+-.,_=~";
+    extern const char real_uri_chars[]  = "#%&";
 
     // some specific constant character classes
     // they must be static to be useable by lexer

diff --git a/src/constants.hpp b/src/constants.hpp
@@ -144,6 +144,7 @@ namespace Sass {
 
     // constants for uri parsing (RFC 3986 Appendix A.)
     extern const char uri_chars[];
+    extern const char real_uri_chars[];
 
     // some specific constant character classes
     // they must be static to be useable by lexer

diff --git a/src/lexer.cpp b/src/lexer.cpp
@@ -75,6 +75,24 @@ namespace Sass {
       return unsigned(chr) > 127;
     }
 
+    // check if char is outside ascii range
+    // but with specific ranges (copied from Ruby Sass)
+    bool is_nonascii(const char& chr)
+    {
+      return (
+        (unsigned(chr) > 127 && unsigned(chr) < 55296) ||
+        (unsigned(chr) > 57343 && unsigned(chr) < 65534) ||
+        (unsigned(chr) > 65535 && unsigned(chr) < 1114111)
+      );
+    }
+
+    // check if char is within a reduced ascii range
+    // valid in a uri (copied from Ruby Sass)
+    bool is_uri_character(const char& chr)
+    {
+      return unsigned(chr) > 41 && unsigned(chr) < 127;
+    }
+
     // Match word character (look ahead)
     bool is_character(const char& chr)
     {
@@ -90,11 +108,13 @@ namespace Sass {
     const char* space(const char* src) { return is_space(*src) ? src + 1 : 0; }
     const char* alpha(const char* src) { return is_alpha(*src) ? src + 1 : 0; }
     const char* unicode(const char* src) { return is_unicode(*src) ? src + 1 : 0; }
+    const char* nonascii(const char* src) { return is_nonascii(*src) ? src + 1 : 0; }
     const char* digit(const char* src) { return is_digit(*src) ? src + 1 : 0; }
     const char* xdigit(const char* src) { return is_xdigit(*src) ? src + 1 : 0; }
     const char* alnum(const char* src) { return is_alnum(*src) ? src + 1 : 0; }
     const char* punct(const char* src) { return is_punct(*src) ? src + 1 : 0; }
     const char* character(const char* src) { return is_character(*src) ? src + 1 : 0; }
+    const char* uri_character(const char* src) { return is_uri_character(*src) ? src + 1 : 0; }
 
     // Match multiple ctype characters.
     const char* spaces(const char* src) { return one_plus<space>(src); }

diff --git a/src/lexer.hpp b/src/lexer.hpp
@@ -32,7 +32,9 @@ namespace Sass {
     bool is_alnum(const char& src);
     bool is_xdigit(const char& src);
     bool is_unicode(const char& src);
+    bool is_nonascii(const char& src);
     bool is_character(const char& src);
+    bool is_uri_character(const char& src);
 
     // Match a single ctype predicate.
     const char* space(const char* src);
@@ -42,7 +44,9 @@ namespace Sass {
     const char* alnum(const char* src);
     const char* punct(const char* src);
     const char* unicode(const char* src);
+    const char* nonascii(const char* src);
     const char* character(const char* src);
+    const char* uri_character(const char* src);
 
     // Match multiple ctype characters.
     const char* spaces(const char* src);

diff --git a/src/parser.cpp b/src/parser.cpp
@@ -501,36 +501,31 @@ namespace Sass {
     return p;
   }
 
-  Arguments* Parser::parse_arguments(bool has_url)
+  Arguments* Parser::parse_arguments()
   {
     std::string name(lexed);
     Position position = after_token;
     Arguments* args = SASS_MEMORY_NEW(ctx.mem, Arguments, pstate);
     if (lex_css< exactly<'('> >()) {
       // if there's anything there at all
       if (!peek_css< exactly<')'> >()) {
-        do (*args) << parse_argument(has_url);
+        do (*args) << parse_argument();
         while (lex_css< exactly<','> >());
       }
       if (!lex_css< exactly<')'> >()) error("expected a variable name (e.g. $x) or ')' for the parameter list for " + name, position);
     }
     return args;
   }
 
-  Argument* Parser::parse_argument(bool has_url)
+  Argument* Parser::parse_argument()
   {
     if (peek_css< sequence < exactly< hash_lbrace >, exactly< rbrace > > >()) {
       position += 2;
       css_error("Invalid CSS", " after ", ": expected expression (e.g. 1px, bold), was ");
     }
 
     Argument* arg;
-    // some urls can look like line comments (parse literally - chunk would not work)
-    if (has_url && lex< sequence < uri_value, lookahead < loosely<')'> > > >(false)) {
-      String* the_url = parse_interpolated_chunk(lexed);
-      arg = SASS_MEMORY_NEW(ctx.mem, Argument, the_url->pstate(), the_url);
-    }
-    else if (peek_css< sequence < variable, optional_css_comments, exactly<':'> > >()) {
+    if (peek_css< sequence < variable, optional_css_comments, exactly<':'> > >()) {
       lex_css< variable >();
       std::string name(Util::normalize_underscores(lexed));
       ParserState p = pstate;
@@ -1410,6 +1405,9 @@ namespace Sass {
       }
       return string;
     }
+    else if (peek< real_uri_value >()) {
+      return parse_url_function_string();
+    }
     else if (peek< re_functional >()) {
       return parse_function_call();
     }
@@ -1790,14 +1788,19 @@ namespace Sass {
     return SASS_MEMORY_NEW(ctx.mem, Function_Call, call_pos, name, args);
   }
 
+  String* Parser::parse_url_function_string()
+  {
+    lex< real_uri_value >();
+    return SASS_MEMORY_NEW(ctx.mem, String_Constant, pstate, lexed);
+  }
+
   Function_Call* Parser::parse_function_call()
   {
     lex< identifier >();
     std::string name(lexed);
 
     ParserState call_pos = pstate;
-    bool expect_url = name == "url" || name == "url-prefix";
-    Arguments* args = parse_arguments(expect_url);
+    Arguments* args = parse_arguments();
     return SASS_MEMORY_NEW(ctx.mem, Function_Call, call_pos, name, args);
   }
 

diff --git a/src/parser.hpp b/src/parser.hpp
@@ -220,8 +220,8 @@ namespace Sass {
     Parameters* parse_parameters();
     Parameter* parse_parameter();
     Mixin_Call* parse_include_directive();
-    Arguments* parse_arguments(bool has_url = false);
-    Argument* parse_argument(bool has_url = false);
+    Arguments* parse_arguments();
+    Argument* parse_argument();
     Assignment* parse_assignment();
     // Propset* parse_propset();
     Ruleset* parse_ruleset(Lookahead lookahead, bool is_root = false);
@@ -256,6 +256,7 @@ namespace Sass {
     Function_Call* parse_calc_function();
     Function_Call* parse_function_call();
     Function_Call_Schema* parse_function_call_schema();
+    String* parse_url_function_string();
     String* parse_interpolated_chunk(Token, bool constant = false);
     String* parse_string();
     String_Constant* parse_static_expression();

diff --git a/src/prelexer.cpp b/src/prelexer.cpp
@@ -914,22 +914,64 @@ namespace Sass {
                            exactly<'\f'> >(src);
     }*/
 
-    /* not used anymore - remove?
     const char* H(const char* src) {
       return std::isxdigit(*src) ? src+1 : 0;
-    }*/
+    }
 
-    /* not used anymore - remove?
-    const char* unicode(const char* src) {
+    const char* W(const char* src) {
+      return zero_plus< alternatives<
+        space,
+        exactly< '\t' >,
+        exactly< '\r' >,
+        exactly< '\n' >,
+        exactly< '\f' >
+      > >(src);
+    }
+
+    const char* UUNICODE(const char* src) {
       return sequence< exactly<'\\'>,
                        between<H, 1, 6>,
-                       optional< class_char<url_space_chars> > >(src);
-    }*/
+                       optional< W >
+                       >(src);
+    }
+
+    const char* NONASCII(const char* src) {
+      return nonascii(src);
+    }
 
-    /* not used anymore - remove?
     const char* ESCAPE(const char* src) {
-      return alternatives< unicode, class_char<escape_chars> >(src);
-    }*/
+      return alternatives<
+        UUNICODE,
+        sequence<
+          exactly<'\\'>,
+          NONASCII,
+          class_char< escape_chars >
+        >
+      >(src);
+    }
+
+
+    const char* real_uri_value(const char* src) {
+      return
+      sequence<
+        exactly< url_kwd >,
+        W,
+        zero_plus< alternatives<
+          class_char< real_uri_chars >,
+          uri_character,
+          NONASCII,
+          ESCAPE
+        > >,
+        alternatives<
+          sequence<
+            W,
+            exactly< ')' >
+          >,
+          exactly< hash_lbrace >
+        >
+      >
+      (src);
+    }
 
     const char* static_string(const char* src) {
       const char* pos = src;

diff --git a/src/prelexer.hpp b/src/prelexer.hpp
@@ -340,6 +340,15 @@ namespace Sass {
     // match urls
     const char* url(const char* src);
 
+    // match url()
+    const char* H(const char* src);
+    const char* W(const char* src);
+    // `UNICODE` makes VS sad
+    const char* UUNICODE(const char* src);
+    const char* NONASCII(const char* src);
+    const char* ESCAPE(const char* src);
+    const char* real_uri_value(const char* src);
+
     // Path matching functions.
     // const char* folder(const char* src);
     // const char* folders(const char* src);