Bug 252097 - Enforce HTML range restriction on setting unsigned attribute values
Summary: Enforce HTML range restriction on setting unsigned attribute values
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2023-02-10 17:18 PST by Ahmad Saleem
Modified: 2024-05-22 06:08 PDT (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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).