Fix CGI response status handling

This PR fixes MiniServ ignoring CGI Status headers and returning the wrong HTTP status.

Both CGI execution modes now honor explicit statuses, preserve default redirects, and record the correct status in request logs.

The Status control header is no longer forwarded to clients.

Validation passed: 102 local tests and 68 live checks on Debian 13, covering both CGI modes.

miniserv-lib.pl

@@ -1565,8 +1565,23 @@ if (&get_type($full) eq "internal/cgi" && $validated != 4) {
 				$cgiheader{lc($1)} = $2;
 				push(@cgiheader, [ $1, $2 ]);
 				}
+			# A CGI can reject a request before producing normal
+			# output. Use its explicit status instead of the default
+			# success code.
+			my $cgi_status = $cgiheader{'status'} =~
+				/\A([1-5][0-9][0-9])\s+([^\r\n]+)\z/;
+			if ($cgi_status) {
+				($ok_code, $ok_message) = ($1, $2);
+				}
+			else {
+				# Invalid control headers must not affect
+				# redirect logging.
+				delete($cgiheader{'status'});
+				}
 			if ($cgiheader{"location"}) {
-				&write_data("HTTP/1.0 302 Moved Temporarily\r\n");
+				&write_data("HTTP/1.0 ".($cgi_status
+					? "$ok_code $ok_message"
+					: "302 Moved Temporarily")."\r\n");
 				&write_data("Date: $datestr\r\n");
 				&write_data("Server: @{[&server_info()]}\r\n");
 				&write_keep_alive(0);
@@ -1588,6 +1603,7 @@ if (&get_type($full) eq "internal/cgi" && $validated != 4) {
 				&write_keep_alive(0);
 				}
 			foreach $h (@cgiheader) {
+				next if (lc($h->[0]) eq 'status');
 				&write_data("$h->[0]: $h->[1]\r\n");
 				}
 			&write_data("\r\n");
@@ -1671,7 +1687,9 @@ else {
 # log the request
 &log_request($loghost, $authuser, $reqline,
 	     $logged_code ? $logged_code :
-	     $cgiheader{"location"} ? "302" : $ok_code, &byte_count());
+	     $cgiheader{"location"} && !$cgiheader{"status"}
+		? "302"
+		: $ok_code, &byte_count());
 return $rv;
 }
 
@@ -2499,9 +2517,22 @@ foreach $d (@_) {
 				}
 			}
 		if ($doneheaders) {
+			# Match forked CGI status handling for in-process
+			# scripts.
+			my $cgi_status = $cgiheader{'status'} =~
+				/\A([1-5][0-9][0-9])\s+([^\r\n]+)\z/;
+			if ($cgi_status) {
+				($ok_code, $ok_message) = ($1, $2);
+				}
+			else {
+				# Invalid control headers must not affect
+				# redirect logging.
+				delete($cgiheader{'status'});
+				}
 			if ($cgiheader{"location"}) {
-				&write_data(
-					"HTTP/1.0 302 Moved Temporarily\r\n");
+				&write_data("HTTP/1.0 ".($cgi_status
+					? "$ok_code $ok_message"
+					: "302 Moved Temporarily")."\r\n");
 				&write_data("Date: $datestr\r\n");
 				&write_data("Server: @{[&server_info()]}\r\n");
 				&write_keep_alive(0);
@@ -2516,6 +2547,7 @@ foreach $d (@_) {
 				&write_keep_alive(0);
 				}
 			foreach $h (@cgiheader) {
+				next if (lc($h->[0]) eq 'status');
 				&write_data("$h->[0]: $h->[1]\r\n");
 				}
 			&write_data("\r\n");
@@ -2553,7 +2585,9 @@ if ($doing_cgi_eval && $$ == $main_process_id) {
 	close(SOCK);
 	close($PASSINw); close($PASSOUTw);
 	&log_request($loghost, $authuser, $reqline,
-		     $cgiheader{"location"} ? "302" : $ok_code, &byte_count());
+		     $cgiheader{"location"} && !$cgiheader{"status"}
+		     	? "302"
+			: $ok_code, &byte_count());
 	}
 }
 

t/fixtures/miniserv-cgi-status.py

