sankalpsthakur opened a new pull request, #1390: URL: https://github.com/apache/arrow-go/pull/1390
### Rationale for this change arrow-go cannot read a Parquet file that parquet-mr (Spark, Hive, Iceberg, anything on parquet-java) wrote with modular encryption. `file.OpenParquetFile` succeeds and the first page read fails: ``` decrypting data page: encrypted payload length prefix is 4204214195, available length is 40: unexpected EOF ``` I hit it with `data/external_key_material_java.parquet.encrypted` from parquet-testing. parquet-mr zero-pads every encrypted thrift module to the size of its buffer: `Util.java` encrypts the whole backing array of its `TMemoryBuffer`, not only the bytes written. So an encrypted page header is the thrift struct followed by zeros, all inside one length-prefixed module. `readPageHeader` (`parquet/file/page_reader.go:796`) decrypts the module, parses the thrift, and then advances the stream by the amount of thrift it consumed plus the cipher overhead (line 837). That is only right when nothing follows the struct. With padding it stops part way through the module, and the next read takes ciphertext for page data. Files from arrow-go and from C++ (the ones in parquet-testing) are not padded, which is why the existing tests never saw it. ### What changes are included in this PR? For an encrypted header, `readPageHeader` now skips the whole framed ciphertext (the 4-byte length prefix plus the module, which `encryption.FramedCiphertext` already returns). The unencrypted path is unchanged. ### Are these changes tested? Yes. Both new tests fail on main and pass with the change. - `TestDecryptParquetMrExternalKeyMaterial` (`parquet/encryption_read_config_test.go`) reads the parquet-testing file above and expects 100 rows and 2 columns. That file uses external key material with double wrapping and arrow-go has no key toolkit, so the test carries a small key retriever that unwraps the keys with the same master keys the C++ tests use (`kf`, `kc1`, `kc2`). On main it fails with the error above. - `TestReadEncryptedPageHeaderWithTrailingPadding` (`parquet/file/page_reader_internal_test.go`) encrypts a page header whose plaintext is the thrift struct plus 76 zero bytes, puts page data after it, and checks that `readPageHeader` leaves the stream at the page data. On main it is left inside the module. Commands, with `PARQUET_TEST_DATA=$PWD/parquet-testing/data`: - `go test ./parquet -run 'TestFileEncryptionDecryption|TestDecryptParquetMrExternalKeyMaterial' -count=1`: ok (the existing C++-written configurations still pass) - `go test ./parquet/file -run 'TestReadEncryptedPageHeaderWithTrailingPadding|TestPageReader' -count=1`: ok - `go vet ./parquet ./parquet/file` and `gofmt -l` on the changed files: clean The fixture is the one in the parquet-testing submodule this repo already pins. Its README says it was encrypted with parquet-mr using external key material and the in-memory KMS client. No fixture files were added. ### Are there any user-facing changes? Yes. Encrypted files written by parquet-mr / parquet-java can be read. Nothing changes for files written by arrow-go or C++. Related to #566. That issue reports that cross-library encryption does not work in general; this covers the parquet-mr to Go read path only. ### AI/LLM disclosure - AI coding tools (Claude Code) were used to find and write this fix, the two tests and this description. - The failure was reproduced on main, the change was reviewed, and the commands above were run locally before submitting. -- 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]
