Skip to content

Commit

Permalink
Fix #764
Browse files Browse the repository at this point in the history
  • Loading branch information
@cowtowncoder
cowtowncoder committed Jun 13, 2022
1 parent 83c5694 commit 96657ef
Show file tree
Hide file tree
Showing 5 changed files with 139 additions and 39 deletions.
1 change: 1 addition & 0 deletions release-notes/VERSION-2.x
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
55 changes: 36 additions & 19 deletions src/main/java/com/fasterxml/jackson/core/json/UTF8JsonGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
Expand Down
50 changes: 34 additions & 16 deletions src/main/java/com/fasterxml/jackson/core/json/WriterBasedJsonGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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
Expand Down
8 changes: 4 additions & 4 deletions src/test/java/com/fasterxml/jackson/core/json/InputStreamInitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
Expand All @@ -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);
Expand All @@ -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);
Expand Down
64 changes: 64 additions & 0 deletions src/test/java/com/fasterxml/jackson/core/json/OutputStreamInitTest.java
View file
Original file line number Diff line number Diff line change
@@ -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);
}

}

0 comments on commit 96657ef

Please sign in to comment.