1949166 - Firefox crashes when uploading multiples files/folder with multiple files via file picker without the agent running

Status: VERIFIED FIXED

(Firefox :: Data Loss Prevention, defect)

Description

[Bianca Hidecuti, Desktop Test Engineering [:bhidecuti]]

Attachment: video showing the issue

Found in

• 137.0a1 (2025-02-19)

Affected versions

• 137.0a1

Preconditions

• Download the DLP test assets from https://drive.google.com/file/d/1yjqVRuxdKV3WnO7D2wzMgDXBuYBxUgVw/view
• Create a distribution folder inside the Firefox folder and paste the policies-2.json to it and then rename it to policies.json

Tested platforms

• Affected platforms: Windows 10/11

Steps to reproduce

1. Go to https://mdn.github.io/learning-area/html/forms/file-examples/simple-file.html
2. Click the "Browse" button and select at least 2 files from your device
3. Click “Open” and observe the behavior

Expected result

• Firefox should not crash

Actual result

• Firefox is crashing

Regression range

• Mozregression points to bug 1936020

Additional notes

• See the attached video
• Also reproducing with policies-4.json but not with policies-1/-3/-5/-6/-7/-8
• Also reproducing when uploading a folder with at least 2 files (https://developer.mozilla.org/en-US/docs/Web/API/HTMLInputElement/webkitdirectory )
• Not reproducing if the agent is running
• Crash report: https://crash-stats.mozilla.org/report/index/f502b2cc-6803-482c-a4ac-c54a60250219#tab-details

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

:gstoll, since you are the author of the regressor, bug 1936020, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 2

[David Parks [:handyman]]

From the policies, this must be about default-warn.

Comment 3

[David Parks [:handyman]]

This is a bit hairy. We have at least two requests coming in when the agent isn't running. They both currently detect the agent isn't running in DoAnalyzeRequest on a background thread. The first one runs CancelWithError, which clears the request-token list for this user action in the user action map. However, since it is running default-warn, it does not remove the user action from the map. Later, the second request cancels in the same spot in DoAnalyzeRequest. When it reaches CancelWithError on the main thread, the token list has therefore already been emptied, but not for any of the expected reasons. It then erroneously removes the entry from the user action map, which means the user's response to the warn result couldn't be reported to the callback. That and the local variable callback are the only references to it, so, when the method returns, the callback is destroyed without having been called, which asserts. The callstack in comment 0 is a bit crazy but it looks much less confusing in debug -- we hit a MOZ_ASSERT in the ContentAnalysisCallback before the MOZ_RELEASE_ASSERT in comment 0 (the ContentAnalysisCallback destructor is stack frame 5).

The patches in bug 1888293 are related and change the behavior slightly but not fundamentally -- the bug persists.

There are a lot of simple approaches to working around this, like adding a boolean to indicate that a user action has hit a default warn, but there should be better solutions. My current theory is that default-warn shouldn't clear the tokens list (or should restore it), since it doesn't imply a response for all tokens -- they would all warn and therefore get their own user-supplied responses.

Comment 4

[David Parks [:handyman]]

Attachment: Bug 1949166: Don't always cancel all requests in ContentAnalysis::CancelWithError r=#dlp-reviewers

Allow the MultipartRequestCallback to handle cancellations based on a verdict.
It will, for example, cancel the other requests when a part receives a block
response. It also means that the userActionId map is handled by the MPCB (with
the exception of the period before the final requests are created -- which is
where we could separate ContentAnalysis from "external" APIs e.g. that get tested,
and "internal" ones that are mocked). This simplifies a lot of logic and was
a major goal of bug 1936020 that didn't quite make it in there.

See bug 1949166 comment 3 for a thorough explanation of the bug and this fix.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/60cab09e8e00
Don't always cancel all requests in ContentAnalysis::CancelWithError r=dlp-reviewers,gstoll

Comment 6

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/60cab09e8e00

Comment 7

[Alexandru Trif, Desktop Test Engineering [:atrif]]

Verified that the crash is no longer reproducing with Firefox 137.0a1 (2025-02-23) after following steps from comment 0 with policies-2 and policies-4 on Windows 10x64.