Skip to content

BLOB download is not being moved to its final destination#36053

Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
pvollan:eng/BLOB-download-is-not-being-moved-to-its-final-destination
Nov 1, 2024
Merged

BLOB download is not being moved to its final destination#36053
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
pvollan:eng/BLOB-download-is-not-being-moved-to-its-final-destination

Conversation

@pvollan
Copy link
Copy Markdown
Contributor

@pvollan pvollan commented Nov 1, 2024

55447b1

BLOB download is not being moved to its final destination
https://bugs.webkit.org/show_bug.cgi?id=282453
rdar://138012063

Reviewed by Chris Dumez.

This is because we are revoking the sandbox extension to the downloaded file before the system moves
the file to its final location. This patch transfers the sandbox extension to the Download object,
in the same way we do for standard downloads. This change makes sure that we are not revoking the
sandbox extension until the file has been moved to its final location.

* Source/WebKit/NetworkProcess/NetworkDataTaskBlob.cpp:
(WebKit::NetworkDataTaskBlob::didFinishDownload):

Canonical link: https://commits.webkit.org/286037@main

a1b82b6

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac 🛠 wpe 🛠 win
🛠 ios-sim ✅ 🛠 mac-AS-debug 🧪 wpe-wk2 🧪 win-tests
✅ 🧪 webkitperl 🧪 ios-wk2 🧪 api-mac 🧪 api-wpe
🧪 ios-wk2-wpt 🛠 wpe-cairo
🧪 api-ios 🧪 mac-wk2 🛠 gtk
🛠 vision 🧪 mac-AS-debug-wk2 🧪 gtk-wk2
🛠 vision-sim 🧪 mac-wk2-stress 🧪 api-gtk
⏳ 🧪 vision-wk2 🧪 mac-intel-wk2
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv 🛠 mac-safer-cpp
🛠 tv-sim
🛠 watch
🛠 watch-sim

@pvollan pvollan requested a review from cdumez as a code owner November 1, 2024 18:29
@pvollan pvollan self-assigned this Nov 1, 2024
@pvollan pvollan added the WebKit Process Model Bugs related to WebKit's multi-process architecture label Nov 1, 2024
@pvollan pvollan requested review from beidson and szewai November 1, 2024 21:57
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::exchange(m_sandboxExtension, nullptr) is better practice in my opinion for data members that may get re-used after the move.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good catch, will fix!

Thanks for reviewing!

@pvollan pvollan force-pushed the eng/BLOB-download-is-not-being-moved-to-its-final-destination branch from fca3174 to a1b82b6 Compare November 1, 2024 23:13
@pvollan pvollan added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Nov 1, 2024
https://bugs.webkit.org/show_bug.cgi?id=282453
rdar://138012063

Reviewed by Chris Dumez.

This is because we are revoking the sandbox extension to the downloaded file before the system moves
the file to its final location. This patch transfers the sandbox extension to the Download object,
in the same way we do for standard downloads. This change makes sure that we are not revoking the
sandbox extension until the file has been moved to its final location.

* Source/WebKit/NetworkProcess/NetworkDataTaskBlob.cpp:
(WebKit::NetworkDataTaskBlob::didFinishDownload):

Canonical link: https://commits.webkit.org/286037@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/BLOB-download-is-not-being-moved-to-its-final-destination branch from a1b82b6 to 55447b1 Compare November 1, 2024 23:58
@webkit-commit-queue
Copy link
Copy Markdown
Collaborator

Committed 286037@main (55447b1): https://commits.webkit.org/286037@main

Reviewed commits have been landed. Closing PR #36053 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 55447b1 into WebKit:main Nov 1, 2024
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WebKit Process Model Bugs related to WebKit's multi-process architecture

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants