Bug 252097

Summary: Enforce HTML range restriction on setting unsigned attribute values
Product: WebKit Reporter: Ahmad Saleem <ahmad.saleem792>
Component: DOMAssignee: Nobody <webkit-unassigned>
Status: RESOLVED INVALID    
Severity: Normal CC: annevk, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   

Description Ahmad Saleem 2023-02-10 17:18:24 PST 
Hi Team,

While going through Blink's commit, I came cross which can be good merge (if possible 1-1) since it will align with web-spec but just wanted to raise for tracking and any input purposes.

Blink Commit - https://chromium.googlesource.com/chromium/blink/+/68e5d6caed2c938e81653855d276ed5fdb964b47

Just wanted to raise so we can fix it.

Thanks!
Comment 1 Ahmad Saleem 2023-02-10 17:20:09 PST 
WebKit Source - https://searchfox.org/wubkat/source/Source/WebCore/dom/Element.cpp#4292
Comment 2 Ahmad Saleem 2023-02-10 17:23:23 PST 
We are using 'limitToOnlyHTMLNonNegative' to restrict it to restrict it to only positive but do we need upper clamping?

@rniwa - appreciate your input? Thanks!
Comment 3 Radar WebKit Bug Importer 2023-02-17 17:19:16 PST 
<rdar://problem/105619831>
Comment 4 Ahmad Saleem 2023-03-01 10:39:30 PST 
(In reply to Ahmad Saleem from comment #2)
> We are using 'limitToOnlyHTMLNonNegative' to restrict it to restrict it to
> only positive but do we need upper clamping?
> 
> @rniwa - appreciate your input? Thanks!

I think search fox link messed up - https://searchfox.org/wubkat/source/Source/WebCore/dom/Element.cpp#4323
Comment 5 Ahmad Saleem 2024-05-22 06:08:15 PDT 
We are already doing it:

void Element::setUnsignedIntegralAttribute(const QualifiedName& attributeName, unsigned value)
{
    setAttribute(attributeName, AtomString::number(limitToOnlyHTMLNonNegative(value)));
}

From this `limitToOnlyHTMLNonNegative`:

// https://html.spec.whatwg.org/#reflecting-content-attributes-in-idl-attributes:idl-unsigned-long
inline unsigned limitToOnlyHTMLNonNegative(unsigned value, unsigned defaultValue = 0)
{
    ASSERT(defaultValue <= maxHTMLNonNegativeInteger);
    return value <= maxHTMLNonNegativeInteger ? value : defaultValue;
}

and then `maxHTMLNonNegativeInteger` goes to:

static const unsigned maxHTMLNonNegativeInteger = 2147483647;

Blink proposed - 0x7fffffffu = 2147483647 (in Decimal).