Skip to content

Don't crash serializing an unknown rule with an unmatched closing brace - #80

Open
eeshsaxena wants to merge 1 commit into
jaraco:mainfrom
eeshsaxena:fix/serialize-unmatched-brace
Open

Don't crash serializing an unknown rule with an unmatched closing brace#80
eeshsaxena wants to merge 1 commit into
jaraco:mainfrom
eeshsaxena:fix/serialize-unmatched-brace

Conversation

@eeshsaxena

Copy link
Copy Markdown

Parsing then serializing certain malformed at-rules raises IndexError from the serializer:

import cssutils
cssutils.parseString('@keyframes k { 0% { opacity: 0; }"}').cssText
# IndexError: pop from empty list   (serialize.py, do_CSSUnknownRule)

do_CSSUnknownRule walks the rule's token sequence keeping a stack of open blocks and pops it on every }. A } with no matching { (which the tokenizer can leave in the sequence for a malformed at-rule) makes it pop from an empty list. I guard the pop with and stacks; a stray } then falls through and is emitted as a plain token. Well-formed unknown rules (including nested blocks) serialize exactly as before.

Added a test to TestCSSUnknownRule; it raises IndexError on main and passes with the change, and the unknown-rule and serialize suites still pass. Found it by fuzzing parseString + cssText.

do_CSSUnknownRule walks the rule's token sequence keeping a stack of open
blocks, popping on each '}'. A '}' with no matching '{' (from a malformed
at-rule) left the stack empty, so stacks.pop() raised IndexError while
computing cssText. Only pop when the stack is non-empty; a stray '}' is then
emitted as a plain token.
@eeshsaxena

Copy link
Copy Markdown
Author

Hi! Gentle nudge on this one whenever you have some bandwidth. It's a small, self-contained fix (Don't crash serializing an unknown rule with an unmatched closing brace), and it's currently mergeable with no conflicts. No urgency at all, and I'm happy to make any changes you'd like. Thanks for maintaining cssutils!

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