CSS URL prefixer comma fix, % fix, and specs

core/util/src/main/scala/net/liftweb/util/CSSHelpers.scala

@@ -24,8 +24,10 @@ import scala.util.parsing.combinator._
 import common._
 import java.io._
 
+// FIXME This needs a capitalization update, but that may be impossible to do
+// FIXME without breaking code :/
+// @deprecated("Please use CssHelpers instead; we are unifying capitalization across Lift.", "3.0")
 object CSSHelpers extends ControlHelpers {
-
   /**
    * Adds a prefix to root relative paths in the url segments from the css content
    *
@@ -47,18 +49,26 @@ object CSSHelpers extends ControlHelpers {
       val str = res toString;
       (CSSParser(rootPrefix).fixCSS(str), str);
   }
+}
 
-
+object CSSParser {
+  @deprecated("Please use CssUrlPrefixer instead; we are unifying capitalization across Lift.", "3.0")
+  def apply(prefix: String) = CssUrlPrefixer(prefix)
 }
 
 /**
- * Combinator parser for prefixing root relative paths with a given prefix
+ * Utility for prefixing root-relative `url`s in CSS with a given prefix.
+ * Typically used to prefix root-relative CSS `url`s with the application
+ * context path.
+ *
+ * After creating the prefixer with the prefix you want to apply to
+ * root-relative paths, call `fixCss` with a CSS string to return a fixed CSS
+ * string.
  */
-case class CSSParser(prefix: String) extends Parsers  {
+case class CssUrlPrefixer(prefix: String) extends Parsers  {
   implicit def strToInput(in: String): Input = new scala.util.parsing.input.CharArrayReader(in.toCharArray)
   type Elem = Char
 
-
  lazy val contentParser = Parser[String] {
     case in =>
       val content = new StringBuilder;
@@ -92,38 +102,90 @@ case class CSSParser(prefix: String) extends Parsers  {
 
 
   lazy val spaces = (elem(' ') | elem('\t') | elem('\n') | elem('\r')).*
+
+  def pathWith(additionalCharacters: Char*) = {
+    elem("path",
+      c => c.isLetterOrDigit ||
+           c == '?' || c == '/' ||
+           c == '&' || c == '@' ||
+           c == ';' || c == '.' ||
+           c == '+' || c == '-' ||
+           c == '=' || c == ':' ||
+           c == ' ' || c == '_' ||
+           c == '#' || c == ',' ||
+           c == '%' || additionalCharacters.contains(c)
+    ).+ ^^ {
+      case l =>
+        l.mkString("")
+    }
+  }
+
   // consider only root relative paths that start with /
-  lazy val path = elem("path", c => c.isLetterOrDigit ||
-                         c == '?' || c == '/' ||
-                         c == '&' || c == '@' ||
-                         c == ';' || c == '.' ||
-                         c == '+' || c == '-' ||
-                         c == '=' || c == ':' ||
-                         c == ' ' || c == '_' ||
-                         c == '#').+ ^^ {case l => l.mkString("")}
+  lazy val path = pathWith()
+
+  def fullUrl(innerUrl: Parser[String], quoteString: String): Parser[String] = {
+    val escapedPrefix =
+      if (quoteString.isEmpty) {
+        prefix
+      } else {
+        prefix.replace(quoteString, "\\" + quoteString)
+      }
+
+    // do the parsing per CSS spec http://www.w3.org/TR/REC-CSS2/syndata.html#uri section 4.3.4
+    spaces ~> innerUrl <~ (spaces <~ elem(')')) ^^ {
+      case urlPath => {
+        val trimmedPath = urlPath.trim
+
+        val updatedPath =
+          if (trimmedPath.charAt(0) == '/') {
+            escapedPrefix + trimmedPath
+          } else {
+            trimmedPath
+          }
+
+        quoteString + updatedPath + quoteString + ")"
+      }
+    }
+  }
 
   // the URL might be wrapped in simple quotes
-  lazy val seq1 = elem('\'') ~> path <~ elem('\'')
+  lazy val singleQuotedPath = fullUrl(elem('\'') ~> pathWith('"') <~ elem('\''), "'")
   // the URL might be wrapped in double quotes
-  lazy val seq2 = elem('\"') ~> path <~ elem('\"')
-  // do the parsing per CSS spec http://www.w3.org/TR/REC-CSS2/syndata.html#uri section 4.3.4
-  lazy val expr = spaces ~> ( seq1 | seq2 | path ) <~ (spaces <~ elem(')')) ^^ {case s => {
-      "'" + (s.trim.startsWith("/") match {
-        case true => prefix + s.trim
-        case _ => s.trim
-      }) + "')"
+  lazy val doubleQuotedPath = fullUrl(elem('\"') ~> pathWith('\'') <~ elem('\"'), "\"")
+  // the URL might not be wrapped at all
+  lazy val quotelessPath = fullUrl(path, "")
+
+  lazy val phrase =
+    (((contentParser ~ singleQuotedPath) |||
+      (contentParser ~ doubleQuotedPath) |||
+      (contentParser ~ quotelessPath)).* ^^ {
+      case l =>
+        l.flatMap(f => f._1 + f._2).mkString("")
+    }) ~ contentParser ^^ {
+      case a ~ b =>
+        a + b
     }
-  }
 
-  lazy val phrase = ((contentParser ~ expr).* ^^ {case l => l.flatMap(f => f._1 + f._2).mkString("")}) ~ contentParser ^^ {case a ~ b => a + b}
+  def fixCss(cssString: String): Box[String] = {
+    phrase(cssString) match {
+      case Success(updatedCss, remaining) if remaining.atEnd =>
+        Full(updatedCss)
 
-  def fixCSS(in: String): Box[String] = phrase(in) match {
-    case Success(v, r) => (r atEnd) match {
-      case true => Full(v)
-      case _ => common.Failure("Parser did not consume all input.  Parser error?") // return Failure if the reader is not at end as it implies that parsing ended due to a parser error
-      }
-    case x => common.Failure("Parse failed with result %s".format(x))
+      case Success(_, remaining) =>
+        val remainingString =
+          remaining.source.subSequence(
+            remaining.offset,
+            remaining.source.length
+          ).toString
+
+        common.Failure(s"Parser did not consume all input. Parser error? Unconsumed:\n$remainingString")
+
+      case failure =>
+        common.Failure(s"Parse failed with result $failure") ~> failure
+    }
   }
 
+  @deprecated("Please use fixCss instead; we are unifying capitalization across Lift.", "3.0")
+  def fixCSS(in: String): Box[String] = fixCss(in)
 }
 

core/util/src/test/scala/net/liftweb/util/CssHelpersSpec.scala

@@ -0,0 +1,101 @@
+/*
+ * Copyright 2007-2015 WorldWide Conferencing, LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.liftweb
+package util
+
+import xml._
+
+import org.specs2.mutable.Specification
+
+import common._
+
+class CssHelpersSpec extends Specification {
+  import CSSHelpers._
+
+  "CSSParser" should {
+    "leave most CSS alone" in {
+      val baseCss = 
+      """
+      #booyan {
+        text-indent: 1em;
+        -moz-columns: 3;
+        -webkit-text-antialiasing: grayscale;
+        -magical-fake-thing: booyan;
+        superfake: but-still-reasonably-css-y;
+      }
+      """
+
+      CssUrlPrefixer("prefix").fixCss(baseCss) must_== Full(baseCss)
+    }
+
+    "leave relative CSS urls alone" in {
+      val baseCss = 
+      """
+      #booyan {
+        background: url(boom);
+        background-image: url('boom?bam,sloop#"shap%20bap');
+        image-set: url("http://boom.com/magic?'bam,sloop#bam%21bap")
+      }
+
+      .bam {
+        background-image: url("boom?bam,sloop#shap%20bap");
+      }
+      """
+
+      CssUrlPrefixer("prefix").fixCss(baseCss) must_== Full(baseCss)
+    }
+
+    "prefix root-relative CSS urls with the specified prefix" in {
+      val baseCss = 
+      """
+      |#booyan {
+      |  background: url(/boom);
+      |  background-image: url('/boom?bam,"sloop#shap%20bap');
+      |  image-set: url("/boom.com/magic?bam,'sloop#bam%21bap")
+      |}""".stripMargin('|')
+
+      CssUrlPrefixer("prefix").fixCss(baseCss) must_==
+        Full(
+          """
+          |#booyan {
+          |  background: url(prefix/boom);
+          |  background-image: url('prefix/boom?bam,"sloop#shap%20bap');
+          |  image-set: url("prefix/boom.com/magic?bam,'sloop#bam%21bap")
+          |}""".stripMargin('|')
+        )
+    }
+
+    "fail on mismatched quotes or parens and report where it failed" in {
+      CssUrlPrefixer("prefix").fixCss("#boom { url('ha) }") must beLike {
+        case Failure(message, _, _) =>
+          message must contain("'ha")
+      }
+
+      CssUrlPrefixer("prefix").fixCss("#boom { url(\"ha) }") must beLike {
+        case Failure(message, _, _) =>
+          message must contain("\"ha")
+      }
+
+      CssUrlPrefixer("prefix").fixCss("#boom { url('ha' }") must beLike {
+        case Failure(message, _, _) =>
+          message must contain("ha' }")
+      }
+    }
+
+    // Escaped quotes-in-quotes currently fail. Maybe we want to support these?
+  }
+}