Skip to content

Reject alignment prefixes without a keyword instead of crashing - #280

Open
eXpl0it3r wants to merge 1 commit into
masterfrom
fix/alignment-value-crash
Open

eXpl0it3r wants to merge 1 commit into
masterfrom
fix/alignment-value-crash

Conversation

@eXpl0it3r

Copy link
Copy Markdown
Collaborator

A safe, unsafe, first or last prefix with nothing valid after it, e.g. align-items: safe or justify-content: first baseline, throws a NullReferenceException in ConditionalStartsWithValueConverter, the cascade fails and the document renders empty. The same happens for some place-* values like place-self: center left or place-content: baseline, because the shorthand converter tries to parse an empty token list when the second value doesn't match. The converter read enumerator.Current after MoveNext() had already returned false, and it wrapped a null result when the keyword after the prefix didn't parse. Both cases now return null, so the declaration gets dropped as invalid and the rest of the document renders.

Note that some of these values are valid per CSS Box Alignment 3 (safe start, place-self: center left, place-items: start legacy, ...), but they still get rejected for now. Accepting the full grammar is a separate change.

How to test this PR?

CI should succeed. AlignmentPrefixWithoutKeywordIsRejected covers the lone prefixes and AlignmentValueDoesntBreakTheDocument checks that the document still renders with these values, both fail on master.

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