imbajin commented on PR #3290: URL: https://github.com/apache/hugegraph/pull/3290#issuecomment-6100877127
Supplemental review at `577f4c3b74dccdd4ce8b9e6d35852639bad7d51d`: **7/10**. I would hold merge until the existing Cypher Basic-auth decoder issue is addressed and current-head CI passes. The REST UTF-8/first-colon fix is focused, and the added Cypher regression is useful. Its credential does not exercise the standard Base64 `+` or `/` alphabet, however. Two concrete cases are: - `cypher_colon_user:a:b:c~` → `Y3lwaGVyX2NvbG9uX3VzZXI6YTpiOmN+` - `cypher_colon_user:a:b:c?` → `Y3lwaGVyX2NvbG9uX3VzZXI6YTpiOmM/` Please use the standard Base64 decoder and add Cypher endpoint regressions for both cases, including wrong-password rejection. This complements the [existing inline discussion](https://github.com/apache/hugegraph/pull/3290#discussion_r4237664246), whose reviewer reports REST 200 versus Cypher 500. Shared parsing is optional; no broader refactor is required. Evidence boundary: I inspected the current source paths and verified these encodings; I did not rerun repository tests or live probes. The author's colon-password regression report and the reviewer's runtime probes remain their reported evidence. All five workflows at this exact head—License Checker, Server Memory CI, CodeQL, HugeGraph-Server CI and Server Compatibility CI—currently have `action_required`, so they do not establish a passing CI result. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
