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.

diff --git a/lib/PublicInbox/HTTP.pm b/lib/PublicInbox/HTTP.pm
index f456f3c07b248328cb38853dc8c426808ecef25f..83f2c6a24dccd8916a5d95a25de0e0d87c1d2ae4 100644 (file)
--- 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 68ffaebe97b9b9cc080d46c487fb6a4502776f02..827499ffb0aa6c7b52bd159553cdea2959351049 100644 (file)
--- 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(@_) };
                };
        }