Don't treat pipe as descendant combinator

There is currently a bug where selectors such as 'div | .c' matches
a .c-descendant of div. This is because of a bug in the parser, where
the '|' is consumed as part of the failing branch of CSSSelectorParser::
ConsumeName.

This CL changes the behavior to look ahead at the token after the pipe
delimiter: if it's not an <ident> or '*', we consume nothing and leave
the '|' unconsumed. This causes subsequent parsing attempts on the
CSSParserTokenRange to fail.

We had one test (non-WPT) that tried to use selectors such as
tns|#testInsertRule, but this is actually not valid. (Sidenote: the
test failed in Firefox). Added '*' to make it valid.

Bug: 1052847
Change-Id: I51e6a67a5e44cf3c0fe9d9fc7abea3772d0f4d9b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2058814Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742115}

third_party/blink/renderer/core/css/parser/css_selector_parser.cc

@@ -405,15 +405,16 @@ bool CSSSelectorParser::ConsumeName(CSSParserTokenRange& range,
   if (range.Peek().GetType() != kDelimiterToken ||
       range.Peek().Delimiter() != '|')
     return true;
-  range.Consume();
 
   namespace_prefix =
       name == CSSSelector::UniversalSelectorAtom() ? g_star_atom : name;
-  const CSSParserToken& name_token = range.Consume();
-  if (name_token.GetType() == kIdentToken) {
-    name = name_token.Value().ToAtomicString();
-  } else if (name_token.GetType() == kDelimiterToken &&
-             name_token.Delimiter() == '*') {
+  if (range.Peek(1).GetType() == kIdentToken) {
+    range.Consume();
+    name = range.Consume().Value().ToAtomicString();
+  } else if (range.Peek(1).GetType() == kDelimiterToken &&
+             range.Peek(1).Delimiter() == '*') {
+    range.Consume();
+    range.Consume();
     name = CSSSelector::UniversalSelectorAtom();
   } else {
     name = g_null_atom;

third_party/blink/renderer/core/css/parser/css_selector_parser_test.cc

@@ -287,6 +287,23 @@ TEST(CSSSelectorParserTest, UnresolvedNamespacePrefix) {
   }
 }
 
+TEST(CSSSelectorParserTest, UnexpectedPipe) {
+  const char* test_cases[] = {"div | .c", "| div", " | div"};
+
+  auto* context = MakeGarbageCollected<CSSParserContext>(
+      kHTMLStandardMode, SecureContextMode::kInsecureContext);
+  auto* sheet = MakeGarbageCollected<StyleSheetContents>(context);
+
+  for (auto* test_case : test_cases) {
+    CSSTokenizer tokenizer(test_case);
+    const auto tokens = tokenizer.TokenizeToEOF();
+    CSSParserTokenRange range(tokens);
+    CSSSelectorList list =
+        CSSSelectorParser::ParseSelector(range, context, sheet);
+    EXPECT_FALSE(list.IsValid());
+  }
+}
+
 TEST(CSSSelectorParserTest, SerializedUniversal) {
   const char* test_cases[][2] = {
       {"*::-webkit-volume-slider", "::-webkit-volume-slider"},

third_party/blink/web_tests/cssom/insertrule-namespace-mapping.html

@@ -16,14 +16,14 @@ function assertColorGreen(id) {
 }
 
 test(function() {
-  style.sheet.insertRule('tns|#testInsertRule[tns|green] { color: green; }', style.sheet.cssRules.length);
+  style.sheet.insertRule('tns|*#testInsertRule[tns|green] { color: green; }', style.sheet.cssRules.length);
   assertColorGreen('testInsertRule');
 }, 'Selectors added to CSSStyleSheets via insertRule() should use the @namespace mapping');
 
 test(function() {
   var mediaRule = style.sheet.rules[1];
   console.assert(mediaRule instanceof CSSMediaRule); // CSSMediaRule inherits from the CSSGroupingRule interface.
-  mediaRule.insertRule('tns|#testMediaInsertRule[tns|green] { color: green; }', style.sheet.length);
+  mediaRule.insertRule('tns|*#testMediaInsertRule[tns|green] { color: green; }', style.sheet.length);
   assertColorGreen('testMediaInsertRule');
 }, 'Selectors added to CSSGroupingRules via insertRule() should use the @namespace mapping');
 </script>