Skip to content

Commit 6d8e306

Browse files
committed
ci: propagate curl's exit status in the archive canary
`read -r ... < <(curl ...)` returns read's own status and discards curl's. A transfer that fails after the response headers — a truncated body, exit 18 or 56 — still emits `%{http_code} 200`, so every assertion below it passed on a fetch that never completed. Raised in review of #2376. Verified rather than assumed. Simulating curl emitting its -w output and then exiting 56: read -r a b c < <(printf '200 0 https://x/\n'; exit 56) -> script continues with status=200, exit 0 out="$( { printf '200 0 https://x/\n'; exit 56; } )" || rc=$? -> rc=56, caught Switches to command substitution with an explicit `|| rc=$?` capture, so the status can be inspected before `set -e` acts on it, and parses with a here-string — which also supplies the trailing newline `read` needs, making the `\n` in the -w format redundant. Fifth false-pass path closed in this workflow, and the fourth found in review. Each one had the same shape: a check that reports success without having measured the thing it names. Signed-off-by: Mark Chmarny <mark@chmarny.com>
1 parent a57c133 commit 6d8e306

1 file changed

Lines changed: 18 additions & 7 deletions

File tree

.github/workflows/dgxc-goproxy-probe.yaml

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -164,15 +164,26 @@ jobs:
164164
auth="$("${GOAUTH}" | grep -i '^Authorization:')"
165165
[ -n "${auth}" ] || { echo "::error::GOAUTH helper produced no Authorization header"; exit 1; }
166166
167-
# The trailing newline in -w is load-bearing. `read` returns non-zero
168-
# at EOF even when it assigned every variable, so without it `set -e`
169-
# kills this step with no output at all — which is exactly how it
170-
# failed the first time.
171-
read -r status redirects effective < <(
167+
# Command substitution with an explicit status capture, not process
168+
# substitution. `read < <(...)` returns read's own status and
169+
# discards curl's, so a transfer that fails *after* the response
170+
# headers — a truncated body, exit 18 or 56 — still emits
171+
# `%{http_code} 200` and every assertion below passes on a fetch
172+
# that did not complete. Verified: a simulated exit 56 after output
173+
# is silently accepted under process substitution and caught here.
174+
#
175+
# `|| rc=$?` keeps the assignment from tripping `set -e` before the
176+
# status can be inspected, and the here-string below supplies the
177+
# trailing newline `read` needs to avoid returning non-zero at EOF.
178+
rc=0
179+
out="$(
172180
printf 'header = "%s"\nurl = "%s"\n' "${auth}" "${url}" \
173181
| curl --config - -sSL --max-time 60 -o /dev/null \
174-
-w '%{http_code} %{num_redirects} %{url_effective}\n'
175-
)
182+
-w '%{http_code} %{num_redirects} %{url_effective}'
183+
)" || rc=$?
184+
[ "${rc}" -eq 0 ] \
185+
|| { echo "::error::curl exited ${rc}; the transfer did not complete"; exit 1; }
186+
read -r status redirects effective <<<"${out}"
176187
echo "status: ${status}"
177188
echo "redirects: ${redirects}"
178189
echo "final URL host: $(printf '%s' "${effective}" | sed -E 's#^https?://([^/]+)/.*#\1#')"

0 commit comments

Comments
 (0)