httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: mod_http2 behavior in case of too many or too large request headers
Date Thu, 10 Sep 2020 09:24:06 GMT


On 9/10/20 9:31 AM, Stefan Eissing wrote:
> 
> 
>> Am 10.09.2020 um 09:29 schrieb Ruediger Pluem <rpluem@apache.org>:
>>
>>
>>
>> On 9/9/20 10:21 AM, Stefan Eissing wrote:
>>>
>>>
>>>> Am 08.09.2020 um 21:11 schrieb Ruediger Pluem <rpluem@apache.org>:
>>>>
>>>>
>>>>
>>>> On 9/8/20 9:22 AM, Stefan Eissing wrote:
>>>>>
>>>>>
>>>>>> Am 08.09.2020 um 08:27 schrieb Ruediger Pluem <rpluem@apache.org>:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 8/21/20 9:20 AM, Ruediger Pluem wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 8/20/20 11:38 AM, Stefan Eissing wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>> Am 20.08.2020 um 11:35 schrieb Ruediger Pluem <rpluem@apache.org>:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 8/20/20 10:47 AM, Stefan Eissing wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> Am 20.08.2020 um 10:01 schrieb Ruediger Pluem
<rpluem@apache.org>:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 8/19/20 12:18 PM, Stefan Eissing wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>> Am 19.08.2020 um 12:08 schrieb Ruediger
Pluem <rpluem@apache.org>:
>>
>>>>>> Any feedback or comments?
>>>>>
>>>>> Sorry about the delay, my inbox in unhealthy these days.
>>>>
>>>> No problem. Even more thanks then for taking time for a review.
>>>
>>> Thanks for improving this.
>>>>
>>>>>
>>>>> Had a quick look. My read: it looks like a good approach. The request
still needs to be processed in a worker, but that should be very light and fast. I was first
confused by the "early_http_status" term as there is the "103 early hints" intermediate response
code stuck in my brain. Maybe we should just call it http_status and have a HTTP_NEEDS_FURTHER_PROCESSING
(0) in our server.
>>>>
>>>> Updated the PR and renamed early_http_status to http_status.
>>>> What do you mean with / what is the purpose of HTTP_NEEDS_FURTHER_PROCESSING?
Should http_status be initialized with this define
>>>> or do you want to replace conditions of the type (http_status) with (http_status
!= HTTP_NEEDS_FURTHER_PROCESSING)?
>>>> At what place should we define it? In h2.h?
>>>
>>> Just a thought that 0 could indicate that the http status has not been determined
yet (default) or in case of an early error the code to return. Which then prevents further
processing. The name for such a value was not entirely serious. We could just check on !=
0.
>> It is already the case that a value of 0 in http_status indicates that the http status
has not been determined yet and 0 is
>> already the default value via the apr_pcalloc of the structure.
>>
>> Would you like to see the following?
>>
>> 1. Make a define like HTTP_NEEDS_FURTHER_PROCESSING (I would propose H2_HTTP_STATUS_UNSET,
sweet naming discussion :-))
>> 2. In addition to the apr_pcalloc which already makes http_status zero set http_status
explicitly to H2_HTTP_STATUS_UNSET.
>> 3. Replace the (http_status) conditions in the ifs with (http_status != H2_HTTP_STATUS_UNSET)
> 
> I like that very much. I think it makes good reading.

That was a quick reply. So the now updated PR is fine for you?

Regards

RĂ¼diger

Mime
View raw message