anxkhn opened a new pull request, #3663:
URL: https://github.com/apache/parquet-java/pull/3663
### Rationale for this change
The shared private `ParquetFileWriter` constructor opens the output stream
early
(`OutputFile.create` / `createOrOverwrite`), and only afterwards validates
that
every column named in the encryption properties actually exists in the file
schema. When a configured encrypted column is missing, it throws
`ParquetCryptoRuntimeException`.
That validation runs after the stream is opened and is not guarded, so on the
throw the half-constructed writer is never returned to the caller. Its
`close()`
is the only place that closes the stream, and it can never run, so the open
stream (and its underlying file/DFS handle and buffers) leaks. A service that
repeatedly constructs writers against a misconfigured encryption schema
accumulates leaked handles until exhaustion.
This is the same acquire-then-throw hazard the sibling `ParquetFileReader`
constructor already guards against by closing its stream before rethrowing,
so
the writer should behave consistently.
### What changes are included in this PR?
- In `ParquetFileWriter`, wrap the encryption-setup and
encrypted-column-in-schema
validation block (which runs after `this.out` is assigned) in a
`try { ... } catch (Exception e) { out.close(); throw e; }`. On any
exception
the already-opened stream is closed before the exception propagates,
mirroring
the existing guard in the `ParquetFileReader` constructor. The early
return for
the no-encryption case is left outside the guard (nothing to clean up
there).
- Add a regression test,
`testConstructorClosesStreamWhenEncryptedColumnMissing`,
that constructs a writer with an encrypted column absent from the schema,
asserts the expected `ParquetCryptoRuntimeException`, and asserts the
stream was
closed. It uses a small `RecordingOutputFile` whose `PositionOutputStream`
flips an `AtomicBoolean` on `close()`, so it needs no disk I/O.
Two files change: `ParquetFileWriter.java` (the guard) and
`TestParquetFileWriter.java` (the test plus helper).
### Are these changes tested?
Yes.
- The new test is red without the source fix (the stream is left open) and
green
with it. It runs parameterized (vectored true/false), 2/2 passing.
- Regression sweep passes: `TestParquetFileWriter` (38), `TestParquetWriter`
(19), `TestEncryptionOptions` (1), 0 failures / 0 errors.
- Spotless is clean on both changed files.
Commands used:
```
mvn -q -pl parquet-hadoop -am -DskipTests -Dspotless.check.skip=true install
mvn -pl parquet-hadoop
-Dtest=TestParquetFileWriter#testConstructorClosesStreamWhenEncryptedColumnMissing
test
mvn -pl parquet-hadoop
-Dtest=TestParquetFileWriter,TestParquetWriter,TestEncryptionOptions test
```
### Are there any user-facing changes?
No. Behavior on the success path is unchanged; the only difference is that a
failed writer construction (invalid encrypted-column configuration) now
releases
the output stream it opened instead of leaking it. No public API changes.
<!-- No GitHub issue filed; using MINOR: per the PR template. -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
