Bug 265420 – Parsing stop-color with invalid values (i.e., Numbers "1234")

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Ahmad Saleem]

Hi Team,

While going through Browser Specific Failures, I came across one lone failing tests in SVG, so I thought to raise bugs since I couldn't find any other.

WPT Test Case: https://wpt.fyi/results/svg/pservers/parsing/stop-color-invalid.svg?label=master&label=experimental&aligned=&q=safari%3Afail

WPT Test Case Live Link: http://wpt.live/svg/pservers/parsing/stop-color-invalid.svg

Just wanted to raise so we can fix it.

Thanks!

[Radar WebKit Bug Importer]

<rdar://problem/119166640>

[Kiet Ho]

Drive-by comment as I happen to stumble upon this.

The test case sets "stop-color" to "123" and expects it to fail, but got "rgb(17, 34, 51)" instead.
tldr: we're interpreting this as a 3 digit hex string and interpret it as "#112233" or "rgb(17, 34, 51)"

This value is parsed by the fast path CSS parser (CSSParserFastPaths::maybeParseValue), which calls the fast path color parser routine (parseColor). It then parses it as a numeric color (parseNumericColor). This SVG document is parsed in HTMLQuirksMode, so not strict, and this code runs:

    if (!strict && (characters.size() == 3 || characters.size() == 6)) {
        if (auto hexColor = parseHexColorInternal(characters))
            return *hexColor;
    }

which parses "123". I'd imagine if we're not doing quirks parsing, then "123" would not be accepted.

