Skip to content

fix: guard startSyncFolderOperation against a null folder (#4738) - #4986

Merged
joragua merged 2 commits into
owncloud:masterfrom
ibrahim-iqbal:fix/npe-receive-external-files-4738
Sep 29, 2026
Merged

joragua merged 2 commits into
owncloud:masterfrom
ibrahim-iqbal:fix/npe-receive-external-files-4738

Conversation

@ibrahim-iqbal

Copy link
Copy Markdown
Contributor

Related Issues

App: closes #4738

ReceiveExternalFilesViewModel.refreshFolderUseCase(folderToSync: OCFile) is a Kotlin function with a non-null parameter. ReceiveExternalFilesActivity.startSyncFolderOperation forwarded whatever came in without a null check, so the two entry points that can hand back a null folder crashed with the Kotlin platform-type NPE reported in issue #4738:

  • onSavedCertificate() → startSyncFolderOperation(getCurrentDir()), and FileActivity.getCurrentDir() returns null when getFile() is null, when the current file isn't a folder and the storage manager is missing, or when the storage manager can't resolve the parent path (FileActivity.java:455-466).
  • The Play console stack in the issue matches this exact bridge — refreshFolderUseCase (Unknown Source:6) is the Kotlin null check on the parameter.

Changes

  • ReceiveExternalFilesActivity.startSyncFolderOperation: short-circuit and log via Timber when folder is null instead of forwarding to Kotlin.

  • Kept every other caller unchanged — onBackPressed (line 471), onItemClick (line 500), and the initial spaces-root call (line 259) already null-check the folder they pass in, so no other site needed the guard.

  • Changelog file at changelog/unreleased/4738 (Bugfix: type).

  • Release Notes in ReleaseNotesViewModel.kt — crash-only, not a user-visible feature; happy to add one if you'd prefer.


QA

  • Local:
    • ./gradlew :owncloudApp:compileOriginalDebugJavaWithJavac → BUILD SUCCESSFUL.
    • No other callers of startSyncFolderOperation had to change; grep confirms four call sites and the three that aren't getCurrentDir() already guard the value.
  • Manual repro (attach a file into the app, navigate through folders, dismiss a certificate dialog, then rotate/back) is worth running once during review since the crash is timing-dependent, but the null path is now impossible to reach past the guard.

@ibrahim-iqbal
ibrahim-iqbal requested a review from a team as a code owner September 21, 2026 07:34

@joragua joragua left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for this contribution @ibrahim-iqbal! 👌🏻 The first CR is here!

Do not forget to split the changes into two commit: one for the fix and another one for the calens file (chore: add calens file). Moreover, all commits must be signed. Otherwise, we will not be able to merge this PR.

Let us know if you have any doubts!

Comment thread changelog/unreleased/4738 Outdated
Signed-off-by: ibrahim-iqbal <ibrahim-iqbal@users.noreply.github.com>
)

Signed-off-by: ibrahim-iqbal <ibrahim-iqbal@users.noreply.github.com>
@ibrahim-iqbal
ibrahim-iqbal force-pushed the fix/npe-receive-external-files-4738 branch from 692d2e2 to 2698bec Compare September 26, 2026 12:05
@ibrahim-iqbal

Copy link
Copy Markdown
Contributor Author

Hey @joragua! 👋 All feedback addressed in caff30d + 2698bec:

  • ✅ Calens file renamed to 4986, rewritten in present perfect passive tense, PR link added
  • ✅ Removed the inline comment — the null check + Timber log are self-explanatory
  • ✅ Split into two commits: chore: add calens file + fix: guard startSyncFolderOperation against a null folder (#4738)
  • ✅ Both commits are GPG-signed and include Signed-off-by

Let me know if anything else needs adjusting! 🚀

@joragua joragua left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM! Good job! 💯

Let's move it to QA, but this fix will need to be verified in the Play Console when a new version of the app is released. Stay tuned!

@jesmrec

jesmrec commented Sep 29, 2026

Copy link
Copy Markdown
Member

CI passes, that's enough for the moment. Will check when next release is out, and no more appearances in GPC.

@joragua
joragua merged commit 5605f68 into owncloud:master Sep 29, 2026
7 checks passed
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.

[BUG] Play console crash: NPE refreshFolderUseCase

3 participants