This is an automated email from the ASF dual-hosted git repository. garydgregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-codec.git
commit 0130064aa803637090adff8beea3c33d2c44708d Author: Gary Gregory <[email protected]> AuthorDate: Fri Sep 18 05:52:32 2026 -0700 Ensure BaseNCodecOutputStream closes underlying resources on failure Close the underlying stream even when final conversion or flushing fails. Preserve the original exception and suppress any additional close failure. Add regression tests for cleanup and exception handling. --- src/changes/changes.xml | 1 + .../codec/binary/BaseNCodecOutputStream.java | 10 ++- .../codec/binary/BaseNCodecOutputStreamTest.java | 78 ++++++++++++++++++++++ 3 files changed, 86 insertions(+), 3 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 25e3558f..8e3a2410 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -58,6 +58,7 @@ The <action> type attribute can be add,update,fix,remove. <action type="fix" dev="ggregory" due-to="Gary Gregory">Limit Base58 decoding to 8192 encoded bytes by default, checking cumulative stream input before buffering. Use Base58.Builder.setMaxDecodeLength(int) for larger trusted input. Encoding defaults to a configurable 8192-byte binary input limit through Base58.Builder.setMaxEncodeLength(int).</action> <action type="fix" dev="ggregory" due-to="Gary Gregory">Grow Base58 accumulation buffers geometrically within the configured input limits and validate actual accumulated length.</action> <action type="fix" dev="ggregory" due-to="Gary Gregory">Implement Base58 encoded-length calculation and explicitly reject unsupported line chunking.</action> + <action type="fix" dev="ggregory" due-to="Gary Gregory">Close the underlying BaseNCodecOutputStream output even when final conversion or flushing fails, preserving suppressed close exceptions.</action> <!-- ADD --> <action type="add" dev="ggregory" due-to="Gary Gregory">Add and use PhoneticEngine.Builder and deprecate old constructors.</action> <action type="add" dev="ggregory" due-to="Gary Gregory">Add BeiderMorseEncoder.Builder and deprecate old constructor.</action> diff --git a/src/main/java/org/apache/commons/codec/binary/BaseNCodecOutputStream.java b/src/main/java/org/apache/commons/codec/binary/BaseNCodecOutputStream.java index d037c1e6..49993819 100644 --- a/src/main/java/org/apache/commons/codec/binary/BaseNCodecOutputStream.java +++ b/src/main/java/org/apache/commons/codec/binary/BaseNCodecOutputStream.java @@ -123,6 +123,9 @@ public class BaseNCodecOutputStream<C extends BaseNCodec, T extends BaseNCodecOu /** * Closes this output stream and releases any system resources associated with the stream. * <p> + * The underlying stream is closed even if final conversion or flushing fails. If closing also fails, its exception is suppressed on the original exception. + * </p> + * <p> * To write the EOF marker without closing the stream, call {@link #eof()} or use an <a href="https://commons.apache.org/proper/commons-io/";>Apache Commons * IO</a> * <a href= "https://commons.apache.org/proper/commons-io/apidocs/org/apache/commons/io/output/CloseShieldOutputStream.html"; >CloseShieldOutputStream</a>. @@ -132,9 +135,10 @@ public class BaseNCodecOutputStream<C extends BaseNCodec, T extends BaseNCodecOu */ @Override public void close() throws IOException { - eof(); - flush(); - out.close(); + try (OutputStream outputStream = out) { // NOPMD + eof(); + flush(); + } } /** diff --git a/src/test/java/org/apache/commons/codec/binary/BaseNCodecOutputStreamTest.java b/src/test/java/org/apache/commons/codec/binary/BaseNCodecOutputStreamTest.java index b044961c..366e91ec 100644 --- a/src/test/java/org/apache/commons/codec/binary/BaseNCodecOutputStreamTest.java +++ b/src/test/java/org/apache/commons/codec/binary/BaseNCodecOutputStreamTest.java @@ -17,9 +17,17 @@ package org.apache.commons.codec.binary; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + import java.io.ByteArrayOutputStream; +import java.io.IOException; import java.io.OutputStream; +import org.junit.jupiter.api.Test; + /** * Tests {@link BaseNCodecOutputStream}. */ @@ -29,4 +37,74 @@ public class BaseNCodecOutputStreamTest extends AbstractBaseNOutputStreamTest { OutputStream newOutputStream() { return new BaseNCodecOutputStream<>(new ByteArrayOutputStream(), new NoOpBaseNCodec(), true); } + + @Test + void testCloseAfterInvalidBase58() throws IOException { + final boolean[] closed = { false }; + final ByteArrayOutputStream sink = new ByteArrayOutputStream() { + @Override + public void close() { + closed[0] = true; + } + }; + final Base58OutputStream stream = Base58OutputStream.builder().setOutputStream(sink).setEncode(false).get(); + stream.write('0'); + final IOException failure = assertThrows(IOException.class, stream::close); + assertTrue(failure.getCause() instanceof IllegalArgumentException); + assertTrue(closed[0]); + assertEquals(0, sink.size()); + } + + @Test + void testCloseFailureAfterSuccessfulConversion() throws IOException { + final IOException closeFailure = new IOException("close"); + final ByteArrayOutputStream sink = new ByteArrayOutputStream() { + @Override + public void close() throws IOException { + throw closeFailure; + } + }; + final Base58OutputStream stream = new Base58OutputStream(sink); + stream.write(1); + assertSame(closeFailure, assertThrows(IOException.class, stream::close)); + assertEquals("2", sink.toString("US-ASCII")); + } + + @Test + void testCloseFailureSuppressedAfterFlushFailure() { + final IOException flushFailure = new IOException("flush"); + final IOException closeFailure = new IOException("close"); + final ByteArrayOutputStream sink = new ByteArrayOutputStream() { + @Override + public void close() throws IOException { + throw closeFailure; + } + + @Override + public void flush() throws IOException { + throw flushFailure; + } + }; + final Base64OutputStream stream = new Base64OutputStream(sink); + assertSame(flushFailure, assertThrows(IOException.class, stream::close)); + assertEquals(1, flushFailure.getSuppressed().length); + assertSame(closeFailure, flushFailure.getSuppressed()[0]); + } + + @Test + void testCloseFailureSuppressedAfterInvalidBase58() throws IOException { + final IOException closeFailure = new IOException("close"); + final ByteArrayOutputStream sink = new ByteArrayOutputStream() { + @Override + public void close() throws IOException { + throw closeFailure; + } + }; + final Base58OutputStream stream = Base58OutputStream.builder().setOutputStream(sink).setEncode(false).get(); + stream.write('0'); + final IOException failure = assertThrows(IOException.class, stream::close); + assertTrue(failure.getCause() instanceof IllegalArgumentException); + assertEquals(1, failure.getSuppressed().length); + assertSame(closeFailure, failure.getSuppressed()[0]); + } }
