Bug 225636 – Allow logging minimal info about sending media files through XMLHttpRequest

• Status: RESOLVED
• Resolution: FIXED
• Component: Images

[Said Abou-Hallawa]

Some files may be uploaded from JavaScript through XMLHttpRequest. We need to handle this case as we did for HTMLFormElement::submit in r275103.

[Said Abou-Hallawa]

<rdar://problem/76639138>

[Said Abou-Hallawa]

Created attachment 428252 [details]
Patch

[Said Abou-Hallawa]

Created attachment 428254 [details]
Patch

[Said Abou-Hallawa]

Created attachment 428302 [details]
Patch

[Alex Christensen]

Comment on attachment 428302 [details]
Patch

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

> Source/WebCore/html/HTMLFormElement.cpp:408
> +    document().page()->logSubmittedImageOrMediaFiles(imageOrMediaFilesCount);

Let's null check page:
if (auto* page = document().page())
    page->...

> Source/WebCore/page/Page.cpp:1056
> +void Page::logSubmittedImageOrMediaFiles(unsigned imageOrMediaFilesCount)

Why don't we just make one function and pass a FormData& as a parameter instead of one function to count then another function to log?

> Source/WebCore/xml/XMLHttpRequest.cpp:549
> +        if (auto page = scriptExecutionContext()->isDocument() ? document()->page() : nullptr)

auto*

It's also not great to introduce code in XHR that only works in document context.  This fixes another case of unreported FormData uploading, but workers are the next case and fetch is probably the one after that.  This might be nicer in a more central location, like if we added a request parameter to addParametersShared and checked its HTTP body there, instead of one in HTMLFormElement and one in xhr.

[Said Abou-Hallawa]

Created attachment 428703 [details]
Patch

[Said Abou-Hallawa]

Comment on attachment 428302 [details]
Patch

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

>> Source/WebCore/xml/XMLHttpRequest.cpp:549
>> +        if (auto page = scriptExecutionContext()->isDocument() ? document()->page() : nullptr)
> 
> auto*
> 
> It's also not great to introduce code in XHR that only works in document context.  This fixes another case of unreported FormData uploading, but workers are the next case and fetch is probably the one after that.  This might be nicer in a more central location, like if we added a request parameter to addParametersShared and checked its HTTP body there, instead of one in HTMLFormElement and one in xhr.

This is a great idea. I was looking for a common place for all file uploading but I could not find one because I was looking for it in WebCore. I added the logging in addParametersShared().

[EWS]

Committed r277666 (237868@main): <https://commits.webkit.org/237868@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 428703 [details].