DEV: patchbot: update: never write to stderr, thttpd sends it first

thttpd forwards the CGI's stderr to the client *ahead* of its stdout:
the socket receives the HTTP status line, then any stderr log line, and
only then the CGI headers, turning the log line into a bogus response
header; on an error path the same mechanism could push garbage in front
of the "Status:" header and corrupt the response entirely.

All diagnostics are already carried by the response itself (die()'s
body, the in-band "warning:" line with git's captured error), so the
duplicated stderr logging brings nothing and only risks breaking the
channel it leaks into: drop it, and state the constraint in a comment
above die() so it doesn't come back. The usage text for a bad command
line is folded into the 500 response body, which is also what a shell
user sees when testing by hand.

diff --git a/dev/patchbot/cgi/update.awk b/dev/patchbot/cgi/update.awk
index edb24f86d750df980090243e890591e9f0cd3d3a..401374dcbcfbbc91613fd98277c90d719a4e6b20 100755 (executable)
--- a/dev/patchbot/cgi/update.awk
+++ b/dev/patchbot/cgi/update.awk
@@ -105,10 +105,9 @@ BEGIN {
                        repo = ARGV[i]
                        ARGV[i] = ""
                }
-               else if (ARGV[i] != "") {
-                       print "Usage: update.awk -r /path/to/repo" > "/dev/stderr"
-                       die("500 Internal Server Error", "server misconfigured")
-               }
+               else if (ARGV[i] != "")
+                       die("500 Internal Server Error", \
+                           "server misconfigured (usage: update.awk -r /path/to/repo)")
        }
 
        if (repo == "" || system("test -d " q(repo)) != 0)
@@ -137,13 +136,15 @@ function q(s)
        return "'" s "'"
 }
 
-# prints a complete CGI error response and exits, releasing the lock if it
-# was held. Also logs to stderr (ends up in the web server's error log).
+# Prints a complete CGI error response and exits, releasing the lock if it
+# was held. Nothing is ever written to stderr, here nor anywhere else:
+# thttpd forwards the CGI's stderr to the client *before* its stdout, so
+# anything written there would land ahead of the response headers and
+# corrupt them; every diagnostic must be carried by the response itself.
 function die(status, msg)
 {
        if (lock_held)
                lock_release()
-       print "update.awk: " msg " (" status ")" > "/dev/stderr"
        printf "Status: %s\r\nContent-Type: text/plain\r\n\r\n%s\n", status, msg
        exit 0
 }
@@ -768,17 +769,15 @@ function handle_post(   body, i, fname, nb_confl, git_failed, attempt, renamed)
                        # writer folds it into its own commit. But they must not stay
                        # invisible either or the history silently stops being
                        # recorded, so they are reported as a warning line in the
-                       # response on top of the log. A no-op (identical content, e.g.
-                       # re-pushed identical states) is not a failure: the commit is
-                       # simply skipped when nothing is staged. Never checkout or
-                       # reset here, it would eat an admin's uncommitted hand-edit.
+                       # response. A no-op (identical content, e.g. re-pushed
+                       # identical states) is not a failure: the commit is simply
+                       # skipped when nothing is staged. Never checkout or reset
+                       # here, it would eat an admin's uncommitted hand-edit.
                        if (run_git("add -- " q(branch)) != 0)
                                git_failed = 1
                        else if (run_git("diff --cached --quiet") != 0 && \
                                 run_git("commit -q -m " q("update " branch)) != 0)
                                git_failed = 1
-                       if (git_failed)
-                               print "update.awk: git commit failed in " repo ": " GITMSG > "/dev/stderr"
                }
                # else: everything conflicted, nothing changed, nothing to write