Hmmm, replied back to the wrong list, user versus developers, for this.

Anyway, I'll admit I haven't looked at actual patches yet, but some
more comments about some of what is being addressed.

In doing changes to this area of code, especially if you are going to
want to support chunked encoding somehow, you also probably need to
address:

  http://issues.apache.org/jira/browse/MODPYTHON-240

That is, deal with the LimitRequestBody directive properly.

To do this though, you have to put the check for it before you even
call into the user handler code, preferably even before you leave the
C code.

The check for this is:

    limit = ap_get_limit_req_body(r);

    if (limit && limit < r->remaining) {
        ap_discard_request_body(r);
        return OK;
    }

To be able to call this though, you must have first called
ap_setup_client_block() else the required attributes of request_rec
will not have been set yet.

If you aren't going to support chunked encoding that is fine, as just use:

    status = ap_setup_client_block(r, REQUEST_CHUNKED_ERROR);

    if (status != OK)
        return status;

prior to check of request body length.

If you want to support chunked however it gets more complicated. This
is because in your way of doing things it sounds like the policy of
how request content is handled is set by calling attribute of
mod_python request object. At the point where the initialisation would
now need to go, because of request body length check, no user code
would have been called and so no opportunity for policy to be set.

What probably needs to be investigated is whether the default can't
just be REQUEST_CHUNKED_DECHUNK all the time and code behave just as
it did before. If this can be done, then all that needs to be solved
is your ability to do non blocking reads.

For non blocking reads, I rather don't like the idea of a special read
function. What I have instead thought about for this in the past for
mod_wsgi, even though support for chunked and non blocking is outside
of WGSI specification, is to have a setblocking() function just like
sockets do. Technically one may even be able to implement the more
general settimeout() since timeouts can be specified for Apache
connections if you do it right.

The only thing is what exception you use to indicate socket would
block and/or timeout. Do you reuse socket exception and use it in same
way as sockets do for indicating this, or use a new exception.

I might have another think about how to do this for mod_wsgi case as
probably still applicable to mod_python.

Graham

2008/7/17 Graham Dumpleton <[EMAIL PROTECTED]>:
> Thanks for adding the patches.
>
> If I ever get a chance, I'll compare your patches against completely
> rewritten equivalent code I have in mod_wsgi and see how they compare
> as far as satisfying the required functionality. Although, the
> mod_wsgi code doesn't attempt to handled chunked as WSGI specification
> doesn't allow it.
>
> Graham
>
> 2008/7/16 Alex Cichowski <[EMAIL PROTECTED]>:
>> OK, I have added the patch under MODPYTHON-222 (chunked requests) and
>> referenced it under the others.
>>
>> For 234 (read() SystemError), if Matthew Woodcraft is correct about the
>> unchecked call to ap_get_client_block() being the problem, then yes this
>> patch will fix it.
>>
>> The original patch did not address 211 (readlines() leak), but for
>> completeness I have added a rewrite of readlines() that is more rigorous in
>> its error handling and memory management to the patch in JIRA.
>>
>> The revised patch also includes a new function, read_partial(), which
>> attempts to ensure that it has returned all buffered data before blocking,
>> by returning less than the requested amount where necessary. This is helpful
>> to avoid deadlock when you would like to have the server read and respond to
>> the first part of the client's request before the client sends the second
>> part. I don't know whether it is strictly valid to use HTTP in this way or
>> not, but I think it's nice to have this feature available anyway.
>>
>> Alex
>>
>>
>>
>> Graham Dumpleton wrote:
>>>
>>> Can you possibly add the suggested patches against the issues in JIRA,
>>> otherwise they will get lost in the mailing list.
>>>
>>> Does, your suggested patches address:
>>>
>>>  http://issues.apache.org/jira/browse/MODPYTHON-234
>>>
>>>  http://issues.apache.org/jira/browse/MODPYTHON-211
>>>
>>> Graham
>>>
>>
>>
>

Reply via email to