Skip to content

Fix InputStream leak in UploadModel.buildContributions() - #6861

Open
Roniscend wants to merge 1 commit into
commons-app:mainfrom
Roniscend:unsafe-resource
Open

Fix InputStream leak in UploadModel.buildContributions()#6861
Roniscend wants to merge 1 commit into
commons-app:mainfrom
Roniscend:unsafe-resource

Conversation

@Roniscend

Copy link
Copy Markdown
Contributor

Description (required)
Wrapped the InputStream initialization in a Kotlin .use block to guarantee safe resource closure
Fixes #6853

Tests performed (required)

Tested 6.5.0-debug on Pixel 9(Android 16)

Copilot AI review requested due to automatic review settings April 16, 2026 21:33

Copilot AI left a comment

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.

Pull request overview

This PR addresses Issue #6853 by ensuring the InputStream opened in UploadModel.buildContributions() is reliably closed, preventing resource leaks during SHA1 computation for uploads.

Changes:

  • Wraps ContentResolver.openInputStream(...) in a Kotlin use {} block when computing the image SHA1.
  • Adds a null-handling fallback when openInputStream(...) returns null.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread app/src/main/java/fr/free/nrw/commons/upload/UploadModel.kt Outdated
Signed-off-by: Owm Dubey <owmdubey163@gmail.com>
@github-actions

Copy link
Copy Markdown

✅ Generated APK variants!

@chrisdebian

Copy link
Copy Markdown

In the interest of being consistent with what we said on #6853: re-checked getSHA1() and it still closes the stream in its own finally block on every reachable path, so this isn't fixing an active leak. The one gap is if MessageDigest.getInstance("SHA1") throws NoSuchAlgorithmException, which returns before that finally runs — but SHA1 is a guaranteed algorithm on every JVM/Android implementation, so that's not reachable in practice either. The .use { } wrapper is harmless and does technically close the stream in that unreachable case too, so no objection to merging it, just flagging that it's a defensive addition rather than an actual leak fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

InputStream leak in UploadModel.buildContributions() due to unsafe resource handling

3 participants