On Thu, Mar 2, 2023 at 8:22 PM Ruediger Pluem <rpl...@apache.org> wrote:
>
> On 3/2/23 7:21 PM, Christophe JAILLET wrote:
> > Le 02/03/2023 à 16:10, yla...@apache.org a écrit :
> >> @@ -313,18 +313,16 @@ static int uwsgi_response(request_rec *r
> >>       pass_bb = apr_brigade_create(r->pool, c->bucket_alloc);
> >>         len = ap_getline(buffer, sizeof(buffer), rp, 1);
> >> -
> >>       if (len <= 0) {
> >> -        /* oops */
> >> +        /* invalid or empty */
> >>           return HTTP_INTERNAL_SERVER_ERROR;
> >>       }
> >> -
> >>       backend->worker->s->read += len;
> >> -
> >> -    if (len >= sizeof(buffer) - 1) {
> >> -        /* oops */
> >> +    if ((apr_size_t)len >= sizeof(buffer)) {
> >
> > Hi Yann,
> >
> > Why removing the -1?
> >
> > My understading is that it is there in case of:
> >   uwsgi_response()
> >     ap_getline()
> >       ap_rgetline()
> >         ap_fgetline_core()
> >           code around cleanup:
> >
> > In this path, IIUC, sizeof(buffer) - 1 is returned.
> > Can this happen?
>
> I think ap_fgetline_core can only return a len of sizeof(buffer) - 1 when:
>
> 1. The line is really that long.
> 2. The line is longer, but then rv != APR_SUCCESS.
>
> In case 1. all is fine and we should proceed the result.
> In case 2. ap_getline will return sizeof(buffer).
>
> Hence I think the change is correct.

Yes exactly, ap_fgetline_core() returns APR_ENOSPC if the buffer is
truncated, and ap_getline() returns sizeof(buffer).
This change avoids failing unnecessarily for a valid response line of
exactly sizeof(buffer)-1 bytes length.

Regards;
Yann.

>
> Regards
>
> Rüdiger
>

Reply via email to