From 96657ef4d016ff6bbef4e2732ad9f90388a54f9b Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Sun, 12 Jun 2022 18:57:11 -0700 Subject: [PATCH] Fix #764 --- release-notes/VERSION-2.x | 1 + .../jackson/core/json/UTF8JsonGenerator.java | 55 ++++++++++------ .../core/json/WriterBasedJsonGenerator.java | 50 ++++++++++----- .../core/json/InputStreamInitTest.java | 8 +-- .../core/json/OutputStreamInitTest.java | 64 +++++++++++++++++++ 5 files changed, 139 insertions(+), 39 deletions(-) create mode 100644 src/test/java/com/fasterxml/jackson/core/json/OutputStreamInitTest.java diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index 5127f15404..dc78455b35 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -33,6 +33,7 @@ JSON library. #759: JsonGenerator to provide current value to the context before starting objects (reported by Illia O) #763: `JsonFactory.createParser()` with `File` may leak `InputStream`s +#764: `JsonFactory.createGenerator()` with `File` may leak `OutputStream`s 2.13.3 (14-May-2022) diff --git a/src/main/java/com/fasterxml/jackson/core/json/UTF8JsonGenerator.java b/src/main/java/com/fasterxml/jackson/core/json/UTF8JsonGenerator.java index 1dfe6100d0..29ea90b972 100644 --- a/src/main/java/com/fasterxml/jackson/core/json/UTF8JsonGenerator.java +++ b/src/main/java/com/fasterxml/jackson/core/json/UTF8JsonGenerator.java @@ -1199,24 +1199,30 @@ public void close() throws IOException { super.close(); - /* 05-Dec-2008, tatu: To add [JACKSON-27], need to close open - * scopes. - */ + // 05-Dec-2008, tatu: To add [JACKSON-27], need to close open scopes. // First: let's see that we still have buffers... - if ((_outputBuffer != null) - && isEnabled(Feature.AUTO_CLOSE_JSON_CONTENT)) { - while (true) { - JsonStreamContext ctxt = getOutputContext(); - if (ctxt.inArray()) { - writeEndArray(); - } else if (ctxt.inObject()) { - writeEndObject(); - } else { - break; + IOException flushFail = null; + try { + if ((_outputBuffer != null) + && isEnabled(Feature.AUTO_CLOSE_JSON_CONTENT)) { + while (true) { + JsonStreamContext ctxt = getOutputContext(); + if (ctxt.inArray()) { + writeEndArray(); + } else if (ctxt.inObject()) { + writeEndObject(); + } else { + break; + } } } + _flushBuffer(); + } catch (IOException e) { + // 10-Jun-2022, tatu: [core#764] Need to avoid failing here; may + // still need to close the underlying output stream + flushFail = e; } - _flushBuffer(); + _outputTail = 0; // just to ensure we don't think there's anything buffered /* 25-Nov-2008, tatus: As per [JACKSON-16] we are not to call close() @@ -1226,15 +1232,26 @@ && isEnabled(Feature.AUTO_CLOSE_JSON_CONTENT)) { * may not be properly recycled if we don't close the writer. */ if (_outputStream != null) { - if (_ioContext.isResourceManaged() || isEnabled(Feature.AUTO_CLOSE_TARGET)) { - _outputStream.close(); - } else if (isEnabled(Feature.FLUSH_PASSED_TO_STREAM)) { - // If we can't close it, we should at least flush - _outputStream.flush(); + try { + if (_ioContext.isResourceManaged() || isEnabled(Feature.AUTO_CLOSE_TARGET)) { + _outputStream.close(); + } else if (isEnabled(Feature.FLUSH_PASSED_TO_STREAM)) { + // If we can't close it, we should at least flush + _outputStream.flush(); + } + } catch (IOException | RuntimeException e) { + if (flushFail != null) { + e.addSuppressed(flushFail); + } + throw e; } } // Internal buffer(s) generator has can now be released as well _releaseBuffers(); + + if (flushFail != null) { + throw flushFail; + } } @Override diff --git a/src/main/java/com/fasterxml/jackson/core/json/WriterBasedJsonGenerator.java b/src/main/java/com/fasterxml/jackson/core/json/WriterBasedJsonGenerator.java index a5b7c6a1b0..bf8c63eac7 100644 --- a/src/main/java/com/fasterxml/jackson/core/json/WriterBasedJsonGenerator.java +++ b/src/main/java/com/fasterxml/jackson/core/json/WriterBasedJsonGenerator.java @@ -981,20 +981,27 @@ public void close() throws IOException // 05-Dec-2008, tatu: To add [JACKSON-27], need to close open scopes // First: let's see that we still have buffers... - if (_outputBuffer != null - && isEnabled(Feature.AUTO_CLOSE_JSON_CONTENT)) { - while (true) { - JsonStreamContext ctxt = getOutputContext(); - if (ctxt.inArray()) { - writeEndArray(); - } else if (ctxt.inObject()) { - writeEndObject(); - } else { - break; + IOException flushFail = null; + try { + if ((_outputBuffer != null) + && isEnabled(Feature.AUTO_CLOSE_JSON_CONTENT)) { + while (true) { + JsonStreamContext ctxt = getOutputContext(); + if (ctxt.inArray()) { + writeEndArray(); + } else if (ctxt.inObject()) { + writeEndObject(); + } else { + break; + } } } + _flushBuffer(); + } catch (IOException e) { + // 10-Jun-2022, tatu: [core#764] Need to avoid failing here; may + // still need to close the underlying output stream + flushFail = e; } - _flushBuffer(); _outputHead = 0; _outputTail = 0; @@ -1005,15 +1012,26 @@ && isEnabled(Feature.AUTO_CLOSE_JSON_CONTENT)) { * may not be properly recycled if we don't close the writer. */ if (_writer != null) { - if (_ioContext.isResourceManaged() || isEnabled(Feature.AUTO_CLOSE_TARGET)) { - _writer.close(); - } else if (isEnabled(Feature.FLUSH_PASSED_TO_STREAM)) { - // If we can't close it, we should at least flush - _writer.flush(); + try { + if (_ioContext.isResourceManaged() || isEnabled(Feature.AUTO_CLOSE_TARGET)) { + _writer.close(); + } else if (isEnabled(Feature.FLUSH_PASSED_TO_STREAM)) { + // If we can't close it, we should at least flush + _writer.flush(); + } + } catch (IOException | RuntimeException e) { + if (flushFail != null) { + e.addSuppressed(flushFail); + } + throw e; } } // Internal buffer(s) generator has can now be released as well _releaseBuffers(); + + if (flushFail != null) { + throw flushFail; + } } @Override diff --git a/src/test/java/com/fasterxml/jackson/core/json/InputStreamInitTest.java b/src/test/java/com/fasterxml/jackson/core/json/InputStreamInitTest.java index 01f09adbad..ede1a98d35 100644 --- a/src/test/java/com/fasterxml/jackson/core/json/InputStreamInitTest.java +++ b/src/test/java/com/fasterxml/jackson/core/json/InputStreamInitTest.java @@ -31,12 +31,12 @@ static class FailingJsonFactory extends JsonFactory { public FailingInputStream lastStream; @Override - protected InputStream _fileInputStream(File f) throws IOException { + protected InputStream _fileInputStream(File f) { return (lastStream = new FailingInputStream()); } @Override - protected InputStream _optimizedStreamFromURL(URL url) throws IOException { + protected InputStream _optimizedStreamFromURL(URL url) { return (lastStream = new FailingInputStream()); } } @@ -47,7 +47,7 @@ public void testForFile() throws Exception try { /*JsonParser p =*/ jsonF.createParser(new File("/tmp/test.json")); fail("Should not pass"); - } catch (IOException e) { + } catch (Exception e) { verifyException(e, "Will not read"); } assertNotNull(jsonF.lastStream); @@ -60,7 +60,7 @@ public void testForURL() throws Exception try { /*JsonParser p =*/ jsonF.createParser(new URL("http://localhost:80/")); fail("Should not pass"); - } catch (IOException e) { + } catch (Exception e) { verifyException(e, "Will not read"); } assertNotNull(jsonF.lastStream); diff --git a/src/test/java/com/fasterxml/jackson/core/json/OutputStreamInitTest.java b/src/test/java/com/fasterxml/jackson/core/json/OutputStreamInitTest.java new file mode 100644 index 0000000000..cb80c85127 --- /dev/null +++ b/src/test/java/com/fasterxml/jackson/core/json/OutputStreamInitTest.java @@ -0,0 +1,64 @@ +package com.fasterxml.jackson.core.json; + +import java.io.*; + +import com.fasterxml.jackson.core.JsonEncoding; +import com.fasterxml.jackson.core.JsonFactory; +import com.fasterxml.jackson.core.JsonGenerator; + +//[core#764] (and [databind#3508] +public class OutputStreamInitTest + extends com.fasterxml.jackson.core.BaseTest +{ + static class FailingOutputStream extends OutputStream { + public int written = 0; + public boolean failWrites = false; + public boolean closed = false; + + public void startFailingWrites() { + failWrites = true; + } + + @Override + public void close() { + closed = true; + } + + @Override + public void write(int b) throws IOException { + ++written; + if (failWrites) { + throw new IOException("No writes!"); + } + } + } + + static class FailingJsonFactory extends JsonFactory { + private static final long serialVersionUID = 1L; + + public FailingOutputStream lastStream; + + @Override + protected OutputStream _fileOutputStream(File f) { + return (lastStream = new FailingOutputStream()); + } + } + + public void testForFile() throws Exception + { + final FailingJsonFactory jsonF = new FailingJsonFactory(); + try { + JsonGenerator g = jsonF.createGenerator(new File("/tmp/test.json"), + JsonEncoding.UTF8); + g.writeString("foo"); + jsonF.lastStream.startFailingWrites(); + g.close(); + fail("Should not pass"); + } catch (Exception e) { + verifyException(e, "No writes"); + } + assertNotNull(jsonF.lastStream); + assertTrue(jsonF.lastStream.closed); + } + +}