-----Original Message-----
From: Justin Erenkrantz [mailto:[EMAIL PROTECTED]]
Sent: Thursday, October 04, 2001 4:51 PM
[snip]
> - if (ssl == NULL) {
> - return -1;
> - }
> -
> => Why is the checking not required ?.. If it's justified, then the
> corresponding check has to be eliminated from ssl_io_hook_write() also..
Yeah, it should be removed from ssl_io_hook_write as well. But,
I wasn't changing that function. =) Checking for null on something
like this isn't our preferred style - if it is NULL, then something
really bad happened anyway. (ssl can't be null under any normal
circumstances here if you see how the code is called - it just
doesn't add any value...)
[snip]
I'm always of the opinion that bailing out is a correct way to go. It's
possible that the ssl_io_hook_read be registered directly with (Open)SSL to
perform the read/write operations - in which case, bailing out is a better
alternative.. Whatever..
[snip]
> -
> - /* read filter */
> - ret = apr_bucket_read(pbktIn, &data, &len, (apr_read_type_e)eMode);
> - if (!(eMode == AP_MODE_NONBLOCKING && APR_STATUS_IS_EAGAIN(ret)))
{
> - /* allow retry */
> - APR_BUCKET_REMOVE(pbktIn);
> - }
> - if (ret == APR_SUCCESS && len == 0 && eMode == AP_MODE_BLOCKING)
> - ret = APR_EOF;
> - if (len == 0) {
> - /* Lazy frickin browsers just reset instead of shutting down.
> */
> - /* also gotta handle timeout of keepalive connections */
> - if (ret == APR_EOF || APR_STATUS_IS_ECONNRESET(ret) ||
> - ret == APR_TIMEUP)
> - {
> - if (APR_BRIGADE_EMPTY(pRec->pbbPendingInput))
> - return APR_EOF;
> - else
> - /* Next time around, the incoming brigade will be empty,
> - * so we'll return EOF then
> - */
> - return APR_SUCCESS;
> - }
> -
> [snip]
>
> Why was this removed ?.. The reason why the (len == 0) checking is being
> done is to take care of browsers like IE, which just closes the connection
> upon termination, or a handshake failure..
[snip]
Any errors such as these will be handled by CORE_IN and it will
pass back the correct error code. mod_ssl shouldn't do anything
here. Remember, previously, it had to handle all of the socket
code - that is gone now.
[snip]
Tell me if I'm missing something or my eyes are playing tricks here.. Except
for the if (len == 0) loop, I don't see anything much that's missing from
here.. If that's what you're referring, then fine.
[snip]
> - if (bio_is_renegotiating(pRec->pbioRead)) {
> - /* we're doing renegotiation in the access phase */
> - if (len >= *readbytes) {
> - /* trick ap_http_filter into leaving us alone */
> - *readbytes = 0;
> - break; /* SSL_renegotiate will take it from here */
> - }
> - }
>
> How are the renegotiations handled ?.. SSL doesn't want you to process the
> data when it's in the middle of a renegotiation..
This trick doesn't get you anything if you look at the code path.
The renegotiation is handled by the process_connection hook.
[snip]
'not necessarily. The renegotiation request can come from the
ssl_hook_Access() also - in which case ssl_hook_process_connection has no
business whatsoever..
[snip]
Remember - we are only asking for *AT MOST* readbytes. There is
no contract (this is what Greg and I have been pointing out) that
you will receive exactly readbytes worth of data. In fact, with
mod_ssl, I'll bet you will often get back less. But, it definitely
doesn't make sense to try to increase readbytes in mod_ssl to
handle it. In most cases, I'm going to think that the data will
be an almost 1<->1 comparison (one encyrpted byte may be worth
one unencrypted byte). So, I don't think it would any value to
try to satisfy the complete readbytes. We should and often
will send back less than what they asked for.
[snip]
I generally don't like the idea. It's really a *great* thing to have for
HTTP filters.. But, I have some concerns :
Some applications may legitimately ask for x bytes, some may ask for a upper
limit of x bytes, and some other may want to have all the data in the
channel (SSL)..
I'm a novice here and 'obviously missing something - can somebody tell me
why should a application not be given whatever it's asking for - especially
if it's geniune (think SSL) ?.. Also, I guess there has to be a
differentiator b/w a protocol and a application here.. A protocol should to
be given all the data it asks for (and in the format it asks for) - the
interpretation of the data has to be minimal..
It may so happen that a protocol is designed such that it first reads all
the data available and then starts operating on it. I agree that it may be a
bad design - but the requirements may be such - are we telling that such
applications *cannot* work with Apache ?..
Also, consider the performance impact that this would have on the SSL
transactions - take the case of a busy site where ssl data + certificates is
of length 2k (some random size > 1024).. For each transaction, the
SSL_accept is called atleast once more than the current model (because of
the lack of data given to it).. Taking into account how costly each SSL
call can be, the new method would be introducing a substantial amount of
delay. The SSL performance of Zeus webserver is something like 3x of Apache
- every extra transaction in Apache matters. (I'm not trying to get into
Zeus/Apache comparision here, but I'm just trying to see where Apache+SSL is
heading towards).
I'll probably to go thru' another round of learning filters..
'just my thoughts,
-Madhu
