diff --git a/miniserv-lib.pl b/miniserv-lib.pl index 0123b78f9..a0ffbbe22 100644 --- a/miniserv-lib.pl +++ b/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()); } } diff --git a/t/fixtures/miniserv-cgi-status.py b/t/fixtures/miniserv-cgi-status.py new file mode 100644 index 000000000..8912af956 --- /dev/null +++ b/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.') diff --git a/t/miniserv-cgi-status.t b/t/miniserv-cgi-status.t new file mode 100644 index 000000000..b0b1c1914 --- /dev/null +++ b/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();