Hi,
Pl. find my comments below :
[snip]
Index: modules/ssl/ssl_engine_io.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/ssl/ssl_engine_io.c,v
retrieving revision 1.37
diff -u -r1.37 ssl_engine_io.c
--- modules/ssl/ssl_engine_io.c 2001/10/04 17:50:39 1.37
+++ modules/ssl/ssl_engine_io.c 2001/10/04 19:54:22
@@ -72,14 +72,10 @@
static const char ssl_io_filter[] = "SSL/TLS Filter";
-static int ssl_io_hook_read(SSL *ssl, unsigned char *buf, int len)
+static int ssl_io_hook_read(SSL *ssl, char *buf, int len)
{
int rc;
- if (ssl == NULL) {
- return -1;
- }
-
[snip]
=> Why is the checking not required ?.. If it's justified, then the
corresponding check has to be eliminated from ssl_io_hook_write() also..
[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]
- if (eMode != AP_MODE_NONBLOCKING)
- ap_log_error(APLOG_MARK,APLOG_ERR,ret,NULL,
- "Read failed in ssl input filter");
-
- if ((eMode == AP_MODE_NONBLOCKING) &&
- (APR_STATUS_IS_SUCCESS(ret) || APR_STATUS_IS_EAGAIN(ret)))
- {
- /* In this case, we have data in the output bucket, or we
were
- * non-blocking, so returning nothing is fine.
- */
- return APR_SUCCESS;
- }
- else {
- return ret;
- }
- }
[snip]
Again - why is it deleted ?.. This is also *required* when
AP_MODE_NONBLOCKING is set to true - pl. justify.
[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 */
- }
- }
[snip]
How are the renegotiations handled ?.. SSL doesn't want you to process the
data when it's in the middle of a renegotiation..
[snip]
+ /* Readbytes == 0 implies we only want a LF line.
+ * 1024 seems like a good number for now. */
+ *tempread = 1024;
[snip]
WHAT ?.. SSL is pretty different from the way HTTP works.. During a SSL
handshake, there's a tightly coupled message exchange that goes on b/w
client and server.. client sends something, server interprets and sends back
something, client interprets and communicates back and so on..
The actual size of the data communicated in each phase depends upon the
cert. size, the ciphers available and various other parameters.. You can
never assume that x bytes will be communicated during a phase phase.. (1024
- is a assumption according to me)..
So, if SSL asks for more data (SSL_ERROR_WANT_READ), it means that it wants
*all* the data that's available in the buffer.. If it get buffer less than
the size it is expecting, then it just cries for more data.. We *have* to
handle that condition - understand *how much* more data is it asking for..
The information is *already* available in the SSL context - but I'm afraid
to use them directly.. There should be some API that should tell *how much*
data is SSL expecting from the communication channel.
PS : SSL gurus - pl. correct me if I'm wrong anywhere..
Thx
-Madhu
