> Am 05.04.2022 um 10:20 schrieb Ruediger Pluem <rpl...@apache.org>:
>
>
>
> On 4/5/22 9:53 AM, Stefan Eissing wrote:
>>
>>
>>> 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.
>
> So the proposal is to drop possible trailers on HEAD requests, which is what
> the code above does?
I think that is the only sane thing to do. Because we do not (and do not want
to) guarantee that content is processed for HEAD requests.
>
> Regards
>
> Rüdiger
