FasterXML / FasterXML/jackson-core
File/Path generator construction can leak or bypass decorated output cleanup
- Dominant language
- Java
- Stars
- 2.4k
- Forks
- 928
- Avg merge
- 2d 18h
- Merged PRs (30d)
- 24
Description
## Description
While reviewing the recent failed-construction cleanup changes in `TextualTSFactory` and `BinaryTSFactory`, I noticed two remaining lifecycle gaps in the `File`/`Path` generator paths.
I added regression tests for these cases and confirmed that they currently fail on `3.x`.
### 1. Output stream is opened before `IOContext` creation
For example, `TextualTSFactory.createGenerator(File, JsonEncoding)` currently starts with:
```java
final OutputStream out = _fileOutputStream(f);
final IOContext ioCtxt = _createContext(_createContentReference(f), true, enc);
try {
// generator construction
} catch (RuntimeException e) {
_releaseOnFailedConstruction(ioCtxt, e);
_closeOnFailedConstruction(out, e);
throw e;
}
```
If `_fileOutputStream(f)` succeeds but `_createContext(...)` fails, the exception occurs before entering the protected construction block.
At that point the factory-created `OutputStream` has already been opened, but it is not closed.
The lifecycle is roughly:
```text
open OutputStream
↓
create IOContext
↓
failure
↓
cleanup block is never entered
↓
OutputStream remains open
```
This reproduces for both the `File` and `Path` generator paths.
One possible way to avoid this gap is to create the `IOContext` before opening the output stream, and then perform output resource acquisition inside the protected construction block.
One implementation detail to watch is `BinaryTSFactory.createGenerator(File, ...)`, which currently builds the content reference from the opened `OutputStream`. If the stream open is moved inside the guarded block, that path may need to use the `File` as the content reference instead, matching the textual factory.
### 2. Decorated output is not closed when later construction fails
There is another ownership transition after output decoration.
Currently the decorated stream can be passed directly into generator construction:
```java
_createUTF8Generator(writeCtxt, ioCtxt, _decorate(ioCtxt, out))
```
If the `OutputDecorator` successfully returns a wrapper and generator construction subsequently fails, the failure handler closes the original `out`:
```java
_closeOnFailedConstruction(out, e);
```
but not the decorated output returned by the `OutputDecorator`.
For a decorator that owns additional state or resources which are released by its `close()` implementation, closing only the underlying stream bypasses the wrapper's cleanup.
The lifecycle in this case is:
```text
factory OutputStream
↓
OutputDecorator
↓
decorated OutputStream
↓
generator construction fails
↓
original OutputStream closed
decorated OutputStream not closed
```
Once decoration succeeds, the decorated output is the resource passed downstream and should be the resource cleaned up if later generator construction fails.
If decoration itself fails before returning a wrapper, the original factory-created output remains the resource that needs to be closed.
### Reproduction
I added regression tests covering:
- `File` output when `IOContext` creation fails
- `Path` output when `IOContext` creation fails
- decorated `File` output when later generator construction fails
All three currently fail on `3.x` with the expected cleanup assertions.
```text
Tests run: 4, Failures: 3
```
### Affected paths
Affected paths appear to include:
- `TextualTSFactory.createGenerator(File, JsonEncoding)`
- `TextualTSFactory.createGenerator(Path, JsonEncoding)`
- `BinaryTSFactory.createGenerator(File, JsonEncoding)`
- `BinaryTSFactory.createGenerator(Path, JsonEncoding)`
### Expected behavior
`File`/`Path` generator construction should clean up the resource currently owned by the construction path at every failure point:
- If `IOContext` creation fails, no factory-created output stream should be left open.
- If output stream creation fails after an `IOContext` has been created, the context should be released.
- If output decoration fails, the original factory-created output stream should be closed.
- If decoration succeeds but later generator construction fails, the decorated output should be closed.
- `IOContext` instances should be released on failed construction.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.