Bug 229814

Summary:

Implement origin partitioning (top-origin/frame-origin) for BroadcastChannel

Product:

WebKit

Reporter:

Chris Dumez <cdumez>

Component:

WebCore Misc.

Assignee:

Chris Dumez <cdumez>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, beidson, darin, esprehn+autocc, ews-watchlist, ggaren, japhet, kangil.han, rniwa, sam, webkit-bug-importer, wilander

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

URL:

https://github.com/whatwg/html/issues/5803

See Also:

https://bugs.webkit.org/show_bug.cgi?id=230164

Bug Depends on:

Bug Blocks:

161472

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none

Chris Dumez

Reported 2021-09-02 10:05:53 PDT

Implement origin partitioning (top-origin/frame-origin) for BroadcastChannel to address privacy concerns, as discussed here: - https://github.com/whatwg/html/issues/5803

Attachments
Patch (35.89 KB, patch) 2021-09-02 10:13 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (34.37 KB, patch) 2021-09-02 10:15 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (34.08 KB, patch) 2021-09-02 10:44 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (32.51 KB, patch) 2021-09-02 13:50 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (33.76 KB, patch) 2021-09-02 16:55 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Chris Dumez

Comment 1 2021-09-02 10:13:51 PDT

Created attachment 437163 [details] Patch

Chris Dumez

Comment 2 2021-09-02 10:15:41 PDT

Created attachment 437164 [details] Patch

Chris Dumez

Comment 3 2021-09-02 10:44:16 PDT

Created attachment 437166 [details] Patch

Sam Weinig

Comment 4 2021-09-02 13:08:32 PDT

Comment on attachment 437166 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437166&action=review > Tools/TestRunnerShared/TestFeatures.cpp:128 > +static bool shouldDisableBroadcastChannelOriginPartitioning(const std::string& pathOrURL) > +{ > + return pathContains(pathOrURL, "localhost:8800/") || pathContains(pathOrURL, "localhost:9443/"); > +} We'd like to avoid adding more of these path based filters (the goal was to remove them all eventually). Can we just disable it by default for all tests instead and use header comments to enable it where needed?

Chris Dumez

Comment 5 2021-09-02 13:10:05 PDT

(In reply to Sam Weinig from comment #4) > Comment on attachment 437166 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=437166&action=review > > > Tools/TestRunnerShared/TestFeatures.cpp:128 > > +static bool shouldDisableBroadcastChannelOriginPartitioning(const std::string& pathOrURL) > > +{ > > + return pathContains(pathOrURL, "localhost:8800/") || pathContains(pathOrURL, "localhost:9443/"); > > +} > > We'd like to avoid adding more of these path based filters (the goal was to > remove them all eventually). > > Can we just disable it by default for all tests instead and use header > comments to enable it where needed? We could. But since it would be enabled in the shipping configuration, ideally, I'd want it to be the default in our tests I think.

Chris Dumez

Comment 6 2021-09-02 13:50:54 PDT

Created attachment 437189 [details] Patch

Chris Dumez

Comment 7 2021-09-02 13:58:55 PDT

Chris Dumez

Comment 8 2021-09-02 16:55:03 PDT

Created attachment 437220 [details] Patch

Sam Weinig

Comment 9 2021-09-06 10:15:45 PDT

(In reply to Chris Dumez from comment #5) > (In reply to Sam Weinig from comment #4) > > Comment on attachment 437166 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=437166&action=review > > > > > Tools/TestRunnerShared/TestFeatures.cpp:128 > > > +static bool shouldDisableBroadcastChannelOriginPartitioning(const std::string& pathOrURL) > > > +{ > > > + return pathContains(pathOrURL, "localhost:8800/") || pathContains(pathOrURL, "localhost:9443/"); > > > +} > > > > We'd like to avoid adding more of these path based filters (the goal was to > > remove them all eventually). > > > > Can we just disable it by default for all tests instead and use header > > comments to enable it where needed? > > We could. But since it would be enabled in the shipping configuration, > ideally, I'd want it to be the default in our tests I think. The opposite would also be fine (enable by default, disable explicitly where needed).

Chris Dumez

Comment 10 2021-09-07 07:25:13 PDT

(In reply to Sam Weinig from comment #9) > (In reply to Chris Dumez from comment #5) > > (In reply to Sam Weinig from comment #4) > > > Comment on attachment 437166 [details] > > > Patch > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=437166&action=review > > > > > > > Tools/TestRunnerShared/TestFeatures.cpp:128 > > > > +static bool shouldDisableBroadcastChannelOriginPartitioning(const std::string& pathOrURL) > > > > +{ > > > > + return pathContains(pathOrURL, "localhost:8800/") || pathContains(pathOrURL, "localhost:9443/"); > > > > +} > > > > > > We'd like to avoid adding more of these path based filters (the goal was to > > > remove them all eventually). > > > > > > Can we just disable it by default for all tests instead and use header > > > comments to enable it where needed? > > > > We could. But since it would be enabled in the shipping configuration, > > ideally, I'd want it to be the default in our tests I think. > > The opposite would also be fine (enable by default, disable explicitly where > needed). Put I cannot add header comments to disable the feature in WPT tests, which is why I was doing it some other way.

EWS

Comment 11 2021-09-07 12:38:15 PDT

Committed r282105 (241403@main): <https://commits.webkit.org/241403@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 437220 [details].

Radar WebKit Bug Importer

Comment 12 2021-09-07 12:39:21 PDT

<rdar://problem/82834757>

Chris Dumez

Comment 13 2021-09-07 14:18:10 PDT

Follow-up fix in <https://commits.webkit.org/r282106>.

Chris Dumez

Comment 14 2021-09-09 17:02:08 PDT

Follow-up fix in <https://commits.webkit.org/r282246>.

Note You need to log in before you can comment on or make changes to this bug.