You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@parquet.apache.org by "anxkhn (via GitHub)" <gi...@apache.org> on 2026/07/14 03:53:55 UTC
[PR] MINOR: Close output stream when ParquetFileWriter construction fails on encryption validation [parquet-java]
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] MINOR: Close output stream when ParquetFileWriter construction fails on encryption validation [parquet-java]
Posted by "anxkhn (via GitHub)" <gi...@apache.org>.
anxkhn commented on code in PR #3663:
URL: https://github.com/apache/parquet-java/pull/3663#discussion_r3659175384
##########
parquet-hadoop/src/test/java/org/apache/parquet/hadoop/TestParquetFileWriter.java:
##########
@@ -1498,6 +1505,94 @@ public void testMergeMetadataWithNoConflictingKeyValues() {
assertEquals("d", mergedValues.get("c"));
}
+ @Test
+ public void testConstructorClosesStreamWhenEncryptedColumnMissing() throws Exception {
+ ColumnEncryptionProperties missingColumn = ColumnEncryptionProperties.builder("not_in_schema")
+ .withKey("0123456789012345".getBytes(StandardCharsets.UTF_8))
+ .build();
+ Map<ColumnPath, ColumnEncryptionProperties> encryptedColumns = new HashMap<>();
+ encryptedColumns.put(missingColumn.getPath(), missingColumn);
+ FileEncryptionProperties encryptionProperties = FileEncryptionProperties.builder(
+ "0123456789012345".getBytes(StandardCharsets.UTF_8))
+ .withEncryptedColumns(encryptedColumns)
+ .build();
+
+ RecordingOutputFile file = new RecordingOutputFile();
+ try {
+ new ParquetFileWriter(
+ file,
+ SCHEMA,
+ CREATE,
+ DEFAULT_BLOCK_SIZE,
+ MAX_PADDING_SIZE_DEFAULT,
+ ParquetProperties.DEFAULT_COLUMN_INDEX_TRUNCATE_LENGTH,
+ ParquetProperties.DEFAULT_STATISTICS_TRUNCATE_LENGTH,
+ ParquetProperties.DEFAULT_PAGE_WRITE_CHECKSUM_ENABLED,
+ encryptionProperties);
+ fail("Expected ParquetCryptoRuntimeException for an encrypted column absent from the schema");
Review Comment:
rebased onto current master and retained the assertj assertion. the targeted test and full TestParquetFileWriter suite pass. thanks!
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] MINOR: Close output stream when ParquetFileWriter construction fails on encryption validation [parquet-java]
Posted by "anxkhn (via GitHub)" <gi...@apache.org>.
anxkhn commented on PR #3663:
URL: https://github.com/apache/parquet-java/pull/3663#issuecomment-4989710484
Addressed the review feedback, rebased onto the latest master, and reran the targeted test suite successfully.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] MINOR: Close output stream when ParquetFileWriter construction fails on encryption validation [parquet-java]
Posted by "dossett (via GitHub)" <gi...@apache.org>.
dossett commented on PR #3663:
URL: https://github.com/apache/parquet-java/pull/3663#issuecomment-5007010766
+1 (non-binding)
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] MINOR: Close output stream when ParquetFileWriter construction fails on encryption validation [parquet-java]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3663:
URL: https://github.com/apache/parquet-java/pull/3663#issuecomment-5841309013
This pull request has been automatically marked as stale because it has had no activity for at least 2 months. If you are still working on this change or plan to move it forward, please leave a comment or push a new commit so we know to keep it open. Otherwise, this PR will be closed automatically in about one month. Thank you for your contribution to Apache Parquet!
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] MINOR: Close output stream when ParquetFileWriter construction fails on encryption validation [parquet-java]
Posted by "anxkhn (via GitHub)" <gi...@apache.org>.
anxkhn commented on code in PR #3663:
URL: https://github.com/apache/parquet-java/pull/3663#discussion_r3593754023
##########
parquet-hadoop/src/test/java/org/apache/parquet/hadoop/TestParquetFileWriter.java:
##########
@@ -1498,6 +1505,94 @@ public void testMergeMetadataWithNoConflictingKeyValues() {
assertEquals("d", mergedValues.get("c"));
}
+ @Test
+ public void testConstructorClosesStreamWhenEncryptedColumnMissing() throws Exception {
+ ColumnEncryptionProperties missingColumn = ColumnEncryptionProperties.builder("not_in_schema")
+ .withKey("0123456789012345".getBytes(StandardCharsets.UTF_8))
+ .build();
+ Map<ColumnPath, ColumnEncryptionProperties> encryptedColumns = new HashMap<>();
+ encryptedColumns.put(missingColumn.getPath(), missingColumn);
+ FileEncryptionProperties encryptionProperties = FileEncryptionProperties.builder(
+ "0123456789012345".getBytes(StandardCharsets.UTF_8))
+ .withEncryptedColumns(encryptedColumns)
+ .build();
+
+ RecordingOutputFile file = new RecordingOutputFile();
+ try {
+ new ParquetFileWriter(
+ file,
+ SCHEMA,
+ CREATE,
+ DEFAULT_BLOCK_SIZE,
+ MAX_PADDING_SIZE_DEFAULT,
+ ParquetProperties.DEFAULT_COLUMN_INDEX_TRUNCATE_LENGTH,
+ ParquetProperties.DEFAULT_STATISTICS_TRUNCATE_LENGTH,
+ ParquetProperties.DEFAULT_PAGE_WRITE_CHECKSUM_ENABLED,
+ encryptionProperties);
+ fail("Expected ParquetCryptoRuntimeException for an encrypted column absent from the schema");
Review Comment:
Updated the exception assertion to AssertJ and included the expected message. Thanks.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] MINOR: Close output stream when ParquetFileWriter construction fails on encryption validation [parquet-java]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #3663: MINOR: Close output stream when ParquetFileWriter construction fails on encryption validation
URL: https://github.com/apache/parquet-java/pull/3663
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] MINOR: Close output stream when ParquetFileWriter construction fails on encryption validation [parquet-java]
Posted by "anxkhn (via GitHub)" <gi...@apache.org>.
anxkhn commented on PR #3663:
URL: https://github.com/apache/parquet-java/pull/3663#issuecomment-5007053710
rebased onto the latest master, consolidated the changes into one commit, and verified all 70 targeted and related tests plus spotless on both changed files
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] MINOR: Close output stream when ParquetFileWriter construction fails on encryption validation [parquet-java]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #3663:
URL: https://github.com/apache/parquet-java/pull/3663#discussion_r3585423347
##########
parquet-hadoop/src/test/java/org/apache/parquet/hadoop/TestParquetFileWriter.java:
##########
@@ -1498,6 +1505,94 @@ public void testMergeMetadataWithNoConflictingKeyValues() {
assertEquals("d", mergedValues.get("c"));
}
+ @Test
+ public void testConstructorClosesStreamWhenEncryptedColumnMissing() throws Exception {
+ ColumnEncryptionProperties missingColumn = ColumnEncryptionProperties.builder("not_in_schema")
+ .withKey("0123456789012345".getBytes(StandardCharsets.UTF_8))
+ .build();
+ Map<ColumnPath, ColumnEncryptionProperties> encryptedColumns = new HashMap<>();
+ encryptedColumns.put(missingColumn.getPath(), missingColumn);
+ FileEncryptionProperties encryptionProperties = FileEncryptionProperties.builder(
+ "0123456789012345".getBytes(StandardCharsets.UTF_8))
+ .withEncryptedColumns(encryptedColumns)
+ .build();
+
+ RecordingOutputFile file = new RecordingOutputFile();
+ try {
+ new ParquetFileWriter(
+ file,
+ SCHEMA,
+ CREATE,
+ DEFAULT_BLOCK_SIZE,
+ MAX_PADDING_SIZE_DEFAULT,
+ ParquetProperties.DEFAULT_COLUMN_INDEX_TRUNCATE_LENGTH,
+ ParquetProperties.DEFAULT_STATISTICS_TRUNCATE_LENGTH,
+ ParquetProperties.DEFAULT_PAGE_WRITE_CHECKSUM_ENABLED,
+ encryptionProperties);
+ fail("Expected ParquetCryptoRuntimeException for an encrypted column absent from the schema");
Review Comment:
we're migrating the codebase to AssertJ assertions, so you might want to change this to `assertThatThrownBy(() -> ...).isInstanceOf(ParquetCryptoRuntimeException.class).hasMessage(...)`
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] MINOR: Close output stream when ParquetFileWriter construction fails on encryption validation [parquet-java]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3663:
URL: https://github.com/apache/parquet-java/pull/3663#issuecomment-6405246009
Closing this pull request due to at least 3 months of inactivity. If you would like to continue the work, please feel free to reopen this pull request or open a new one. Thank you for your contribution to Apache Parquet!
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org