Skip to content

Fix case-sensitive matching of and/or and from/to keywords in CSS parsers - #240

Draft
meziantou wants to merge 2 commits into
AngleSharp:develfrom
meziantou:feature/css-keyword-case-sensitivity-c30828
Draft

Fix case-sensitive matching of and/or and from/to keywords in CSS parsers#240
meziantou wants to merge 2 commits into
AngleSharp:develfrom
meziantou:feature/css-keyword-case-sensitivity-c30828

Conversation

@meziantou

Copy link
Copy Markdown
Contributor

Types of Changes

Prerequisites

Please make sure you can check the following two boxes:

  • I have read the CONTRIBUTING document
  • My code follows the code style of this project

Contribution Type

What types of changes does your code introduce? Put an x in all the boxes that apply:

  • Bug fix (non-breaking change which fixes an issue, please reference the issue id)
  • New feature (non-breaking change which adds functionality, make sure to open an associated issue first)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

Description

CSS keywords are ASCII case-insensitive, but two micro parsers compared them with ordinal equality.

ConditionParser used .Is(...) for the and / or keywords in @supports conditions. Negation on the line just above already used the case-insensitive .Isi(...) for not, which is what makes the divergence visible: @supports NOT (...) worked, while @supports (color: red) AND (display: flex) { ... } parsed to zero rules, silently discarding the conditional group and every rule inside it with no parse error.

A third comparison in Scan had the same problem. It compares each subsequent keyword against the raw text of the first one, so a chain like (a) And (b) aND (c) still truncated after the second group even once the first two comparisons were fixed.

KeyframeParser used .Is(...) for from / to. @keyframes x { FROM {...} TO {...} } failed selector parsing, leaving CssKeyframeRule._selector null. The rule stays in the CSSOM with a null KeyText, so the animation endpoints vanish and the round-tripped CSS is invalid.

Changes

Switched all five comparisons to the case-insensitive Isi helper:

  • Parser/Micro/ConditionParser.csand / or in ConjunctionOrDisjunction, and the chain continuation in Scan
  • Parser/Micro/KeyframeParser.csfrom / to

Tests

7 new tests, each confirmed failing before the change and passing after:

  • Rules/CssSupports.cs — uppercase AND, uppercase OR, a mixed-case And / aND chain, and one asserting the inner rules of an uppercase-AND group survive
  • Rules/CssKeyframeRule.csFROM, TO, and mixed-case From, To

Full suite: 2094 passed, 0 failed.

Notes for reviewers

  • A grep over Parser/ confirms the only remaining ordinal .Is( is sheet.Href.Is(href) in CssParser.cs, a URL comparison that should stay case-sensitive.
  • Not included: a malformed keyframe selector still yields a rule with a null key rather than being rejected outright. That is a separate public CSSOM behavior change and no longer reachable through the uppercase path now that these parse. Happy to add it if you'd prefer the rule be dropped or the parse throw.

The @supports condition parser compared the "and" / "or" keywords with
ordinal equality, so an uppercase or mixed-case keyword silently
discarded the whole conditional group and every rule inside it. The
keyframe selector parser did the same for "from" / "to", leaving the
rule in the CSSOM with a null key.

Switched both to the case-insensitive Isi helper, including the chain
continuation in Scan, which compared each subsequent keyword against
the raw text of the first one.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant