SEZ9 commented on PR #12595: URL: https://github.com/apache/seatunnel/pull/12595#issuecomment-5976189078
Thanks @SEPURI-SAI-KRISHNA, that settles the JDK 8 question. The `jdk8u` source you pasted has the same shape as what you saw in the 17 bytecode: length difference folded into `result`, loop runs `lenA` times, branchless index for the shorter array. No early exit on length mismatch, so the javadoc claim stands as written on the Java 8 target. Agreed on skipping the SHA-256-then-compare variant. A digest per request to hide the length of a credential that is already compared in constant time on both JDKs we ship against is not worth the extra moving part. On the caveats: - The `lenB == 0` early return is fine to leave as is. An empty configured username or password is a misconfiguration, not a secret, and the null guard already covers the unset case. I would not add a code path for it. - Yes, please add the one-line javadoc note that the constant-time property was verified against `jdk8u` and 17. It is cheap and it saves the next reader from re-deriving this. On CI, thanks for the breakdown. `BasicAuthFilterTest` passing 6 of 6 on all four unit-test legs and `BasicAuthenticationIT` reporting 9 run / 0 failures in `engine-v2-it (8)` is what I wanted to see; the `seatunnel-flink-starter-common` dependency resolution failure on the Windows job is clearly infra. Ping me once the rerun finishes so I can confirm it is green across the board. Remaining asks before merge: 1. Javadoc line recording the 8u / 17 verification. 2. Green rerun of the previously failed job. Nothing else from my side. <!-- streview-comment:1509 --> -- 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]
