Bug 246526

Summary: [Fetch API] Headers iteration should not happen on cached headers list
Product: WebKit Reporter: Raphael Kubo da Costa (:rakuco) <rakuco>
Component: WebCore Misc.Assignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, annevk, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 151937    

Description Raphael Kubo da Costa (:rakuco) 2022-10-14 09:56:01 PDT
See https://github.com/web-platform-tests/wpt/pull/36455

FetchHeaders::Iterator seems to cache a FetchHeaders' keys at the time iteration starts, so if the same instance is modified during iteration not all changes will be visible.
Comment 1 Radar WebKit Bug Importer 2022-10-17 00:01:56 PDT
<rdar://problem/101235580>
Comment 2 youenn fablet 2022-10-17 00:50:07 PDT
Pull request: https://github.com/WebKit/WebKit/pull/5437
Comment 3 Anne van Kesteren 2022-10-17 01:38:40 PDT
Raphael, did you happen to check `FormData` and `URLSearchParams` as well? This might benefit from testing in each API with `iterable<>` given how it's implemented.
Comment 4 EWS 2022-10-17 12:06:43 PDT
Committed 255639@main (68f5a2f24c94): <https://commits.webkit.org/255639@main>

Reviewed commits have been landed. Closing PR #5437 and removing active labels.
Comment 5 Raphael Kubo da Costa (:rakuco) 2022-10-18 12:02:59 PDT
(In reply to Anne van Kesteren from comment #3)
> Raphael, did you happen to check `FormData` and `URLSearchParams` as well?
> This might benefit from testing in each API with `iterable<>` given how it's
> implemented.

URLSearchParams is covered by https://github.com/web-platform-tests/wpt/blob/master/webidl/ecmascript-binding/iterator-invalidation-foreach.html (I actually based the Fetch tests on it).

I've submitted https://github.com/web-platform-tests/wpt/pull/36532 to cover FormData.

I haven't checked it in depth, but I think all engines implement them correctly because their "value pairs to iterate over" are just their internal name-value lists.

Things get trickier with Headers because it's one of the few cases where the value pairs require modification to the internal data structure. AFAICS only a handful of interfaces do that: Headers, MediaKeyStatusMap (possibly), XRHand (also possibly), and StylePropertyMap (it's CSS so I have close to no idea :-).
Comment 6 Anne van Kesteren 2022-10-19 02:00:46 PDT
Thanks Raphael, much appreciated!