Stack trace from the beginning of the JS call, if it helps.
  * frame #0: 0x0000000304800d5c WebCore`WebCore::finishParsingHexColor(value=291, length=3) at CSSParserFastPaths.cpp:468:16
    frame #1: 0x00000003047dc7b0 WebCore`std::__1::optional<WebCore::BoundedGammaEncoded<unsigned char, WebCore::SRGBADescriptor>> WebCore::parseHexColorInternal<unsigned char>(characters=size=3) at CSSParserFastPaths.cpp:503:12
    frame #2: 0x00000003047f4c90 WebCore`std::__1::optional<WebCore::BoundedGammaEncoded<unsigned char, WebCore::SRGBADescriptor>> WebCore::parseNumericColor<unsigned char>(characters=size=3, strict=false) at CSSParserFastPaths.cpp:610:29
    frame #3: 0x00000003047f4b10 WebCore`WebCore::parseNumericColor(string={ length = 3, contents = '123' }, context=0x000000016b359968) at CSSParserFastPaths.cpp:669:16
    frame #4: 0x00000003047dd320 WebCore`WebCore::parseColor(string={ length = 3, contents = '123' }, context=0x000000016b359968) at CSSParserFastPaths.cpp:682:22
    frame #5: 0x00000003047da084 WebCore`WebCore::CSSParserFastPaths::maybeParseValue(propertyID=CSSPropertyStopColor, string={ length = 3, contents = '123' }, context=0x000000016b359968) at CSSParserFastPaths.cpp:1079:16
    frame #6: 0x00000003047da274 WebCore`WebCore::CSSParser::parseValue(declaration=0x0000000118395800, propertyID=CSSPropertyStopColor, string={ length = 3, contents = '123' }, important=No, context=0x000000016b359968) at CSSParser.cpp:131:24
    frame #7: 0x00000003045e5284 WebCore`WebCore::MutableStyleProperties::setProperty(this=0x0000000118395800, propertyID=CSSPropertyStopColor, value={ length = 3, contents = '123' }, parserContext=CSSParserContext @ 0x000000016b359968, important=No, didFailParsing=0x0000000000000000) at MutableStyleProperties.cpp:155:24
    frame #8: 0x00000003045e7ecc WebCore`WebCore::PropertySetCSSStyleDeclaration::setPropertyInternal(this=0x0000000142008bb0, propertyID=CSSPropertyStopColor, value={ length = 3, contents = '123' }, important=No) at PropertySetCSSStyleDeclaration.cpp:230:24
    frame #9: 0x00000003044ba5f8 WebCore`WebCore::CSSStyleDeclaration::setPropertyValueForDashedIDLAttribute(this=0x0000000142008bb0, attribute={ length = 10, contents = 'stop-color' }, value={ length = 3, contents = '123' }) at CSSStyleDeclaration.cpp:277:12
    frame #10: 0x0000000300d77468 WebCore`WebCore::setJSCSSStyleDeclaration_propertyValueForDashedIDLAttributeSetter(this=0x000000016b359b70)::'lambda'()::operator()() const at JSCSSStyleDeclaration.cpp:3024:21
    frame #11: 0x0000000300d77398 WebCore`void WebCore::invokeFunctorPropagatingExceptionIfNecessary<WebCore::setJSCSSStyleDeclaration_propertyValueForDashedIDLAttributeSetter(JSC::JSGlobalObject&, WebCore::JSCSSStyleDeclaration&, JSC::JSValue, JSC::PropertyName)::'lambda'()>(lexicalGlobalObject=0x00000001191c8088, throwScope=0x000000016b359c28, functor=0x000000016b359b70) at JSDOMExceptionHandling.h:97:23
    frame #12: 0x0000000300d77324 WebCore`WebCore::setJSCSSStyleDeclaration_propertyValueForDashedIDLAttributeSetter(lexicalGlobalObject=0x00000001191c8088, thisObject=0x00000001185ce308, value=JSValue @ 0x000000016b359c80, propertyName=PropertyName @ 0x000000016b359c78) at JSCSSStyleDeclaration.cpp:3023:5
    frame #13: 0x0000000300ca3bb0 WebCore`bool WebCore::IDLAttribute<WebCore::JSCSSStyleDeclaration>::setPassingPropertyName<&WebCore::setJSCSSStyleDeclaration_propertyValueForDashedIDLAttributeSetter(JSC::JSGlobalObject&, WebCore::JSCSSStyleDeclaration&, JSC::JSValue, JSC::PropertyName), (WebCore::CastedThisErrorBehavior)0>(lexicalGlobalObject=0x00000001191c8088, thisValue=4703707912, encodedValue=4715458448, attributeName=PropertyName @ 0x000000016b359d80) at JSDOMAttribute.h:72:9
    frame #14: 0x0000000300ca3a84 WebCore`WebCore::setJSCSSStyleDeclaration_propertyValueForDashedIDLAttribute(lexicalGlobalObject=0x00000001191c8088, thisValue=4703707912, encodedValue=4715458448, attributeName=PropertyName @ 0x000000016b359dc8) at JSCSSStyleDeclaration.cpp:3031:12
    frame #15: 0x0000000133386208 JavaScriptCore`WTF::FunctionPtr<(WTF::PtrTag)28258, bool (JSC::JSGlobalObject*, long long, long long, JSC::PropertyName), (WTF::FunctionAttributes)1>::operator()(this=0x000000016b359ff0, in=0x00000001191c8088, in=4703707912, in=4715458448, in=PropertyName @ 0x000000016b359e18) const at FunctionPtr.h:114:16
    frame #16: 0x00000001334a5e00 JavaScriptCore`JSC::JSObject::putInlineSlow(this=0x00000001185ce308, globalObject=0x00000001191c8088, propertyName=PropertyName @ 0x000000016b35a0c0, value=JSValue @ 0x000000016b35a0b8, slot=0x000000016b35a590) at JSObject.cpp:867:17
    frame #17: 0x00000001329f8944 JavaScriptCore`JSC::JSObject::putInlineForJSObject(cell=0x00000001185ce308, globalObject=0x00000001191c8088, propertyName=PropertyName @ 0x000000016b35a1f0, value=JSValue @ 0x000000016b35a1e8, slot=0x000000016b35a590) at JSObjectInlines.h:317:28
    frame #18: 0x00000001334a0f9c JavaScriptCore`JSC::JSObject::put(cell=0x00000001185ce308, globalObject=0x00000001191c8088, propertyName=PropertyName @ 0x000000016b35a258, value=JSValue @ 0x000000016b35a250, slot=0x000000016b35a590) at JSObject.cpp:805:12
    frame #19: 0x0000000300ca1200 WebCore`WebCore::JSCSSStyleDeclaration::put(cell=0x00000001185ce308, lexicalGlobalObject=0x00000001191c8088, propertyName=PropertyName @ 0x000000016b35a460, value=JSValue @ 0x000000016b35a458, putPropertySlot=0x000000016b35a590) at JSCSSStyleDeclaration.cpp:2841:9
    frame #20: 0x000000013292e4d8 JavaScriptCore`JSC::JSValue::put(this=0x000000016b35a658, globalObject=0x00000001191c8088, propertyName=PropertyName @ 0x000000016b35a4f0, value=JSValue @ 0x000000016b35a4e8, slot=0x000000016b35a590) at JSCJSValueInlines.h:1184:12
    frame #21: 0x0000000133062308 JavaScriptCore`llint_slow_path_put_by_val(callFrame=0x000000016b35a7d0, pc=0x0000000118187c09) at LLIntSlowPaths.cpp:1368:15
    frame #22: 0x0000000133f82330 JavaScriptCore`jsc_llint_begin + 86320

[Ahmad Saleem]

Thanks @Kiet for pointer, I just got some time to look back into it and this fixes the test case:

if ((isQuirksModeBehavior(context.mode) && isUnitlessValueParsingForcedForMode(context.mode)) && (characters.size() == 3 || characters.size() == 6)) {
        if (auto hexColor = parseHexColorInternal(characters))
            return *hexColor;
    }

Can do draft PR to see, if it leads to any other failures or not but at least, this test is fixed.

[Ahmad Saleem]

Pull request: https://github.com/WebKit/WebKit/pull/46192

[Rob Buis]

Pull request: https://github.com/WebKit/WebKit/pull/51073

[EWS]

Committed 300296@main (dc1800749842): <https://commits.webkit.org/300296@main>

Reviewed commits have been landed. Closing PR #51073 and removing active labels.