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?
Regards
Rüdiger
