WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
187461
Fetch using FormData with file doesn't go through Service Worker
https://bugs.webkit.org/show_bug.cgi?id=187461
Summary
Fetch using FormData with file doesn't go through Service Worker
nyro
Reported
2018-07-09 02:15:03 PDT
Created
attachment 344569
[details]
Zip containing all 4 files described When using a FormData with file to send content of a form using the fetch API, the request doesn't go through the associated Service Worker. To show it, I setup the attached file : # index.html Contains 3 forms with a simple debug div to show what's happening on devices # test.js Handle the submit of the form in order to: 1. prevent default it 2. Create a formData 3. If the form has the id "removeFile", delete the file from the formData 4. Send data to service worker/server using the fetch API 5. Get a JSON response 6. Show the json response on the debug # sw.js A minimal service worker that only add a "X-FROM-SW" header to the request containing the timestamp # up.php A simple PHP script to return a JSON with the received "X-FROM-SW" or false if not found. # Observed results - Simple Form : everything ok as expected and the result contains the X-FROM-SW - File form : doesn't pass through the service worker (no log in the fetch event), so there is no header added - Remove file form : treated like the simple form, so the result contains the X-FROM-SW I setup these file on the following URL:
https://bugs.nyro.com/safariFileUpload/
Attachments
Zip containing all 4 files described
(4.91 KB, application/zip)
2018-07-09 02:15 PDT
,
nyro
no flags
Details
Patch
(42.87 KB, patch)
2022-01-03 08:09 PST
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Patch
(37.10 KB, patch)
2022-01-03 08:14 PST
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Patch
(41.90 KB, patch)
2022-01-03 11:08 PST
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Patch
(46.13 KB, patch)
2022-01-04 04:20 PST
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Patch for landing
(46.79 KB, patch)
2022-01-05 01:30 PST
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2018-07-09 09:14:18 PDT
<
rdar://problem/41975544
>
youenn fablet
Comment 2
2018-07-15 07:13:04 PDT
Thanks for the report. This is a known limitation of the service worker implementation at the moment in FetchBody::fromFormData.
youenn fablet
Comment 3
2018-07-15 07:14:41 PDT
A potential workaround is to use blobs if possible although there might be limitations there as well.
nyro
Comment 4
2018-07-24 08:31:51 PDT
Youenn, following your recommandation, I ended up with the following: ##################### formData.delete('file'); // get it as blob adn readd it as regular post field // NB: File has to be recreated on server side using _files[file][name] and _files[file][data] to work as it was a regular file upload var file = form.querySelector('input[type="file"]').files[0]; formData.append('_files[file][name]', file.name); var reader = new FileReader(); reader.onload = function (e) { formData.append('_files[file][data]', e.target.result); doFetch(id, form, formData); }; reader.readAsDataURL(file); ##################### Which works, the request goes through the service Worker. On server side, it becomes a little tricky regarding the language choosen, but I mentionned all the necessearry regarding PHP, the one I'm using. I didn't implemented it yet on my production PWA, but I'll do it soon and come back here if I found out more gotcha. In the mean time, I hope this will be fixed soon on Safari in order to remove all this code ;) All the updated code and exemple have been updated on my demo page:
https://bugs.nyro.com/safariFileUpload/
nyro
Comment 5
2020-07-25 23:11:03 PDT
Hello, Is there any news about that? It's been 2 years now...
nyro
Comment 6
2021-07-28 14:55:30 PDT
Hello, Is there any news about that? It's been 3 years now...
youenn fablet
Comment 7
2022-01-03 08:09:24 PST
Created
attachment 448239
[details]
Patch
youenn fablet
Comment 8
2022-01-03 08:14:03 PST
Created
attachment 448241
[details]
Patch
youenn fablet
Comment 9
2022-01-03 11:08:55 PST
Created
attachment 448249
[details]
Patch
youenn fablet
Comment 10
2022-01-04 04:20:49 PST
Created
attachment 448282
[details]
Patch
Chris Dumez
Comment 11
2022-01-04 09:14:21 PST
Comment on
attachment 448282
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=448282&action=review
r=me with nits.
> Source/WebCore/Modules/fetch/FetchBodyConsumer.cpp:278 > + if (!value.size()) {
could probably use `value.empty()`
> Source/WebCore/Modules/fetch/FetchBodyConsumer.cpp:284 > + builder.append(value.data(), value.size());
A bit disappointing that FragmentedSharedBuffer doesn't have an append() overload that takes a Span<>. We should probably add one.
> Source/WebCore/Modules/fetch/FetchBodyConsumer.cpp:306 > + if (!value.size()) {
could be `value.empty()`
> Source/WebCore/Modules/fetch/FormDataConsumer.cpp:59 > + WTF::switchOn(m_formData->elements()[m_currentElementIndex++].data, [this](const Vector<uint8_t>& content) {
May not need WTF::
> Source/WebCore/Modules/fetch/FormDataConsumer.cpp:70 > + consume(Span<const uint8_t> { content.data(), content.size() });
Can't you simply call `content.span()` ?
> Source/WebCore/Modules/fetch/FormDataConsumer.cpp:121 > + m_callback(Span<const uint8_t> { content.data(), content.size() });
Source and destination are both a Span<const uint8_t>, why aren't we passing content directly instead of reconstructing a span? Am I missing something?
youenn fablet
Comment 12
2022-01-05 01:07:49 PST
***
Bug 183695
has been marked as a duplicate of this bug. ***
youenn fablet
Comment 13
2022-01-05 01:30:09 PST
Created
attachment 448367
[details]
Patch for landing
EWS
Comment 14
2022-01-05 02:47:38 PST
Committed
r287612
(
245739@main
): <
https://commits.webkit.org/245739@main
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 448367
[details]
.
youenn fablet
Comment 15
2022-01-05 02:59:28 PST
> > Source/WebCore/Modules/fetch/FetchBodyConsumer.cpp:278 > > + if (!value.size()) { > > could probably use `value.empty()`
OK
> > Source/WebCore/Modules/fetch/FetchBodyConsumer.cpp:284 > > + builder.append(value.data(), value.size()); > > A bit disappointing that FragmentedSharedBuffer doesn't have an append() > overload that takes a Span<>. We should probably add one.
Added it.
> > Source/WebCore/Modules/fetch/FetchBodyConsumer.cpp:306 > > + if (!value.size()) { > > could be `value.empty()`
OK
> > Source/WebCore/Modules/fetch/FormDataConsumer.cpp:59 > > + WTF::switchOn(m_formData->elements()[m_currentElementIndex++].data, [this](const Vector<uint8_t>& content) { > > May not need WTF::
Removed.
> > Source/WebCore/Modules/fetch/FormDataConsumer.cpp:70 > > + consume(Span<const uint8_t> { content.data(), content.size() }); > > Can't you simply call `content.span()` ?
Didn't know about this one, used it.
> > Source/WebCore/Modules/fetch/FormDataConsumer.cpp:121 > > + m_callback(Span<const uint8_t> { content.data(), content.size() }); > > Source and destination are both a Span<const uint8_t>, why aren't we passing > content directly instead of reconstructing a span? Am I missing something?
I was missing a WTFMove.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug