SEPURI-SAI-KRISHNA commented on PR #12595: URL: https://github.com/apache/seatunnel/pull/12595#issuecomment-6105527206
Thanks @DanielLeens, and thank you for disassembling it rather than letting my sentence stand. You are right, and the way I was wrong is worse than the sentence: **there is no JDK 8 on this machine**, so "Verified against the OpenJDK 8u source" was never something I could have checked. I wrote a security claim I had no means to test. Taking **Option A**. The javadoc now states only what was measured, and names the build each claim came from: - `javap -c` here on **11.0.32, 17.0.20 and 21.0.12**: the length difference is folded into the accumulator (`isub` then `ior`) and the loop bound is the **first** argument's length (`istore_2`, then `if_icmpge`), with no early return on differing lengths. That also confirms the argument-order reasoning. - For **8u71 and 8u171** it attributes the `if_icmpeq` plus immediate `return false` to your disassembly rather than claiming it as mine, and says byte-length equality can still be inferred there while the leading bytes cannot. - It names **no boundary update**. My first attempt at this correction said "hidden only from 11.0.9 onwards", repeating the same mistake in miniature: 11.0.9 came from your review, not from anything I ran, and the build I disassembled is 11.0.32. It now says only that the listed builds were disassembled and that which 8u or 11.x update first changed shape was not established. One thing I found while checking, on every build I disassembled: a second early exit for an **empty configured value**. `isEqual` tests the second argument's length first (`iload_3`, `ifne`) and returns whether the provided value is empty too, without entering the loop. It only arises from an empty configured credential, which the empty-credential test already pins, so it is recorded in the javadoc rather than acted on. **On Option B** I would rather not, and that is yours to overrule. Digesting both sides hides the length on every JDK, but it adds a hash to an authenticated path for a property ASF Security already concluded is not a vulnerability here, and the digest's own runtime is input-length dependent, so it narrows the signal rather than removing it. Say so and I will push it. Comment-only: `git diff` has no non-comment line. `BasicAuthFilterTest` 8 tests 0F 0E, spotless clean. Agreed on the CI read, and thanks for pinning `all-connectors-it-2` to the Maven Central `Connection reset`. That matches the artifact side, where that job had no failing test reports at all. Syncing with `dev` for a fresh run. -- 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]
