From 4b270d7902363bcc3118a66c1f9fd9bf53d92b3d Mon Sep 17 00:00:00 2001 From: David Luhmer Date: Sat, 11 Jul 2026 23:01:32 +0200 Subject: [PATCH] fix: surface readable error when server app exception cannot be deserialized 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 --- .../android/sso/api/AidlNetworkRequest.java | 20 +++- .../sso/api/AidlNetworkRequestTest.java | 106 ++++++++++++++++++ 2 files changed, 123 insertions(+), 3 deletions(-) create mode 100644 lib/src/test/java/com/nextcloud/android/sso/api/AidlNetworkRequestTest.java diff --git a/lib/src/main/java/com/nextcloud/android/sso/api/AidlNetworkRequest.java b/lib/src/main/java/com/nextcloud/android/sso/api/AidlNetworkRequest.java index bc79b59a..aec1b73f 100644 --- a/lib/src/main/java/com/nextcloud/android/sso/api/AidlNetworkRequest.java +++ b/lib/src/main/java/com/nextcloud/android/sso/api/AidlNetworkRequest.java @@ -25,6 +25,7 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import androidx.annotation.VisibleForTesting; import com.google.gson.Gson; import com.google.gson.internal.LinkedTreeMap; @@ -229,10 +230,23 @@ private static T deserializeObject(InputStream is) throws IOException, Class return (T) new ObjectInputStream(is).readObject(); } - private ExceptionResponse deserializeObjectV2(InputStream is) throws IOException, ClassNotFoundException { + @VisibleForTesting + static ExceptionResponse deserializeObjectV2(InputStream is) throws IOException, ClassNotFoundException { final ObjectInputStream ois = new ObjectInputStream(is); final ArrayList headerList = new ArrayList<>(); - final Exception exception = (Exception) ois.readObject(); + Exception exception; + try { + exception = (Exception) ois.readObject(); + } catch (ClassNotFoundException e) { + // The server app serializes the original exception object. If its class - or any + // class in its cause chain - only exists in the server app (e.g. + // com.owncloud.android.lib.common.network.CertificateCombinedException on SSL errors), + // deserialization fails here. Surface a plain exception carrying the original type + // name instead of a ClassNotFoundException that hides the actual error. + exception = new Exception("The Nextcloud Files app responded with an error that could not" + + " be read by this app: " + e.getMessage() + + ". Check the server connection (e.g. SSL certificate) or the Files app log for the actual error."); + } if (exception == null) { final String headers = (String) ois.readObject(); @@ -275,7 +289,7 @@ private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundE } } - private record ExceptionResponse( + record ExceptionResponse( @NonNull ArrayList headers, @Nullable Exception exception ) { diff --git a/lib/src/test/java/com/nextcloud/android/sso/api/AidlNetworkRequestTest.java b/lib/src/test/java/com/nextcloud/android/sso/api/AidlNetworkRequestTest.java new file mode 100644 index 00000000..7a5c01dd --- /dev/null +++ b/lib/src/test/java/com/nextcloud/android/sso/api/AidlNetworkRequestTest.java @@ -0,0 +1,106 @@ +/* + * Nextcloud Android SingleSignOn Library + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.android.sso.api; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.ObjectOutputStream; +import java.nio.charset.StandardCharsets; + +public class AidlNetworkRequestTest { + + /** + * Exception class only used to be renamed in the serialized stream, simulating an exception + * type that exists in the Files app but not in the client app (e.g. + * {@code com.owncloud.android.lib.common.network.CertificateCombinedException}). + */ + public static class OnlyInFilesAppException extends Exception { + public OnlyInFilesAppException(String message) { + super(message); + } + } + + @Test + public void deserializeObjectV2_exceptionClassMissingInClientApp_returnsReadableException() throws Exception { + final byte[] data = serialize(new OnlyInFilesAppException("certificate expired"), "[]"); + + // Rename the class inside the serialized stream to one that does not exist on this + // side of the IPC channel (same length, so the stream structure stays intact) + final byte[] tampered = replaceBytes(data, "OnlyInFilesAppException", "OnlyInF1lesAppException"); + + final var response = AidlNetworkRequest.deserializeObjectV2(new ByteArrayInputStream(tampered)); + + assertNotNull(response.exception()); + assertFalse(response.exception() instanceof ClassNotFoundException); + assertNotNull(response.exception().getMessage()); + assertTrue("message should contain the original exception type", + response.exception().getMessage().contains("OnlyInF1lesAppException")); + } + + @Test + public void deserializeObjectV2_noException_parsesHeaders() throws Exception { + final byte[] data = serialize(null, "[{\"name\":\"Content-Type\",\"value\":\"application/json\"}]"); + + final var response = AidlNetworkRequest.deserializeObjectV2(new ByteArrayInputStream(data)); + + assertNull(response.exception()); + assertEquals(1, response.headers().size()); + assertEquals("Content-Type", response.headers().get(0).getName()); + assertEquals("application/json", response.headers().get(0).getValue()); + } + + @Test + public void deserializeObjectV2_knownException_returnsItUnchanged() throws Exception { + final byte[] data = serialize(new IllegalStateException("CE_1"), "[]"); + + final var response = AidlNetworkRequest.deserializeObjectV2(new ByteArrayInputStream(data)); + + assertTrue(response.exception() instanceof IllegalStateException); + assertEquals("CE_1", response.exception().getMessage()); + } + + /** Mirrors {@code InputStreamBinder#serializeObjectToInputStreamV2} in the Files app. */ + private static byte[] serialize(Exception exception, String headers) throws IOException { + try (ByteArrayOutputStream baos = new ByteArrayOutputStream(); + ObjectOutputStream oos = new ObjectOutputStream(baos)) { + oos.writeObject(exception); + oos.writeObject(headers); + oos.flush(); + return baos.toByteArray(); + } + } + + private static byte[] replaceBytes(byte[] data, String search, String replace) { + final byte[] searchBytes = search.getBytes(StandardCharsets.US_ASCII); + final byte[] replaceBytes = replace.getBytes(StandardCharsets.US_ASCII); + assertEquals(searchBytes.length, replaceBytes.length); + + final byte[] result = data.clone(); + for (int i = 0; i <= result.length - searchBytes.length; i++) { + boolean match = true; + for (int j = 0; j < searchBytes.length; j++) { + if (result[i + j] != searchBytes[j]) { + match = false; + break; + } + } + if (match) { + System.arraycopy(replaceBytes, 0, result, i, replaceBytes.length); + } + } + return result; + } +}