Adarsh-Me commented on PR #3192: URL: https://github.com/apache/hugegraph/pull/3192#issuecomment-5791789511
`b8801a6` takes @bitflicker64's simplification and answers all five threads. Short version: | finding | change | what fails without it | | --- | --- | --- | | `props_set` can write a value ending in an odd number of backslashes, which then swallows the next line | refuse it; the file is left byte-for-byte untouched | reverting the guard alone turns the suite red | | `authentication:` matched at any indentation, so a nested mapping decided the REST side | key must start at column 0; the indented-`authentication:` test is dropped and the nested case asserted instead | same | | `get-decoded` did not trim, so a trailing space produced a wrong `WARN` | the mode is deleted — nothing compares the two classes any more | n/a | | the post-startup backend read accepted only `=` | goes through `props.awk` and trims in bash, so `backend : hstore` reaches the partition-wait check | n/a (behaviour asserted in the suite) | | parse which authenticator the yaml names | don't: `check_auth_sides` decides by presence and refuses one-sided layouts | making the check always pass turns the suite red | Two things worth flagging rather than leaving in the diff: **The proposal as sketched reopened the fail-open.** `has_yaml_authentication_block` is true for any mapping that exists, so a mounted file with an `authentication:` block that names no authenticator plus `auth.authenticator` set in the REST file reads as `rest=1, yaml=1` → passes → snakeyaml resolves no authenticator → Gremlin on `AllowAllAuthenticator`, REST enforcing. The read is therefore three-valued (`none` / `named` / `nameless`) and `nameless` is refused by itself, before the comparison — still without ever reading the class. **Column 0 costs a test that used to pass.** The old suite asserted ` authentication:` opened the block; that and rejecting a nested mapping are the same bytes, so they cannot both hold. I dropped the permissive one: the shipped config puts the key at column 0 and TinkerPop resolves it as a top-level key, so the indented form was this script being more generous than what it models. Say so if you would rather keep the lenient reading and reject only the nested case by tracking the parent path. Honest state of the run, same caveat as before: this host is Git Bash, so the four assertions that need a real CR byte, a working `chmod`, or a symlink cannot execute here — the suite passes with exactly those four neutralised in a scratch copy, and the committed file is unchanged. Production delta is net **-56** lines of shell and awk; tests net **+105**, so the overall diff is not the ~250-line reduction the thread estimated. CI on `b8801a6` is `action_required` like every previous head on this fork PR, so nothing has executed it yet — could someone with write access approve the workflows, and take a look at the auth decision above? -- 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]
