bitflicker64 commented on code in PR #3290:
URL: https://github.com/apache/hugegraph/pull/3290#discussion_r4237664246


##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/cypher/CypherAPI.java:
##########
@@ -221,7 +221,7 @@ private Pair<String, String> toUserPass(String auth) {
             String encoded = auth.substring(basic.length());
             byte[] userPass = this.decoder.decode(encoded);

Review Comment:
   Important: this decoder is `Base64.getUrlDecoder()` (line 70), so the Cypher 
endpoint still answers 500 for many of the passwords this PR makes usable on 
REST.
   
   Evidence, measured at this head on a RocksDB server configured as in 
`run-api-test.sh`:
   - A Basic header carries standard Base64 (RFC 7617, RFC 4648 section 4), 
whose alphabet includes `+` and `/`. The URL-safe decoder throws 
`IllegalArgumentException` on both, so `toUserPass` returns null and 
`clientViaBasic` fails in `E.checkNotNull`.
   - User `probe`, password `密码pass1`, header `Basic cHJvYmU65a+G56CBcGFzczE=`: 
`GET graphspaces/DEFAULT/graphs` answers 200, the Cypher endpoint answers 500 
with `The 'user-password-pair' can't be null`. User `probe1` with the same 
password (`cHJvYmUxOuWvhueggXBhc3Mx`, no `+`) passes Cypher authentication.
   - User `probe`, password `a:b:c~` (`cHJvYmU6YTpiOmN+`): 200 on REST, the 
same 500 on Cypher.
   - UTF-8 continuation bytes make `+` and `/` frequent: of 100000 random 
6-character CJK passwords for `admin`, 43845 produce a header this decoder 
rejects.
   
   The decoder predates this PR, and an ASCII password can hit it as well (`~`, 
`?` or `>` as the last byte of a 3-byte group). Until this change, though, a 
non-ASCII password never got past the filter, so this path was not reachable 
with one. The new tests pass because `cypher_colon_user:a:b:c` happens to 
encode without `+` or `/`.
   
   Requested change: change the `decoder` field on line 70 to 
`Base64.getDecoder()`, as `WsAndHttpBasicAuthHandler` uses, and have 
`testBasicAuthWithColonPassword` use a credential whose Base64 contains `+` or 
`/`, for example the password `a:b:c~` (`cypher_colon_user:a:b:c~` encodes to 
`Y3lwaGVyX2NvbG9uX3VzZXI6YTpiOmN+`).



-- 
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]

Reply via email to