Repository navigation
validate.sh: fix flaky fortunes checks (SIGPIPE under pipefail) - #1564
Merged
Merged
Conversation
… grep -q Under `set -o pipefail`, `echo "$body" | grep -q PATTERN` fails whenever grep finds an early match and exits while echo is still writing: echo dies of SIGPIPE (exit 141) and the pipeline reports failure although grep matched. The fortunes checks hit this on a correct ~25 KB page, 769 of 2000 runs on the XSS check and 116 on the DOCTYPE check, and it showed up on #1548 as a false "<script> in row 11 not properly HTML-escaped". A here-string has no writer to kill: 0 of 2000. The other four places that pipe a captured variable into grep -q (the TLS resumption and close_notify probes, the json-tls Content-Type check) are switched too, so the rule is the same everywhere.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The fortunes checks can fail on a correct page.
validate.shruns underset -o pipefail, and the checks were written asecho "$body" | grep -q PATTERN. When grep finds an early match it exits whileechois still writing the rest of the ~25 KB page.echodies of SIGPIPE (PIPESTATUS=(141 0)), so the pipeline reports failure even though grep matched.This is what failed #1548 locally once its directory name was fixed:
FAIL [GET /fortunes XSS escape]: <script> in row 11 not properly HTML-escaped, although row 11 comes back as<script>alert("….Measured by running the three checks 2000 times on the same saved, correct page:
echo | grep -q(two runs)<!DOCTYPE html>The fix matches captured variables with here-strings (
grep -q PATTERN <<< "$var"), so no writer process can be killed. The other four places that pipe a captured variable intogrep -q(TLS resumption probe, close_notify probe, json-tls Content-Type check) are switched too, so the rule is the same everywhere. Those four look at small strings and probably never raced. Pipelines that feed a live command intogrep -qare left alone: the one whose output is big enough to matter (openssl s_client -help) showed 0 failures in 600 runs, because openssl writes its help text in a single call.For context,
FAIL [GET /fortunes XSS escape]also shows up in recent failed CI runs for Ktor (35351621895), vanilla-epoll (38080909170) and morojs (38093469575). I haven't checked whether those were this race.Test plan
bash -n scripts/validate.sh/fortunespage (table above)<script>alert(1)</script>pagescripts/validate.sh aspnet-minimal-satorigc(Add aspnet-minimal-satoriGC #1548's entry, fixed) with this change: 116 passed, 0 failed🤖 Generated with Claude Code