From 224dc91792738caddc9e44dccb87568a5c952ede Mon Sep 17 00:00:00 2001
From: Natalie Weizenbaum <nweiz@google.com>
Date: Thu, 31 May 2018 20:39:33 -0400
Subject: [PATCH] Forbid empty custom properties (#343)

---
 CHANGELOG.md                        | 3 +++
 lib/src/parse/parser.dart           | 5 ++++-
 lib/src/parse/selector.dart         | 4 ++--
 lib/src/parse/stylesheet.dart       | 8 ++++++--
 lib/src/visitor/async_evaluate.dart | 3 +++
 lib/src/visitor/evaluate.dart       | 5 ++++-
 6 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 77f66fa3..669c8860 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,9 @@
 * Fix a bug where an absolute Windows path would be considered an `input:output`
   pair.
 
+* Forbid custom properties that have no values, like `--foo:;`, since they're
+  forbidden by the CSS spec.
+
 ## 1.5.0
 
 * Fix a bug where an importer would be passed an incorrectly-resolved URL when
diff --git a/lib/src/parse/parser.dart b/lib/src/parse/parser.dart
index f98c0762..7aeaa3d6 100644
--- a/lib/src/parse/parser.dart
+++ b/lib/src/parse/parser.dart
@@ -205,8 +205,10 @@ abstract class Parser {
 
   /// Consumes tokens until it reaches a top-level `":"`, `")"`, `"]"`,
   /// or `"}"` and returns their contents as a string.
+  ///
+  /// If [allowEmpty] is `false` (the default), this requires at least one token.
   @protected
-  String declarationValue() {
+  String declarationValue({bool allowEmpty: false}) {
     // NOTE: this logic is largely duplicated in
     // StylesheetParser._interpolatedDeclarationValue. Most changes here should
     // be mirrored there.
@@ -301,6 +303,7 @@ abstract class Parser {
     }
 
     if (brackets.isNotEmpty) scanner.expectChar(brackets.last);
+    if (!allowEmpty && buffer.isEmpty) scanner.error("Expected token.");
     return buffer.toString();
   }
 
diff --git a/lib/src/parse/selector.dart b/lib/src/parse/selector.dart
index eaba51d0..02d97eac 100644
--- a/lib/src/parse/selector.dart
+++ b/lib/src/parse/selector.dart
@@ -282,7 +282,7 @@ class SelectorParser extends Parser {
       if (_selectorPseudoElements.contains(unvendored)) {
         selector = _selectorList();
       } else {
-        argument = declarationValue();
+        argument = declarationValue(allowEmpty: true);
       }
     } else if (_selectorPseudoClasses.contains(unvendored)) {
       selector = _selectorList();
@@ -297,7 +297,7 @@ class SelectorParser extends Parser {
         selector = _selectorList();
       }
     } else {
-      argument = declarationValue().trimRight();
+      argument = declarationValue(allowEmpty: true).trimRight();
     }
     scanner.expectChar($rparen);
 
diff --git a/lib/src/parse/stylesheet.dart b/lib/src/parse/stylesheet.dart
index 9c05286a..c77738fb 100644
--- a/lib/src/parse/stylesheet.dart
+++ b/lib/src/parse/stylesheet.dart
@@ -2119,7 +2119,8 @@ abstract class StylesheetParser extends Parser {
         return null;
     }
 
-    buffer.addInterpolation(_interpolatedDeclarationValue().text);
+    buffer
+        .addInterpolation(_interpolatedDeclarationValue(allowEmpty: true).text);
     scanner.expectChar($rparen);
     buffer.writeCharCode($rparen);
 
@@ -2331,8 +2332,10 @@ abstract class StylesheetParser extends Parser {
   /// Consumes tokens until it reaches a top-level `":"`, `")"`, `"]"`,
   /// or `"}"` and returns their contents as a string.
   ///
+  /// If [allowEmpty] is `false` (the default), this requires at least one token.
+  ///
   /// Unlike [declarationValue], this allows interpolation.
-  StringExpression _interpolatedDeclarationValue() {
+  StringExpression _interpolatedDeclarationValue({bool allowEmpty: false}) {
     // NOTE: this logic is largely duplicated in Parser.declarationValue. Most
     // changes here should be mirrored there.
 
@@ -2449,6 +2452,7 @@ abstract class StylesheetParser extends Parser {
     }
 
     if (brackets.isNotEmpty) scanner.expectChar(brackets.last);
+    if (!allowEmpty && buffer.isEmpty) scanner.error("Expected token.");
     return new StringExpression(buffer.interpolation(scanner.spanFrom(start)));
   }
 
diff --git a/lib/src/visitor/async_evaluate.dart b/lib/src/visitor/async_evaluate.dart
index 1ee32ab4..89e0b0cc 100644
--- a/lib/src/visitor/async_evaluate.dart
+++ b/lib/src/visitor/async_evaluate.dart
@@ -515,6 +515,9 @@ class _EvaluateVisitor
         (!cssValue.value.isBlank || _isEmptyList(cssValue.value))) {
       _parent.addChild(new CssDeclaration(name, cssValue, node.span,
           valueSpanForMap: _expressionSpan(node.value)));
+    } else if (name.value.startsWith('--')) {
+      throw _exception(
+          "Custom property values may not be empty.", node.value.span);
     }
 
     if (node.children != null) {
diff --git a/lib/src/visitor/evaluate.dart b/lib/src/visitor/evaluate.dart
index adb1b78e..c1b7719d 100644
--- a/lib/src/visitor/evaluate.dart
+++ b/lib/src/visitor/evaluate.dart
@@ -5,7 +5,7 @@
 // DO NOT EDIT. This file was generated from async_evaluate.dart.
 // See tool/synchronize.dart for details.
 //
-// Checksum: ff6c50c58c86529e7fed4247a7381649cec1900c
+// Checksum: bb333ed7c02c663af1977b3eb6c666b7bed92c1a
 
 import 'async_evaluate.dart' show EvaluateResult;
 export 'async_evaluate.dart' show EvaluateResult;
@@ -518,6 +518,9 @@ class _EvaluateVisitor
         (!cssValue.value.isBlank || _isEmptyList(cssValue.value))) {
       _parent.addChild(new CssDeclaration(name, cssValue, node.span,
           valueSpanForMap: _expressionSpan(node.value)));
+    } else if (name.value.startsWith('--')) {
+      throw _exception(
+          "Custom property values may not be empty.", node.value.span);
     }
 
     if (node.children != null) {