FasterXML / FasterXML/jackson-core

File/Path generator construction can leak or bypass decorated output cleanup

Offen
#1,711 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
Vorherrschende Sprache
Java
Sterne
2.4k
Forks
928
Ø Merge
2 T. 18 Std.
Gemergte PRs (30 T.)
24

Beschreibung

## 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.

Beitragsleitfaden

Für dieses Repository ist kein Beitragsleitfaden indexiert

Bewertung

Dieses Issue wurde noch nicht bewertet.

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.