Mailing List Archive

1 2  View All
Re: svn commit: r1874775 - /httpd/httpd/trunk/test/README.travis [ In reply to ]
On 4/1/20 9:53 AM, Joe Orton wrote:
> On Wed, Apr 01, 2020 at 09:24:27AM +0200, Ruediger Pluem wrote:
>> I have checked socket, pipe and cgi buckets and none of them seem to return EOF. In case they hit EOF they replace themselves with
>> an immortal bucket of length 0. So above seems just an additional safety if a (future) morphing bucket behaves differently and
>> returns EOF, but with the current code that path should not be hit really.
>
> Yeah, the "read till EOF" is an implementation detail for those bucket
> types, I would very strongly argue if they ever exposed EOF on read() it
> would be a bucket type bug. It could quite possibly obscure a bug
> elsewhere if filters ignored EOF on read() so I don't think that should
> be recommended.

So you recommend that part of the patch to be reverted?

Regards

RĂ¼diger
Re: svn commit: r1874775 - /httpd/httpd/trunk/test/README.travis [ In reply to ]
On Wed, Apr 01, 2020 at 10:06:20AM +0200, Ruediger Pluem wrote:
>
>
> On 4/1/20 9:53 AM, Joe Orton wrote:
> > On Wed, Apr 01, 2020 at 09:24:27AM +0200, Ruediger Pluem wrote:
> >> I have checked socket, pipe and cgi buckets and none of them seem to return EOF. In case they hit EOF they replace themselves with
> >> an immortal bucket of length 0. So above seems just an additional safety if a (future) morphing bucket behaves differently and
> >> returns EOF, but with the current code that path should not be hit really.
> >
> > Yeah, the "read till EOF" is an implementation detail for those bucket
> > types, I would very strongly argue if they ever exposed EOF on read() it
> > would be a bucket type bug. It could quite possibly obscure a bug
> > elsewhere if filters ignored EOF on read() so I don't think that should
> > be recommended.
>
> So you recommend that part of the patch to be reverted?

Thinking about it more, the most likely way to hit that condition is a
FILE bucket where the underlying file is truncated simultaneous to it
being read (i.e. it's shorter than when the bucket was created). That
will definitely result in apr_bucket_read() returning EOF, and I think
should definitely be treated as an error rather than silently ignored.

TL;DR -> yes, or am I missing something?
Re: svn commit: r1874775 - /httpd/httpd/trunk/test/README.travis [ In reply to ]
On 4/1/20 11:02 AM, Joe Orton wrote:
> On Wed, Apr 01, 2020 at 10:06:20AM +0200, Ruediger Pluem wrote:
>>
>>
>> On 4/1/20 9:53 AM, Joe Orton wrote:
>>> On Wed, Apr 01, 2020 at 09:24:27AM +0200, Ruediger Pluem wrote:
>>>> I have checked socket, pipe and cgi buckets and none of them seem to return EOF. In case they hit EOF they replace themselves with
>>>> an immortal bucket of length 0. So above seems just an additional safety if a (future) morphing bucket behaves differently and
>>>> returns EOF, but with the current code that path should not be hit really.
>>>
>>> Yeah, the "read till EOF" is an implementation detail for those bucket
>>> types, I would very strongly argue if they ever exposed EOF on read() it
>>> would be a bucket type bug. It could quite possibly obscure a bug
>>> elsewhere if filters ignored EOF on read() so I don't think that should
>>> be recommended.
>>
>> So you recommend that part of the patch to be reverted?
>
> Thinking about it more, the most likely way to hit that condition is a
> FILE bucket where the underlying file is truncated simultaneous to it
> being read (i.e. it's shorter than when the bucket was created). That
> will definitely result in apr_bucket_read() returning EOF, and I think
> should definitely be treated as an error rather than silently ignored.
>
> TL;DR -> yes, or am I missing something?
>

I think you are correct. This should be an error as it can only happen in error cases.

Regards

RĂ¼diger

1 2  View All