Skip to content

fix: surface readable error when server app exception cannot be deserialized#1029

Draft
David-Development wants to merge 1 commit into
mainfrom
fix-exception-deserialization
Draft

fix: surface readable error when server app exception cannot be deserialized#1029
David-Development wants to merge 1 commit into
mainfrom
fix-exception-deserialization

Conversation

@David-Development

Copy link
Copy Markdown
Member

Problem

When a request fails inside the Files app, InputStreamBinder serializes the original exception object over the AIDL pipe. If the exception's class — or any class in its cause chain — only exists in the Files app (e.g. com.owncloud.android.lib.common.network.CertificateCombinedException on SSL/certificate errors), ObjectInputStream#readObject in AidlNetworkRequest#deserializeObjectV2 throws a ClassNotFoundException in the client app.

The real error is completely lost, and client apps end up showing something like:

java.lang.ClassNotFoundException: com.owncloud.android.lib.common.net...

See nextcloud/news-android#1645 for a report of exactly this (expired SSL certificate → cryptic ClassNotFoundException in the News app).

Fix

Catch the ClassNotFoundException during exception deserialization and replace it with a plain exception that carries the original type name plus a hint to check the server connection. It then flows through the existing SSOException#parseNextcloudCustomException translation, so client apps receive a proper SSOException (UnknownErrorException) and show their regular error dialog instead of a raw ClassNotFoundException.

This is the receiver-side safety net and works against all already-shipped Files app versions. Follow-up (separate PR in nextcloud/android): flatten exceptions to plain JRE types in InputStreamBinder#serializeObjectToInputStreamV2 before serializing, so the original error message survives the IPC as well, not just the type name.

Tests

Added AidlNetworkRequestTest:

  • missing-class scenario: serializes a real exception, then byte-patches its class name in the serialized stream to a nonexistent one — reproducing exactly what happens over the IPC channel; asserts a readable exception (not ClassNotFoundException) containing the original type name is returned
  • happy path: null exception + headers are still parsed correctly
  • deserializable exceptions still pass through unchanged

🤖 Generated with Claude Code

…ialized

The Files app serializes the original exception object over the AIDL
pipe. If its class - or any class in its cause chain - only exists in
the Files app (e.g. CertificateCombinedException from the owncloud
library on SSL errors), ObjectInputStream#readObject throws a
ClassNotFoundException in the client app, hiding the actual error
behind a message like:

  java.lang.ClassNotFoundException: com.owncloud.android.lib.common.net...

Catch the ClassNotFoundException during deserialization and replace it
with a plain exception carrying the original type name and a hint to
check the server connection. It flows through the existing
parseNextcloudCustomException translation, so client apps get a proper
SSOException (UnknownErrorException) instead of a raw
ClassNotFoundException.

Ref nextcloud/news-android#1645

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

SpotBugs

CategoryBaseNew
Bad practice99
Correctness2424
Dodgy code1313
Internationalization22
Malicious code vulnerability1111
Multithreaded correctness44
Performance44
Security77
Total7474

@codacy-production

Copy link
Copy Markdown

Not up to standards ⛔

🔴 Issues 3 minor

Alerts:
⚠ 3 issues (≤ 0 issues of at least minor severity)

Results:
3 new issues

Category Results
CodeStyle 3 minor

View in Codacy

🟢 Metrics 10 complexity

Metric Results
Complexity 10

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

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.

1 participant