bitflicker64 commented on code in PR #3192:
URL: https://github.com/apache/hugegraph/pull/3192#discussion_r3995473971
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/enable-auth.sh:
##########
@@ -41,16 +41,43 @@ if [ ! -d "$BAK_CONF" ]; then
cp "${CONF}/${GREMLIN_SERVER_CONF}"
"${BAK_CONF}/${GREMLIN_SERVER_CONF}.bak"
cp "${CONF}/${REST_SERVER_CONF}" "${BAK_CONF}/${REST_SERVER_CONF}.bak"
cp "${CONF}/graphs/${GRAPH_CONF}" "${BAK_CONF}/${GRAPH_CONF}.bak"
+fi
+
+# The appends below are guarded per file and match only an absent or still
+# commented-out definition, so they are no-ops on any config that already
+# carries authentication (e.g. a mounted one, or a re-run of this script).
+# The guards accept every spelling java.util.Properties reads as the key —
+# '=' or ':' or bare-whitespace separators, leading whitespace and
+# backslash-escaped dots — and the gremlin.graph flip tolerates CRLF
+# endings, which a mounted config saved on Windows carries. Appending
+# unconditionally used to create duplicate definitions that the
+# properties parser (first definition wins) and the yaml parser (last wins)
+# resolved in opposite directions, leaving Gremlin and REST on different
+# authenticators.
+AUTHENTICATOR_CLASS="${AUTHENTICATOR_CLASS:-org.apache.hugegraph.auth.StandardAuthenticator}"
+if ! grep -Eq '^[[:blank:]]*authentication[[:blank:]]*:'
"${CONF}/${GREMLIN_SERVER_CONF}"; then
sed -i -e '$a\authentication: {' \
- -e '$a\ authenticator:
org.apache.hugegraph.auth.StandardAuthenticator,' \
+ -e "\$a\\ authenticator: ${AUTHENTICATOR_CLASS}," \
-e '$a\ authenticationHandler:
org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler,' \
-e '$a\ config: {tokens: conf/rest-server.properties}' \
-e '$a\}' ${CONF}/${GREMLIN_SERVER_CONF}
+fi
- sed -i -e
'$a\auth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator' \
- -e '$a\auth.graph_store=hugegraph' ${CONF}/${REST_SERVER_CONF}
+if ! grep -Eq
'^[[:blank:]]*auth[\\]?\.authenticator[[:blank:]]*([:=]|[[:blank:]])'
"${CONF}/${REST_SERVER_CONF}"; then
+ sed -i -e "\$a\\auth.authenticator=${AUTHENTICATOR_CLASS}"
${CONF}/${REST_SERVER_CONF}
+fi
+
+if ! grep -Eq
'^[[:blank:]]*auth[\\]?\.graph_store[[:blank:]]*([:=]|[[:blank:]])'
"${CONF}/${REST_SERVER_CONF}"; then
+ sed -i -e '$a\auth.graph_store=hugegraph' ${CONF}/${REST_SERVER_CONF}
+fi
- sed -i
's/gremlin.graph=org.apache.hugegraph.HugeFactory/gremlin.graph=org.apache.hugegraph.auth.HugeFactoryAuthProxy/g'
${CONF}/graphs/${GRAPH_CONF}
+# GNU grep reads \r in a pattern as the letter r, so the carriage return a
+# CRLF line ends with is embedded as a byte: without it the anchored guard
+# misses a mounted CRLF config and the factory is never wrapped for auth
+# although both servers already believe authentication is on.
+CR=$'\r'
+if grep -Eq
"^gremlin\\.graph[[:blank:]]*=org\\.apache\\.hugegraph\\.HugeFactory[[:blank:]]*${CR}?\$"
"${CONF}/graphs/${GRAPH_CONF}"; then
Review Comment:
⚠️ This guard is stricter than the `sed` it guards, so part of that `sed`'s
own pattern can never fire. The guard needs `HugeFactory` immediately after
`graph[[:blank:]]*=`, while the replacement on line 82 allows `[[:blank:]]*`
there:
```
gremlin.graph=org.apache.hugegraph.HugeFactory guard=MATCH
gremlin.graph = org.apache.hugegraph.HugeFactory guard=miss, the sed
would have rewritten it
gremlin.graph= org.apache.hugegraph.HugeFactory guard=miss, the sed
would have rewritten it
gremlin.graph=org.apache.hugegraph.HugeFactory guard=miss
```
End to end at this head with `gremlin.graph =
org.apache.hugegraph.HugeFactory` in a mounted `hugegraph.properties`: REST and
the yaml both come out on `StandardAuthenticator` while `gremlin.graph` stays
on the unwrapped `HugeFactory`. `java.util.Properties` reads that spelling
identically to the unspaced one. The guard shape came from the CRLF thread on
the previous head, and the `auth.authenticator` and `auth.graph_store` guards
above were widened to `[:=]`, bare whitespace and escaped keys in that same
round; this one was not.
Requested change: widen the guard and the `sed` together, since relaxing
only the guard just lets the `sed` no-op. Verified against all four spellings
above plus `:`, bare-whitespace and escaped-key forms, and it still skips an
already-proxied line, a commented line, and preserves a trailing CR:
```sh
if grep -Eq
"^[[:blank:]]*gremlin[\\\\]?\\.graph[[:blank:]]*([:=]|[[:blank:]])[[:blank:]]*org\\.apache\\.hugegraph\\.HugeFactory[[:blank:]]*${CR}?\$"
"${CONF}/graphs/${GRAPH_CONF}"; then
sed -i -E
"s#^([[:blank:]]*gremlin[\\\\]?\\.graph[[:blank:]]*([:=]|[[:blank:]])[[:blank:]]*)org\\.apache\\.hugegraph\\.HugeFactory#\\1org.apache.hugegraph.auth.HugeFactoryAuthProxy#"
"${CONF}/graphs/${GRAPH_CONF}"
fi
```
##########
hugegraph-server/hugegraph-dist/docker/props.awk:
##########
@@ -0,0 +1,232 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# props.awk — read and rewrite Java ".properties" files with the grammar
+# HugeConfig (commons-configuration over JDK Properties) applies, so the
+# entrypoint and the server agree on what a mounted file means. grep/sed
+# rewrites do not: they see `\`-escaped keys, `:` separators, continuation
+# lines and duplicate definitions differently, which is how a mounted
+# config ends up with two definitions of one key.
+#
+# One invocation, selected with the `PROPS_MODE` environment variable:
+#
+# PROPS_MODE=get PROPS_KEY=K PROPS_FILE=F
+# print the value of K's first logical definition
+# PROPS_MODE=set PROPS_KEY=K PROPS_FILE=F
+# replace K's first definition in place, drop every other
+# definition of K, append one when the file has none. The new
+# value arrives pre-encoded in PROPS_VALUE_ENCODED (an environment
+# variable, so secrets never appear in `ps` output or in awk's
+# argv), and -v is not used for it so awk cannot mangle its
+# backslash escapes.
+#
+# Grammar implemented (java.util.Properties line reader + the
+# first-definition-wins rule Configuration.getString applies):
+# - '#' / '!' comments and blank lines
+# - '=' / ':' / whitespace separators, with whitespace then an optional
+# single '=' or ':' accepted as one separator
+# - continuations: a physical line ending in an odd number of
+# backslashes joins the next line (its leading whitespace stripped)
+# - backslash escapes in keys and values, including \uXXXX
+# - duplicate logical keys resolve to the first definition
+#
+# Rewrites keep every untouched line byte-for-byte (comments, blank
+# lines, unrelated entries), and replace the first definition where it
+# stands, so mounted configs stay reviewable in git diffs.
+
+function die(msg) {
+ printf "props.awk: %s\n", msg > "/dev/stderr"
+ exit 1
+}
+
+function hex_digit(c) {
+ return index("0123456789abcdef", tolower(c)) - 1
+}
+
+# \uXXXX is a UTF-16 code unit in Java. Values here are effectively
+# ISO-8859-1, so codes above 0xFF are kept as their literal escape text
+# rather than being mangled through a single-byte sprintf.
+function unescape(s, out, i, n, c, code, j, d, ok) {
+ out = ""
+ n = length(s)
+ for (i = 1; i <= n; i++) {
+ c = substr(s, i, 1)
+ if (c != "\\") { out = out c; continue }
+ if (i == n) break
+ i++
+ c = substr(s, i, 1)
+ if (c == "u" && i + 4 <= n) {
+ code = 0
+ ok = 1
+ for (j = 1; j <= 4; j++) {
+ d = hex_digit(substr(s, i + j, 1))
+ if (d < 0) { ok = 0; break }
+ code = code * 16 + d
+ }
+ if (ok) {
+ i += 4
+ if (code <= 255) out = out sprintf("%c", code)
+ else out = out substr(s, i - 5, 6)
+ continue
+ }
+ }
+ if (c == "t") out = out "\t"
+ else if (c == "n") out = out "\n"
+ else if (c == "r") out = out "\r"
+ else if (c == "f") out = out "\f"
+ else out = out c
+ }
+ return out
+}
+
+# A physical line is continued when it ends in an odd number of
+# backslashes (an even count escapes itself).
+function trailing_backslashes(s, n, k) {
+ n = length(s)
+ k = 0
+ while (k < n && substr(s, n - k, 1) == "\\") k++
+ return k
+}
+
+function is_skipped(raw) {
+ return raw ~ /^[ \t]*([#!]|$)/
+}
+
+# Split a logical line into its raw (still-escaped) key and value parts.
+# Results land in K_RAW / V_RAW because awk returns one value.
+function split_kv(s, n, i, c, esc, sep_at, rest) {
+ n = length(s)
+ esc = 0
+ sep_at = 0
+ for (i = 1; i <= n; i++) {
+ c = substr(s, i, 1)
+ if (esc) { esc = 0; continue }
+ if (c == "\\") { esc = 1; continue }
+ if (c == "=" || c == ":" || c == " " || c == "\t") { sep_at = i; break
}
+ }
+ if (sep_at == 0) {
+ K_RAW = s
+ V_RAW = ""
+ return
+ }
+ K_RAW = substr(s, 1, sep_at - 1)
+ rest = substr(s, sep_at)
+ c = substr(rest, 1, 1)
+ if (c == "=" || c == ":") {
+ rest = substr(rest, 2)
+ } else {
+ sub(/^[ \t]+/, "", rest)
+ c = substr(rest, 1, 1)
+ if (c == "=" || c == ":") rest = substr(rest, 2)
+ }
+ sub(/^[ \t]+/, "", rest)
+ V_RAW = rest
+}
+
+# Load `file` into per-block arrays: one block per comment/blank line or
+# logical entry, spanning exactly the physical lines it occupies.
+function props_load(file, raw, nl, next_raw, start, logical) {
+ NLINES = 0
+ while ((getline raw < file) > 0) {
Review Comment:
⚠️ `getline` here keeps the `\r` of a CRLF line, so `props.awk` parses
differently from the grammar it implements. `java.util.Properties` treats
`\r\n` as the line terminator and drops it.
```
# rest-server.properties saved with CRLF
props.awk get auth.authenticator ->
org.apache.hugegraph.auth.StandardAuthenticator\r
java.util.Properties.load ->
org.apache.hugegraph.auth.StandardAuthenticator
```
Continuations break too, because `trailing_backslashes` (line 96) sees the
`\r`, not the backslash, as the last character:
```
# pd.peers=a,\<CR><LF> b<CR><LF>
props.awk get pd.peers -> a,\<CR>
java.util.Properties -> a,b
```
Downstream, `align_auth_config` compares this CR-bearing value against
`get_yaml_authenticator`, whose `scalar()` strips CR, so a REST and a Gremlin
side naming the same class log `WARN: REST and Gremlin name different
authenticators` with two strings that print identically, and neither side is
aligned. Mounted CRLF configs are in scope: the `enable-auth.sh` hunk in this
PR embeds a `CR` byte for exactly that case.
Requested change: strip one trailing `\r` while assembling `logical` in
`props_load`, after this `getline` and inside the continuation loop, rather
than from `RAW[]`, which `props_set` replays byte for byte. A CRLF case in
`test/test-docker-entrypoint.sh` would pin both halves.
##########
hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh:
##########
@@ -70,12 +74,108 @@ set_prop() {
get_prop_encoded() {
local key="$1" file="$2"
- local esc_key
- esc_key=$(printf '%s' "$key" | sed -e 's/[][(){}.^$*+?|\\/]/\\&/g')
- sed -nE \
-
"s~^[[:space:]]*${esc_key}([[:space:]]*[:=][[:space:]]*|[[:space:]]+)(.*)$~\\2~p"
\
- "${file}" | head -n 1
+ PROPS_MODE=get PROPS_KEY="${key}" PROPS_FILE="${file}" \
+ awk -f "${PROPS_AWK}" /dev/null
+}
+
+# First uncommented `authenticator:` inside the gremlin-server.yaml
+# authentication block, or on the `authentication:` line itself (a flow
+# mapping). snakeyaml resolves duplicate top-level keys to the last one,
+# but a mounted file carrying two authentication blocks is pathological;
+# report the first and let the mismatch WARN handle it. The scalar is
+# cleaned the way snakeyaml reads it — an inline comment (a '#' preceded
+# by whitespace), surrounding quotes and padding are stripped — because
+# java.util.Properties keeps all of those in the class name.
+get_yaml_authenticator() {
+ local yaml="./conf/gremlin-server.yaml"
+
+ [[ -f "${yaml}" ]] || return 0
+ awk '
+ function scalar(s, out, i, n, c, q) {
+ out = ""
+ q = ""
+ n = length(s)
+ for (i = 1; i <= n; i++) {
+ c = substr(s, i, 1)
+ if (q != "") {
+ if (c == q) q = ""
+ else out = out c
+ continue
+ }
+ if (c == "\"" || c == "\047") { q = c; continue }
+ if (c == "#" &&
+ (out == "" || substr(out, length(out), 1) ~ /[ \t]/))
+ break
+ if (c == "," || c == "}" || c == "]") break
+ out = out c
+ }
+ sub(/^[ \t\r]+/, "", out)
+ sub(/[ \t\r]+$/, "", out)
+ return out
+ }
+ /^[ \t]*#/ { next }
+ /^[ \t]*authentication[ \t]*:/ {
+ inblk = 1
+ line = $0
+ sub(/^[ \t]*authentication[ \t]*:[ \t]*/, "", line)
+ if (match(line, /authenticator[ \t]*:/)) {
+ print scalar(substr(line, RSTART + RLENGTH))
+ exit
+ }
+ next
+ }
+ inblk && /^[ \t]+authenticator[ \t]*:/ {
+ line = $0
+ sub(/^[ \t]*authenticator[ \t]*:[ \t]*/, "", line)
+ print scalar(line)
+ exit
+ }
+ ' "${yaml}"
+}
+
+# A mounted yaml can carry an authentication block whose authenticator
+# cannot be read (an empty or unparseable one). That is not the
+# both-empty case: exporting the default would override an explicit
+# choice that snakeyaml does resolve, so callers treat it as a mismatch.
+has_yaml_authentication_block() {
+ local yaml="./conf/gremlin-server.yaml"
+
+ [[ -f "${yaml}" ]] || return 1
+ grep -Eq '^[[:blank:]]*authentication[[:blank:]]*:' "${yaml}"
+}
+
+# enable-auth.sh appends definitions to files it did not write. On a
+# mounted config those appended definitions are duplicates the two parsers
+# resolve in opposite directions — HugeConfig (commons-configuration) takes
+# the first, snakeyaml takes the last — so Gremlin and REST can land on
+# different authenticators with no error from either. Normalize both sides
+# to one definition of the same authenticator here; enable-auth.sh's
+# per-file guards then make its appends no-ops on anything already set.
+align_auth_config() {
+ local rest_auth yaml_auth
+
+ rest_auth=$(get_prop_encoded "auth.authenticator" "${REST_SERVER_CONF}")
+ yaml_auth=$(get_yaml_authenticator)
+ if [[ -z "${yaml_auth}" ]] && has_yaml_authentication_block; then
+ log "WARN: gremlin-server.yaml carries an authentication block" \
+ "without a readable authenticator; leaving both sides untouched"
+ return
+ fi
+ if [[ -n "${rest_auth}" && -n "${yaml_auth}" && "${rest_auth}" !=
"${yaml_auth}" ]]; then
Review Comment:
🧹 These two values are decoded differently, so identical configurations can
read as a mismatch. `props.awk` returns the on-disk escaped form on purpose
("The value of get is intentionally *not* unescaped"), while
`get_yaml_authenticator`'s `scalar()` returns what snakeyaml decodes:
```
rest-server.properties:
auth.authenticator=org.apache.hugegraph.auth\.StandardAuthenticator
gremlin-server.yaml: authenticator:
org.apache.hugegraph.auth.StandardAuthenticator
WARN: REST and Gremlin name different authenticators
('org.apache.hugegraph.auth\.StandardAuthenticator' vs
'org.apache.hugegraph.auth.StandardAuthenticator'); leaving both
untouched
```
`java.util.Properties` reads both spellings as the same class, so the WARN
is spurious and the alignment this function exists for is skipped.
Line 173 is the mirror of it: `set_prop_encoded "auth.authenticator"
"${yaml_auth}"` hands a snakeyaml-decoded scalar to the setter that skips
`encode_prop_value`. Harmless for a bare class name, wrong for any scalar
carrying a backslash or leading space.
Requested change: unescape before comparing and before the `export` below,
either through a `get_decoded` mode in `props.awk` (keeping the raw mode for
the secret round trip that needs it) or an unescape here, and use `set_prop`
rather than `set_prop_encoded` on line 173.
##########
hugegraph-server/hugegraph-dist/docker/props.awk:
##########
@@ -0,0 +1,232 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# props.awk — read and rewrite Java ".properties" files with the grammar
+# HugeConfig (commons-configuration over JDK Properties) applies, so the
+# entrypoint and the server agree on what a mounted file means. grep/sed
+# rewrites do not: they see `\`-escaped keys, `:` separators, continuation
+# lines and duplicate definitions differently, which is how a mounted
+# config ends up with two definitions of one key.
+#
+# One invocation, selected with the `PROPS_MODE` environment variable:
+#
+# PROPS_MODE=get PROPS_KEY=K PROPS_FILE=F
+# print the value of K's first logical definition
+# PROPS_MODE=set PROPS_KEY=K PROPS_FILE=F
+# replace K's first definition in place, drop every other
+# definition of K, append one when the file has none. The new
+# value arrives pre-encoded in PROPS_VALUE_ENCODED (an environment
+# variable, so secrets never appear in `ps` output or in awk's
+# argv), and -v is not used for it so awk cannot mangle its
+# backslash escapes.
+#
+# Grammar implemented (java.util.Properties line reader + the
+# first-definition-wins rule Configuration.getString applies):
+# - '#' / '!' comments and blank lines
+# - '=' / ':' / whitespace separators, with whitespace then an optional
+# single '=' or ':' accepted as one separator
+# - continuations: a physical line ending in an odd number of
+# backslashes joins the next line (its leading whitespace stripped)
+# - backslash escapes in keys and values, including \uXXXX
+# - duplicate logical keys resolve to the first definition
+#
+# Rewrites keep every untouched line byte-for-byte (comments, blank
+# lines, unrelated entries), and replace the first definition where it
+# stands, so mounted configs stay reviewable in git diffs.
+
+function die(msg) {
+ printf "props.awk: %s\n", msg > "/dev/stderr"
+ exit 1
+}
+
+function hex_digit(c) {
+ return index("0123456789abcdef", tolower(c)) - 1
+}
+
+# \uXXXX is a UTF-16 code unit in Java. Values here are effectively
+# ISO-8859-1, so codes above 0xFF are kept as their literal escape text
+# rather than being mangled through a single-byte sprintf.
+function unescape(s, out, i, n, c, code, j, d, ok) {
+ out = ""
+ n = length(s)
+ for (i = 1; i <= n; i++) {
+ c = substr(s, i, 1)
+ if (c != "\\") { out = out c; continue }
+ if (i == n) break
+ i++
+ c = substr(s, i, 1)
+ if (c == "u" && i + 4 <= n) {
+ code = 0
+ ok = 1
+ for (j = 1; j <= 4; j++) {
+ d = hex_digit(substr(s, i + j, 1))
+ if (d < 0) { ok = 0; break }
+ code = code * 16 + d
+ }
+ if (ok) {
+ i += 4
+ if (code <= 255) out = out sprintf("%c", code)
+ else out = out substr(s, i - 5, 6)
+ continue
+ }
+ }
+ if (c == "t") out = out "\t"
+ else if (c == "n") out = out "\n"
+ else if (c == "r") out = out "\r"
+ else if (c == "f") out = out "\f"
+ else out = out c
+ }
+ return out
+}
+
+# A physical line is continued when it ends in an odd number of
+# backslashes (an even count escapes itself).
+function trailing_backslashes(s, n, k) {
+ n = length(s)
+ k = 0
+ while (k < n && substr(s, n - k, 1) == "\\") k++
+ return k
+}
+
+function is_skipped(raw) {
+ return raw ~ /^[ \t]*([#!]|$)/
+}
+
+# Split a logical line into its raw (still-escaped) key and value parts.
+# Results land in K_RAW / V_RAW because awk returns one value.
+function split_kv(s, n, i, c, esc, sep_at, rest) {
+ n = length(s)
+ esc = 0
+ sep_at = 0
+ for (i = 1; i <= n; i++) {
+ c = substr(s, i, 1)
+ if (esc) { esc = 0; continue }
+ if (c == "\\") { esc = 1; continue }
+ if (c == "=" || c == ":" || c == " " || c == "\t") { sep_at = i; break
}
+ }
+ if (sep_at == 0) {
+ K_RAW = s
+ V_RAW = ""
+ return
+ }
+ K_RAW = substr(s, 1, sep_at - 1)
+ rest = substr(s, sep_at)
+ c = substr(rest, 1, 1)
+ if (c == "=" || c == ":") {
+ rest = substr(rest, 2)
+ } else {
+ sub(/^[ \t]+/, "", rest)
+ c = substr(rest, 1, 1)
+ if (c == "=" || c == ":") rest = substr(rest, 2)
+ }
+ sub(/^[ \t]+/, "", rest)
+ V_RAW = rest
+}
+
+# Load `file` into per-block arrays: one block per comment/blank line or
+# logical entry, spanning exactly the physical lines it occupies.
+function props_load(file, raw, nl, next_raw, start, logical) {
+ NLINES = 0
+ while ((getline raw < file) > 0) {
+ NLINES++
+ RAW[NLINES] = raw
+ }
+ close(file)
+
+ NBLOCK = 0
+ for (nl = 1; nl <= NLINES; nl++) {
+ raw = RAW[nl]
+ if (is_skipped(raw)) {
+ NBLOCK++
+ BTYPE[NBLOCK] = "skip"
+ BFIRST[NBLOCK] = nl
+ BLAST[NBLOCK] = nl
+ continue
+ }
+ start = nl
+ logical = raw
+ while (trailing_backslashes(logical) % 2 == 1 && nl < NLINES) {
+ logical = substr(logical, 1, length(logical) - 1)
+ nl++
+ next_raw = RAW[nl]
+ sub(/^[ \t]+/, "", next_raw)
+ logical = logical next_raw
+ }
+ # java.util.Properties ignores whitespace before the key; strip it
+ # so split_kv's separator scan agrees (an indented key used to be
+ # read as a key whose name started with a space, and a set then
+ # appended a second definition of the real key).
+ sub(/^[ \t]+/, "", logical)
+ split_kv(logical)
+ NBLOCK++
+ BTYPE[NBLOCK] = "entry"
+ BFIRST[NBLOCK] = start
+ BLAST[NBLOCK] = nl
+ BKEY[NBLOCK] = unescape(K_RAW)
+ # Values stay in their on-disk escaped form. get Prop callers feed
+ # the result straight back into set, which would corrupt a decoded
+ # value by re-writing its backslashes as literals; keys are
+ # unescaped because they are matched against plain names.
+ BVAL[NBLOCK] = V_RAW
+ }
+}
+
+function props_set(file, key, enc_val, b, first, ln) {
+ props_load(file)
+ first = 0
+ for (b = 1; b <= NBLOCK; b++) {
+ if (BTYPE[b] == "entry" && BKEY[b] == key) {
+ if (first == 0) first = b
+ else BDROP[b] = 1
+ }
+ }
+ for (b = 1; b <= NBLOCK; b++) {
+ if (BDROP[b]) continue
+ if (b == first) {
+ printf "%s=%s\n", key, enc_val > file
Review Comment:
⚠️ This rewrite is destructive and non-atomic, a step back from the `sed -i`
it replaces, which writes a temp file and renames.
`props_load`'s `while ((getline raw < file) > 0)` (line 142) treats
`getline`'s `-1`, an unreadable file, the same as EOF. `NLINES` stays 0,
`props_set` reaches the `first == 0` append with nothing to replay, and the
config is gone:
```
$ printf 'a=1\nb=2\nauth.token_secret=keepme\n' > w.properties && chmod 222
w.properties
$ PROPS_MODE=set PROPS_KEY=x PROPS_VALUE_ENCODED=1 PROPS_FILE=w.properties
awk -f props.awk /dev/null; echo $?
0
$ cat w.properties
x=1
```
Separately, awk's `>` truncates on this first write, before the preserved
lines are replayed, so a kill or ENOSPC mid-rewrite leaves a mounted
`rest-server.properties` truncated. `conf-bak/` is no help: it is written
later, by `enable-auth.sh`.
Requested change: `die()` in `props_load` when `getline` returns `-1`, and
have `props_set` write to a sibling temp file, `close()` it, then rename it
over the original with the paths shell-quoted. Note the `die()` also rejects a
genuinely missing file, since awk cannot tell missing from unreadable; nothing
in the entrypoint or the suite depends on that append path.
--
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]
