Bug 227779 - Our structured cloning implementation does not encode all of RegExp's flags
Summary: Our structured cloning implementation does not encode all of RegExp's flags
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Bindings (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-07-07 16:44 PDT by Chris Dumez
Modified: 2021-07-07 19:20 PDT (History)
7 users (show)

See Also:


Attachments
Patch (11.65 KB, patch)
2021-07-07 16:54 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (19.46 KB, patch)
2021-07-07 18:23 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2021-07-07 16:44:35 PDT
Our structured cloning implementation does not encode all of RegExp's flags.

This is causing WebKit to fail one of the subtests on http://wpt.live/IndexedDB/structured-clone.any.html?61-80. This subtest is passing in both Chrome and Firefox.
Comment 1 Chris Dumez 2021-07-07 16:54:55 PDT
Created attachment 433093 [details]
Patch
Comment 2 Yusuke Suzuki 2021-07-07 17:54:28 PDT
Comment on attachment 433093 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433093&action=review

> Source/WebCore/bindings/js/SerializedScriptValue.cpp:1256
>                      flags[flagCount++] = 'i';
>                  if (regExp->regExp()->multiline())
>                      flags[flagCount++] = 'm';
> +                if (regExp->regExp()->dotAll())
> +                    flags[flagCount++] = 's';
> +                if (regExp->regExp()->unicode())
> +                    flags[flagCount++] = 'u';
> +                if (regExp->regExp()->sticky())
> +                    flags[flagCount++] = 'y';

Need to add `hasIndices` flag ("d") too (this is new flag introduced super recently).
Also, can you add a test for "d" flag case?
Comment 3 Chris Dumez 2021-07-07 17:55:30 PDT
(In reply to Yusuke Suzuki from comment #2)
> Comment on attachment 433093 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=433093&action=review
> 
> > Source/WebCore/bindings/js/SerializedScriptValue.cpp:1256
> >                      flags[flagCount++] = 'i';
> >                  if (regExp->regExp()->multiline())
> >                      flags[flagCount++] = 'm';
> > +                if (regExp->regExp()->dotAll())
> > +                    flags[flagCount++] = 's';
> > +                if (regExp->regExp()->unicode())
> > +                    flags[flagCount++] = 'u';
> > +                if (regExp->regExp()->sticky())
> > +                    flags[flagCount++] = 'y';
> 
> Need to add `hasIndices` flag ("d") too (this is new flag introduced super
> recently).
> Also, can you add a test for "d" flag case?

Will do, thanks for pointing that out.
Comment 4 Chris Dumez 2021-07-07 18:23:02 PDT
Created attachment 433105 [details]
Patch
Comment 5 Yusuke Suzuki 2021-07-07 18:58:15 PDT
Comment on attachment 433105 [details]
Patch

r=me
Comment 6 EWS 2021-07-07 19:19:26 PDT
Committed r279706 (239498@main): <https://commits.webkit.org/239498@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 433105 [details].
Comment 7 Radar WebKit Bug Importer 2021-07-07 19:20:19 PDT
<rdar://problem/80301211>