From eaf4c5d1e562df26be51449378f014eb4d1c1993 Mon Sep 17 00:00:00 2001
From: Eric Wong <e@80x24.org>
Date: Sat, 14 Jun 2025 09:26:32 +0000
Subject: [PATCH] http: response_write handles HEAD responses

Since we special-case MSG_MORE for HEAD responses already,
our HTTP server code now strips response bodies from HEAD
requests unconditionally, allowing .psgi files to omit the
`enable "Head"' directive.

Proper placement of `Head' (Plack::Middleware::Head) in the
middleware stack can be confusing and tedious since it needs to
be after things like AccessLog::Timed which need to wrap the
response body with an object which responds to ->close.

Furthermore, removal of the response body in HEAD requests is
required in all relevant HTTP RFCs (2616, 7231 and 9110) and
doing HEAD response handling in the server itself avoids the
possibility of misconfiguration by the .psgi file maintainer.

The main reason in this rewrite was the desire to easily
configure per-map AccessLog::Timed destination log files.
Correct configuration without this patch would require putting
an `enable "Head"' directive inside every Plack::Builder::map
block after `enable "AccessLog::Timed"'.

In other words, the .psgi file having a single `enable "Head"'
encompassing everything:

	builder {
		enable 'Head';
		map 'http://example.com/' => builder {
			enable 'AccessLog::Timed', @a_params;
		};
		map 'http://example.net/' => builder {
			enable 'AccessLog::Timed', @b_params;
		};
	};

...would not log HEAD requests properly, and the correct alternative:

	builder {
		map 'http://example.com/' => builder {
			enable 'AccessLog::Timed', @a_params;
			enable 'Head';
		};
		map 'http://example.net/' => {
			enable 'AccessLog::Timed', @b_params;
			enable 'Head';
		};
	};

...would be more tedious with many `map' directives.  With this
change, all `enable "Head"' directives can simply be omitted for
public-inbox-{netd,httpd} users.
---
 lib/PublicInbox/HTTP.pm  | 69 +++++++++++++++++++++-------------------
 lib/PublicInbox/HTTPD.pm |  1 -
 2 files changed, 36 insertions(+), 34 deletions(-)

diff --git a/lib/PublicInbox/HTTP.pm b/lib/PublicInbox/HTTP.pm
index f456f3c07..83f2c6a24 100644
--- a/lib/PublicInbox/HTTP.pm
+++ b/lib/PublicInbox/HTTP.pm
@@ -173,9 +173,9 @@ sub app_dispatch {
 # we use the non-standard 499 code for client disconnects (matching nginx)
 sub status_msg ($) { status_message($_[0]) // 'error' }
 
-sub response_header_write ($$$) {
+sub response_write {
 	my ($self, $env, $res) = @_;
-	my $proto = $env->{SERVER_PROTOCOL} or return; # HTTP/0.9 :P
+	my $proto = $env->{SERVER_PROTOCOL}; # HTTP/0.9 isn't supported
 	my $status = $res->[0];
 	my $h = "$proto $status " . status_msg($status) . "\r\n";
 	my ($len, $chunked, $alive, $res_body, $req_head);
@@ -197,11 +197,13 @@ sub response_header_write ($$$) {
 	my $term = defined($len) || $chunked ||
 		Plack::Util::status_with_no_entity_body($status);
 	my $prot_persist = ($proto eq 'HTTP/1.1');
-	if (!$term && ref($res->[2]) eq 'ARRAY') {
+	if (ref($res->[2]) eq 'ARRAY') {
 		($res_body, $res->[2]) = ($res->[2], []);
-		$len = sum0(map length, @$res_body);
-		$h .= "Content-Length: $len\r\n";
-		$term = 1;
+		if (!$term) {
+			$len = sum0(map length, @$res_body);
+			$h .= "Content-Length: $len\r\n";
+			$term = 1;
+		}
 	}
 	if ($conn =~ /\bclose\b/i) {
 		$alive = 0;
@@ -220,16 +222,31 @@ sub response_header_write ($$$) {
 		$h .= "Connection: close\r\n";
 	}
 	$h .= 'Date: ' . http_date() . "\r\n\r\n";
-
+	$self->{alive} = $alive;
+	if ($req_head) {
+		$self->write(\$h);
+		return bless(\$self, 'PublicInbox::HTTP::Null') if !$res->[2];
+		$res->[2]->close if ref($res->[2]) ne 'ARRAY';
+		return response_done($self);
+	}
+	my $do_chunk = ($chunked // 0) == 2 ? ($self->{do_chunk} = 1) : undef;
 	if ($res_body) {
 		$self->writev($h, @$res_body);
-	} elsif (($len || $chunked) && !$req_head) {
+		return response_done($self);
+	}
+	if ($len || $chunked) {
 		msg_more($self, $h);
 	} else {
 		$self->write(\$h);
 	}
-	$self->{alive} = $alive;
-	($chunked // 0) == 2 ? ($self->{do_chunk} = 1) : undef;
+	if ($res->[2]) {
+		$self->{forward} = $res->[2];
+		getline_pull($self); # kick-off!
+	} elsif ($do_chunk) {
+		bless \$self, 'PublicInbox::HTTP::Chunked'
+	} else {
+		bless \$self, 'PublicInbox::HTTP::Identity'
+	}
 }
 
 # middlewares such as Deflater may write empty strings
@@ -295,29 +312,6 @@ sub getline_pull {
 	response_done($self);
 }
 
-sub response_write {
-	my ($self, $env, $res) = @_;
-	my $do_chunk = response_header_write($self, $env, $res);
-	if (defined(my $body = $res->[2])) {
-		if (ref $body eq 'ARRAY') {
-			if ($do_chunk) {
-				chunked_write($self, $_) for @$body;
-			} else {
-				identity_write($self, $_) for @$body;
-			}
-			response_done($self);
-		} else {
-			$self->{forward} = $body;
-			getline_pull($self); # kick-off!
-		}
-	# these are returned to the calling application:
-	} elsif ($do_chunk) {
-		bless \$self, 'PublicInbox::HTTP::Chunked';
-	} else {
-		bless \$self, 'PublicInbox::HTTP::Identity';
-	}
-}
-
 sub input_prepare {
 	my ($self, $env) = @_;
 	my ($input, $len);
@@ -537,4 +531,13 @@ sub write {
 	$http->{sock} ? bytes::length($_[1]) : undef;
 }
 
+package PublicInbox::HTTP::Null; # for HEAD responses
+use v5.12;
+our @ISA = qw(PublicInbox::HTTP::Chunked);
+
+sub write {
+	my $http = ${$_[0]};
+	$http->{sock} ? bytes::length($_[1]) : undef;
+}
+
 1;
diff --git a/lib/PublicInbox/HTTPD.pm b/lib/PublicInbox/HTTPD.pm
index 68ffaebe9..827499ffb 100644
--- a/lib/PublicInbox/HTTPD.pm
+++ b/lib/PublicInbox/HTTPD.pm
@@ -69,7 +69,6 @@ EOM
 Plack::Middleware::ReverseProxy missing,
 URL generation for redirects may be wrong if behind a reverse proxy
 EOM
-			enable 'Head';
 			sub { $www->call(@_) };
 		};
 	}
-- 
2.47.3