@@ -0,0 +1,126 @@
+"""Test CGI responses and request logs on a disposable Linux guest.
+
+Usage: python3 t/fixtures/miniserv-cgi-status.py /tmp/task-directory
+The source tree containing this fixture supplies miniserv.pl and miniserv-lib.pl.
+Each CGI mode gets a separate loopback server and temporary configuration.
+No Webmin account or installed configuration is used. Never run locally.
+"""
+from pathlib import Path
+import http.client
+import os
+import re
+import socket
+import subprocess
+import sys
+import tempfile
+import time
+
+if sys.platform != 'linux':
+    raise RuntimeError('Run only inside a disposable Linux test guest')
+task = Path(sys.argv[1]).resolve()
+if not task.is_dir() or not any(p in task.parents for p in (Path('/tmp'), Path('/var/tmp'))):
+    raise RuntimeError('Supply an existing task directory under /tmp or /var/tmp')
+source = Path(__file__).resolve().parents[2]
+os.umask(0o077)
+
+
+def request(port, name):
+    """request(port, name): return status, headers and body without redirects."""
+    conn = http.client.HTTPConnection('127.0.0.1', port, timeout=10)
+    try:
+        conn.request('GET', '/' + name)
+        response = conn.getresponse()
+        return response.status, response.getheaders(), response.read().decode()
+    finally:
+        conn.close()
+
+
+def check(condition, message):
+    """check(condition, message): report a result or stop on failure."""
+    if not condition:
+        raise AssertionError(message)
+    print('PASS: ' + message, flush=True)
+
+
+# Cases cover defaults, explicit errors, redirects, malformed headers and exit.
+cases = [
+    ('normal', '', 200, False),
+    ('denied', 'Status: 403 Forbidden\r\n', 403, False),
+    ('created', 'Status: 201 Created\r\n', 201, False),
+    ('redirect', 'Location: /next\r\n', 302, False),
+    ('redirect307', 'Status: 307 Temporary Redirect\r\nLocation: /next\r\n', 307, False),
+    ('invalid', 'Status: invalid\r\nLocation: /next\r\n', 302, False),
+    ('badcode', 'Status: 999 Invalid\r\n', 200, False),
+    ('mixedcase', 'sTaTuS: 410 Gone\r\n', 410, False),
+    ('deniedexit', 'Status: 403 Forbidden\r\n', 403, True),
+    ('redirectexit', 'Status: 307 Temporary Redirect\r\nLocation: /next\r\n', 307, True),
+    ('invalidexit', 'Status: invalid\r\nLocation: /next\r\n', 302, True),
+]
+for fork in (0, 1):
+    # Own every test file and process; cleanup also runs on assertion failures.
+    with tempfile.TemporaryDirectory(prefix='cgi-status-', dir=task) as directory:
+        work = Path(directory)
+        root = work / 'root'
+        root.mkdir()
+        for name, headers, expected, exits in cases:
+            cgi = root / (name + '.cgi')
+            response = headers + 'Content-type: text/plain\r\n\r\n'
+            literal = response.replace('\\', '\\\\').replace('"', '\\"').replace('\r', '\\r').replace('\n', '\\n')
+            cgi.write_text('#!/usr/bin/perl\nprint "' + literal + '";\n'
+                           'print $miniserv::doing_cgi_eval ? "internal" : "forked";\n'
+                           + ('exit(0);\n' if exits else ''))
+            cgi.chmod(0o700)
+        # Ask the guest for an available port instead of touching an existing server.
+        with socket.socket() as sock:
+            sock.bind(('127.0.0.1', 0))
+            port = sock.getsockname()[1]
+        config = dict(port=port, bind='127.0.0.1', root=root, ssl=0,
+                      pidfile=work/'miniserv.pid', logfile=work/'requests.log',
+                      errorlog=work/'errors.log', session=0, passdelay=0,
+                      log=1, logclear=0, syslog=0, addtype_cgi='internal/cgi',
+                      unauthcgi='^/', forkcgis=fork, internalcgis=0)
+        conf = work / 'miniserv.conf'
+        conf.write_text(''.join(f'{key}={value}\n' for key, value in config.items()))
+        server = subprocess.Popen(['perl', str(source/'miniserv.pl'), '--nofork', str(conf)],
+                                  stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL,
+                                  stderr=subprocess.DEVNULL)
+        try:
+            # Wait for a real HTTP response before testing, with a bounded deadline.
+            for attempt in range(100):
+                if server.poll() is not None:
+                    raise RuntimeError('Fixture server stopped during startup')
+                try:
+                    if request(port, 'normal.cgi')[0] == 200:
+                        break
+                except OSError:
+                    pass
+                time.sleep(.05)
+            else:
+                raise RuntimeError('Fixture server did not become ready')
+            for name, headers, expected, exits in cases:
+                status, received, body = request(port, name + '.cgi')
+                label = f'forkcgis={fork} {name}'
+                check(status == expected, f'{label}: HTTP {status}, expected {expected}')
+                check(not any(key.lower() == 'status' for key, value in received),
+                      label + ': control header is not forwarded')
+                if name == 'normal':
+                    # Prove both execution modes were used, rather than trusting config.
+                    check(body == ('forked' if fork else 'internal'), label + ': execution mode')
+                # Logging can finish just after the response socket closes.
+                pattern = rf'"GET /{name}\.cgi HTTP/1\.[01]"\s+(\d+)\b'
+                for attempt in range(100):
+                    matches = re.findall(pattern, (work/'requests.log').read_text())
+                    if matches:
+                        break
+                    time.sleep(.02)
+                check(bool(matches) and int(matches[-1]) == expected,
+                      label + ': request log has the response status')
+        finally:
+            server.terminate()
+            try:
+                server.wait(timeout=10)
+            except subprocess.TimeoutExpired:
+                # Kill only the test process if it does not honor termination.
+                server.kill()
+                server.wait(timeout=5)
+print('All CGI status and logging checks passed; fixture servers removed.')

