seebee-dev opened a new pull request, #49765: URL: https://github.com/apache/arrow/pull/49765
### Rationale for this change `gdv_hash_using_openssl()` has three bugs reported in #49752: an unnecessarily exported internal symbol, a logic error in digest validation, and a buffer size miscalculation in `snprintf`. ### What changes are included in this PR? All changes are in `cpp/src/gandiva/hash_utils.cc` and `hash_utils.h`: - Removed `GANDIVA_EXPORT` from `gdv_hash_using_openssl` — it's only called by the four public hash wrappers (`gdv_sha512_hash`, `gdv_sha256_hash`, `gdv_sha1_hash`, `gdv_md5_hash`) within the same translation unit, so there's no reason to export it. - Changed `&&` to `||` on the validity check after `EVP_DigestFinal_ex`. The original condition only triggered an error when *both* `result_length != hash_digest_size` and `result_buf_size != (2 * hash_digest_size)` were true. Either mismatch alone indicates something went wrong. - Fixed the second argument to `snprintf` from `result_buf_size` to `result_buf_size - result_buff_index`, so it reflects the actual remaining buffer space rather than the total allocation. ### Are these changes tested? The existing tests for `gdv_sha1_hash`, `gdv_sha256_hash`, `gdv_sha512_hash`, and `gdv_md5_hash` exercise `gdv_hash_using_openssl` through the public API. The fixes correct error handling paths and symbol visibility without changing the happy-path output, so no new tests are needed. ### Are there any user-facing changes? No. `gdv_hash_using_openssl` was internal — removing its export shouldn't affect any downstream consumers. The logic and buffer fixes only affect error/edge-case handling. **This PR contains a "Critical Fix".** The `snprintf` buffer size bug could allow writing past the allocated region when converting hash digests to hex. The `&&`/`||` logic error could silently skip validation in error states. **AI disclosure:** I used an AI assistant (Claude) to help analyze the reported bugs and draft the initial fix. I've reviewed each change against the OpenSSL EVP API docs and verified the logic independently. Apologies for the previous submission (#49755) that didn't follow the contribution guidelines — lesson learned. -- 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]
