Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 97c4d138 authored by Tobias Thierer's avatar Tobias Thierer
Browse files

Fix Base64OutputStream swallowing/throwing the wrong exception. 

Base64OutputStream.close() first write()s (possibly empty) data
and then close()s the underlying stream. Because of an inverted
condition, exceptions thrown by the underlying stream were handled
incorrectly:

 - if both the underlying write() and close()/flush() throw
   then it would throw the latter exception when it should
   have thrown the former, suppressing the latter.
 - if only the underlying close() throws then that exception
   was swallowed when it should have been closed.

This bug existed ever since this file was first added to
Android in commit 9df2ffd4
in Feb 2010 (before the concept of suppressed exceptions
existed/was supported). The buggy code was proposed in a
review comment on that CL, which was then accepted/integrated
into the CL.

This CL fixes the inverted condition and adds test coverage.
It also adds code to add the suppressed exception (if any).

Bug: 111697617
Test: Confirmed that Base64Test fails (as described above) before
  this CL but passes after this CL, when running these commands:
  make FrameworksCoreTests && \
  adb install -r ${ANDROID_PRODUCT_OUT}/data/app/FrameworksCoreTests/FrameworksCoreTests.apk && \
  adb shell am instrument -w -e class android.util.Base64Test \
    com.android.frameworks.coretests/android.support.test.runner.AndroidJUnitRunner

  Specifically, before this CL, the following test failures occur:
  1.) junit.framework.AssertionFailedError
	at junit.framework.Assert.fail(Assert.java:48)
	at junit.framework.Assert.fail(Assert.java:56)
	at android.util.Base64Test.testOutputStream_ioExceptionDuringClose(Base64Test.java:546)
  2.) junit.framework.ComparisonFailure: expected:<[writ]e()> but was:<[clos]e()>
	at junit.framework.Assert.assertEquals(Assert.java:85)
	at junit.framework.Assert.assertEquals(Assert.java:91)
	at android.util.Base64Test.testOutputStream_ioExceptionDuringCloseAndWrite(Base64Test.java:574)

Change-Id: If7fd7c4615ca004638d7c5d8f1869e7eddb16f33
parent 2e8c7670

core/java/android/util/Base64OutputStream.java

+3 −1
Original line number Diff line number Diff line
@@ -117,8 +117,10 @@ public class Base64OutputStream extends FilterOutputStream { 
                out.flush();

            }

        } catch (IOException e) {

            if (thrown != null) {

            if (thrown == null) {

                thrown = e;

            } else {

                thrown.addSuppressed(e);

            }

        }

core/tests/coretests/src/android/util/Base64Test.java

+76 −1
Original line number Diff line number Diff line
@@ -18,13 +18,18 @@ package android.util; 


import android.support.test.filters.LargeTest;



import junit.framework.TestCase;



import java.io.ByteArrayInputStream;

import java.io.ByteArrayOutputStream;

import java.io.IOException;

import java.io.InputStream;

import java.io.OutputStream;

import java.util.Arrays;

import java.util.Collections;

import java.util.List;

import java.util.Random;

import junit.framework.TestCase;

import java.util.stream.Collectors;



@LargeTest

public class Base64Test extends TestCase {
@@ -530,4 +535,74 @@ public class Base64Test extends TestCase { 
            assertEquals(plain, actual);

        }

    }



    public void testOutputStream_ioExceptionDuringClose() {

        OutputStream out = new OutputStream() {

            @Override public void write(int b) throws IOException { }

            @Override public void close() throws IOException {

                throw new IOException("close()");

            }

        };

        OutputStream out2 = new Base64OutputStream(out, Base64.DEFAULT);

        try {

            out2.close();

            fail();

        } catch (IOException expected) {

        }

    }



    public void testOutputStream_ioExceptionDuringCloseAndWrite() {

        OutputStream out = new OutputStream() {

            @Override public void write(int b) throws IOException {

                throw new IOException("write()");

            }

            @Override public void write(byte[] b) throws IOException {

                throw new IOException("write()");

            }

            @Override public void write(byte[] b, int off, int len) throws IOException {

                throw new IOException("write()");

            }

            @Override public void close() throws IOException {

                throw new IOException("close()");

            }

        };

        OutputStream out2 = new Base64OutputStream(out, Base64.DEFAULT);

        try {

            out2.close();

            fail();

        } catch (IOException expected) {

            // Base64OutputStream write()s pending (possibly empty) data

            // before close(), so the IOE from write() should be thrown and

            // any later exception suppressed.

            assertEquals("write()", expected.getMessage());

            Throwable[] suppressed = expected.getSuppressed();

            List<String> suppressedMessages = Arrays.asList(suppressed).stream()

                    .map((e) -> e.getMessage())

                    .collect(Collectors.toList());

            assertEquals(Collections.singletonList("close()"), suppressedMessages);

        }

    }



    public void testOutputStream_ioExceptionDuringWrite() {

        OutputStream out = new OutputStream() {

            @Override public void write(int b) throws IOException {

                throw new IOException("write()");

            }

            @Override public void write(byte[] b) throws IOException {

                throw new IOException("write()");

            }

            @Override public void write(byte[] b, int off, int len) throws IOException {

                throw new IOException("write()");

            }

        };

        // Base64OutputStream write()s pending (possibly empty) data

        // before close(), so the IOE from write() should be thrown.

        OutputStream out2 = new Base64OutputStream(out, Base64.DEFAULT);

        try {

            out2.close();

            fail();

        } catch (IOException expected) {

        }

    }



}