t/miniserv-cgi-status.t

@@ -0,0 +1,44 @@
+#!/usr/bin/perl
+# Test CGI status headers without starting a server or loading Webmin config.
+use strict;
+use warnings;
+no warnings qw(once redefine uninitialized);
+use Test::More;
+use FindBin;
+require "$FindBin::Bin/../miniserv-lib.pl";
+
+# response(headers): capture the real in-process CGI HTTP response writer.
+sub response {
+    my ($headers) = @_;
+    my $output = '';
+    local *miniserv::write_data = sub { $output .= join('', @_); };
+    local *miniserv::write_keep_alive = sub {};
+    local *miniserv::reset_byte_count = sub {};
+    local *miniserv::server_info = sub { return 'fixture'; };
+    local %miniserv::cgiheader;
+    local @miniserv::cgiheader;
+    local $miniserv::headers = '';
+    local $miniserv::doneheaders;
+    local $miniserv::ok_code = 200;
+    local $miniserv::ok_message = 'Document follows';
+    miniserv::write_to_sock($headers."\r\nbody");
+    return $output;
+}
+my $denial = response("Status: 403 Forbidden\r\nContent-type: text/plain\r\n");
+like($denial, qr/^HTTP\/1\.0 403 Forbidden\r\n/, 'CGI denial sets the HTTP status');
+unlike($denial, qr/^Status:/m, 'CGI Status is not forwarded as an HTTP header');
+like(response("Location: /next\r\n"), qr/^HTTP\/1\.0 302 /,
+     'ordinary redirects keep their default status');
+like(response("Status: 307 Temporary Redirect\r\nLocation: /next\r\n"),
+     qr/^HTTP\/1\.0 307 /, 'explicit redirect status is respected');
+like(response("Status: invalid\r\nLocation: /next\r\n"), qr/^HTTP\/1\.0 302 /,
+     'invalid CGI status does not suppress the default redirect');
+
+# Ordinary pages retain the default status; other valid CGI codes also work.
+like(response("Content-type: text/plain\r\n"), qr/^HTTP\/1\.0 200 /,
+     'ordinary CGI response remains successful');
+like(response("Status: 201 Created\r\nContent-type: text/plain\r\n"),
+     qr/^HTTP\/1\.0 201 Created\r\n/, 'CGI creation response sets its status');
+like(response("Status: invalid\r\nContent-type: text/plain\r\n"),
+     qr/^HTTP\/1\.0 200 /, 'invalid CGI status leaves the default unchanged');
+done_testing();