> Am 05.04.2022 um 09:34 schrieb Ruediger Pluem <rpl...@apache.org>:
>
>
>
> On 4/5/22 9:13 AM, Stefan Eissing wrote:
>>
>>
>>> Am 04.04.2022 um 16:07 schrieb Ruediger Pluem <rpl...@apache.org>:
>>>
>>>
>>>
>>> On 4/4/22 1:08 PM, ic...@apache.org wrote:
>>>> Author: icing
>>>> Date: Mon Apr 4 11:08:58 2022
>>>> New Revision: 1899552
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1899552&view=rev
>>>> Log:
>>>> *) mod_http: genereate HEADERS buckets for trailers
>>>> mod_proxy: forward trailers on chunked request encoding
>>>> test: add http/1.x test cases in pytest
>>>>
>>>>
>>>> Added:
>>>> httpd/httpd/trunk/test/modules/http1/
>>>> httpd/httpd/trunk/test/modules/http1/__init__.py
>>>> httpd/httpd/trunk/test/modules/http1/conftest.py
>>>> httpd/httpd/trunk/test/modules/http1/env.py
>>>> httpd/httpd/trunk/test/modules/http1/htdocs/
>>>> httpd/httpd/trunk/test/modules/http1/htdocs/cgi/
>>>> httpd/httpd/trunk/test/modules/http1/htdocs/cgi/files/
>>>> httpd/httpd/trunk/test/modules/http1/htdocs/cgi/files/empty.txt
>>>> httpd/httpd/trunk/test/modules/http1/htdocs/cgi/hello.py (with props)
>>>> httpd/httpd/trunk/test/modules/http1/htdocs/cgi/upload.py (with props)
>>>> httpd/httpd/trunk/test/modules/http1/mod_h1test/
>>>> httpd/httpd/trunk/test/modules/http1/mod_h1test/mod_h1test.c
>>>> httpd/httpd/trunk/test/modules/http1/mod_h1test/mod_h1test.slo
>>>> httpd/httpd/trunk/test/modules/http1/test_001_alive.py
>>>> httpd/httpd/trunk/test/modules/http1/test_003_get.py
>>>> httpd/httpd/trunk/test/modules/http1/test_004_post.py
>>>> httpd/httpd/trunk/test/modules/http1/test_005_trailers.py
>>>> httpd/httpd/trunk/test/modules/http1/test_006_unsafe.py
>>>> httpd/httpd/trunk/test/modules/http1/test_007_strict.py
>>>> Modified:
>>>> httpd/httpd/trunk/modules/http/http_filters.c
>>>> httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
>>>> httpd/httpd/trunk/test/modules/http2/env.py
>>>> httpd/httpd/trunk/test/pyhttpd/conf.py
>>>> httpd/httpd/trunk/test/pyhttpd/env.py
>>>>
>>>> Modified: httpd/httpd/trunk/modules/http/http_filters.c
>>>> URL:
>>>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_filters.c?rev=1899552&r1=1899551&r2=1899552&view=diff
>>>> ==============================================================================
>>>> --- httpd/httpd/trunk/modules/http/http_filters.c (original)
>>>> +++ httpd/httpd/trunk/modules/http/http_filters.c Mon Apr 4 11:08:58 2022
>>>> @@ -1312,6 +1312,15 @@ AP_DECLARE_NONSTD(int) ap_send_http_trac
>>>> return DONE;
>>>> }
>>>>
>>>> +static apr_bucket *create_trailers_bucket(request_rec *r,
>>>> apr_bucket_alloc_t *bucket_alloc)
>>>> +{
>>>> + if (r->trailers_out && !apr_is_empty_table(r->trailers_out)) {
>>>> + ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, r, "sending trailers");
>>>> + return ap_bucket_headers_create(r->trailers_out, r->pool, bucket_alloc);
>>>> + }
>>>> + return NULL;
>>>> +}
>>>> +
>>>> typedef struct header_filter_ctx {
>>>> int headers_sent;
>>>> } header_filter_ctx;
>>>> @@ -1323,7 +1332,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_
>>>> conn_rec *c = r->connection;
>>>> int header_only = (r->header_only || AP_STATUS_IS_HEADER_ONLY(r->status));
>>>> const char *protocol = NULL;
>>>> - apr_bucket *e;
>>>> + apr_bucket *e, *eos = NULL;
>>>> apr_bucket_brigade *b2;
>>>> header_struct h;
>>>> header_filter_ctx *ctx = f->ctx;
>>>> @@ -1364,6 +1373,10 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_
>>>> eb = e->data;
>>>> continue;
>>>> }
>>>> + if (APR_BUCKET_IS_EOS(e)) {
>>>> + if (!eos) eos = e;
>>>> + continue;
>>>> + }
>>>> /*
>>>> * If we see an EOC bucket it is a signal that we should get out
>>>> * of the way doing nothing.
>>>> @@ -1416,6 +1429,22 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_
>>>> goto out;
>>>> }
>>>>
>>>> + if (eos) {
>>>> + /* on having seen EOS and added possible trailers, we
>>>> + * can remove this filter.
>>>> + */
>>>> + e = create_trailers_bucket(r, b->bucket_alloc);
>>>> + if (e) {
>>>> + APR_BUCKET_INSERT_BEFORE(eos, e);
>>>> + }
>>>> + ap_remove_output_filter(f);
>>>> + }
>>>> +
>>>> + if (ctx->headers_sent) {
>>>> + /* we did already the stuff below, just pass on */
>>>> + return ap_pass_brigade(f->next, b);
>>>> + }
>>>> +
>>>
>>> I guess this does not work for header_only requests with trailing headers.
>>
>> The thought came up if we want/need/may support that. Can/should a 304 have
>> trailers if the unconditional request had?
>
> What about HEAD requests where the corresponding GET has trailers?
Looking at https://datatracker.ietf.org/doc/draft-ietf-httpbis-semantics/
ch. 15.3.5 "204 No Content": "A 204 response is terminated by the end of the
header section; it
cannot contain content or trailers."
ch. 15.4.5 "304": "A 304 response is terminated by the end of the header
section; it
cannot contain content or trailers."
ch. 9.3.2. "HEAD": "However, a server MAY omit header fields for which a value
is
determined only while generating the content."
The first 2 are pretty clear. On HEAD responses, the server internally only
discards the
content (eg reads to nothing), if the connection is not tagged for closing. If
we allow
trailers in HEAD responses, this becomes unreliable. The overall connection
state would
determine the response. I think we should avoid that.
In addition, for HTTP/2 HEAD responses, the body is never read by our
implementation.
Related to all this, but outside the proposed changes so far: I think we should
switch
to "chunked" encoding in our HTTP/1.1 responses when a "Trailers" header is
set. That way
a loaded module that, for example, generates content digests could signal that
it will generate trailers.
Kind Regards,
Stefan
> Regards
>
> Rüdiger
