You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by "Schreibman, David" <DS...@eTranslate.com> on 2001/08/28 17:16:37 UTC

RE: cvs commit: jakarta-tomcat/src/share/org/apache/tomcat/util/t estHeader.java HttpRequest.java

For whatever it's worth, here are some changes I made to the 3.2.2 code base
to add support for chunked requests.  I just did the work a few days ago and
am still in the process of moving them to 3.3.  Still, since the topic came
up now I thought it would be helpful to share my approach.  I have been
running some heavy stress tests on this and have not seen any problems yet.
It would be great to run the changes through some unit tests as well.

The changes are in Ajp13ConnectorRequest.java, jk_ajp13_worker.c, and a
minor change to mod_jk.c.

The key change in Ajp13ConnectorRequest.java is not to set a limit on the
BufferedServletInputStream when there is no Content-Length.

On key change in jk_ajp13_worker.c was to add a read buffer to the
ajp13_endpoint struct.

This was needed to ensure that calls to ap_get_client_block() (via the
service's read() function) will always ask for a fixed amount.  Without
this, you can end up calling read_client_block() with a buffer that is too
small and it will return an error. This is due to ap_get_client_block()
requiring a "buffer size large enough to hold a chunk-size line".

In mod_jk.c, I made a simple change to avoid possible cross platform issues
due to assigning a long to an unsigned.

I welcome any questions or comments about this approach.  Hopefully, I have
presented the changes in a way that makes sense.  If not, I'll be happy to
take suggestions about how to make the changes easier to understand.

-David


Here are the changes:

In Ajp13ConnectorRequest.java $Revision: 1.5.2.7
234c234,235
<     	((BufferedServletInputStream)this.in).setLimit(contentLength);
---
> 
> 
236,244c237,246
<     		/* Read present data */
<     		int err = con.receive(msg);
<             if(err < 0) {
<             	return -1;
< 			}
< 
<     		blen = msg.peekInt();
<     		msg.getBytes(bodyBuff);
<     	}
---
> 	    ((BufferedServletInputStream)this.in).setLimit(contentLength);
> 	}
> 	/* Read present data */
> 	int err = con.receive(msg);
> 	if(err < 0) {
> 	    return -1;
> 	}
> 	
> 	blen = msg.peekInt();
> 	msg.getBytes(bodyBuff);
306,311c308,313
< 		MsgBuffer msg = con.getMsgBuffer();
< 		msg.appendByte(JK_AJP13_GET_BODY_CHUNK);
< 		msg.appendInt(MAX_READ_SIZE);
< 		con.send(msg);
< 		
< 		int err = con.receive(msg);
---
> 	MsgBuffer msg = con.getMsgBuffer();
> 	msg.appendByte(JK_AJP13_GET_BODY_CHUNK);
> 	msg.appendInt(MAX_READ_SIZE);
> 	con.send(msg);
> 	
> 	int err = con.receive(msg);
313,314c315,316
<         	throw new IOException();
< 		}
---
> 	    throw new IOException();
> 	}
317,319c319,325
<     	pos = 0;
<     	msg.getBytes(bodyBuff);
< 
---
>         if (blen > 0) {
> 	  pos = 0;
> 	  msg.getBytes(bodyBuff);
> 	} else {
>           pos = 0;
> 	  msg.getBytes(bodyBuff);
> 	}

--------------------------------------------------------

In jk_ajp13_worker.c $Revision: 1.3.2.2

111c111,117
<     unsigned left_bytes_to_send;
---
>     int  left_bytes_to_send;
>     int had_content_length;
> 
>     unsigned char readbuf[READ_BUF_SIZE];
>     unsigned left;
>     unsigned int current;
> 
116c122,124
<     ep->reuse = JK_FALSE; 
---
>     ep->reuse = JK_FALSE;
>     ep->left = 0;
>     ep->current = 0;
236c244,246
< static int read_fully_from_server(jk_ws_service_t *s, 
---
> 
> static int read_fully_from_server(ajp13_endpoint_t *ep,
> 				  jk_ws_service_t *s, 
238c248,249
<                                   unsigned len)
---
>                                   unsigned len,
> 				  jk_logger_t *l)
240a252
>     unsigned this_time;
243,245c255,261
<         unsigned this_time = 0;
<         if(!s->read(s, buf + rdlen, len - rdlen, &this_time)) {
<             return -1;
---
> 
>         if (ep->left <= 0) {
>             if(!s->read(s, ep->readbuf, (unsigned)READ_BUF_SIZE,
&(ep->left))) {
> 	        jk_log(l, JK_LOG_DEBUG, "read_fully says -1\n");
> 	        return -1;
>             }
> 	    ep->current = 0;
247a264,268
>         this_time = (ep->left < (len - rdlen)) ? ep->left : (len-rdlen);
> 	memcpy(buf+rdlen, (ep->readbuf)+ep->current, this_time);
> 	ep->left -= this_time;
> 	ep->current += this_time;
> 
249c270
<             break;
---
> 	  break;
251c272,274
< 	    rdlen += this_time;
---
> 
> 	rdlen += this_time;
> 
264c287
< 
---
>     int just_got = 0;
270c293,297
<     if(read_fully_from_server(r, read_buf, len) < 0) {
---
>     if((just_got = read_fully_from_server(ep, r, read_buf, len, l)) < 0) {
>       if (ep->had_content_length == 0) {
> 	ep->left_bytes_to_send = -1;
> 	return JK_TRUE;
>       } else {
273c300,301
<         return JK_FALSE;                        
---
>         return JK_FALSE;
>       }
276c304,307
<     ep->left_bytes_to_send -= len;
---
>     if ((ep->had_content_length == 0) && (just_got != len)) {
>       ep->left_bytes_to_send = -1;/* we are done */
>       len = just_got;
>     }
323c354
< 	            unsigned len = (unsigned)jk_b_get_int(msg);
---
> 	        unsigned len = (unsigned)jk_b_get_int(msg);
335a367,371
> 		if (ep->left_bytes_to_send < 0) {
>                     read_into_msg_buff(ep, r, msg, l, 0);
> 		    return JK_AJP13_HAS_RESPONSE;
> 		}
> 
339c375,377
<                 if(len > ep->left_bytes_to_send) {
---
> 
> 		if (ep->had_content_length) {
> 		  if(len > ep->left_bytes_to_send) {
341c379,381
<                 }
---
> 		  }
> 		}
> 
546c586
< 	    jk_b_reset(msg);
---
> 	jk_b_reset(msg);
548c588,594
<         p->left_bytes_to_send = s->content_length;
---
>         if (s->content_length > 0) {
> 	  p->had_content_length = 1;
> 	  p->left_bytes_to_send = s->content_length;
>         } else {
> 	  p->had_content_length = 0;
> 	  p->left_bytes_to_send = MAX_SEND_BODY_SZ;
> 	}
597d642
<         
617,639c662,683
< 	    while(1) {
<             int rc = 0;
<             
< 		    if(!connection_tcp_get_message(p, msg, l)) {
< 		        jk_log(l, JK_LOG_ERROR,
< 				       "Error reading request\n");
< 		        return JK_FALSE;
< 		    }
< 
<             rc = ajp13_process_callback(msg, p, s, l);
<             if(JK_AJP13_END_RESPONSE == rc) {
<                 return JK_TRUE;
<             } else if(JK_AJP13_HAS_RESPONSE == rc) {
< 		        rc = connection_tcp_send_message(p, msg, l);
< 		        if(rc < 0) {
< 			        jk_log(l, JK_LOG_DEBUG,
< 				           "Error reading response1 %d\n",
rc);
< 			        return JK_FALSE;
< 		        }
<             } else if(rc < 0) {
<                 break; /* XXX error */
<             }
< 	    }        
---
> 	while(1) {
> 	  int rc = 0;
> 	  
> 	  if(!connection_tcp_get_message(p, msg, l)) {
> 	    jk_log(l, JK_LOG_ERROR,
> 		   "Error reading request\n");
> 	    return JK_FALSE;
> 	  }
> 	  rc = ajp13_process_callback(msg, p, s, l);
> 	  if(JK_AJP13_END_RESPONSE == rc) {
> 	    return JK_TRUE;
> 	  } else if(JK_AJP13_HAS_RESPONSE == rc) {
> 	    rc = connection_tcp_send_message(p, msg, l);
> 	    if(rc < 0) {
> 	      jk_log(l, JK_LOG_DEBUG,
> 		     "Error reading response1 %d\n", rc);
> 	      return JK_FALSE;
> 	    }
> 	  } else if(rc < 0) {
> 	    break; /* XXX error */
> 	  }
> 	}        
641,643c685,687
<         jk_log(l, 
<                JK_LOG_ERROR, 
<                "In jk_endpoint_t::service, NULL parameters\n");
---
>       jk_log(l, 
> 	     JK_LOG_ERROR, 
> 	     "In jk_endpoint_t::service, NULL parameters\n");
683a728,729
> 	    ep->left = 0;
> 	    ep->current = 0;


--------------------------------------------------------

In mod_jk.c for apache1.3 (no revision number in the source)

223a224
> 
235c236
<             *a = ap_get_client_block(p->r, b, l);
---
>             *a = (unsigned) ap_get_client_block(p->r, b, l);
238a240
> 


-----Original Message-----
From: Keith Wannamaker [mailto:Keith@Wannamaker.org]
Sent: Tuesday, August 28, 2001 5:40 AM
To: tomcat-dev@jakarta.apache.org
Subject: RE: cvs commit:
jakarta-tomcat/src/share/org/apache/tomcat/util/testHeader.java
HttpRequest.java


| This has nothing to do with tomcat accepting Chunked encoding on the
| request - we did few fixes but this hasn't been tested yet, I believe
| there are few changes in mod_jk we still have to do.
| 

Almost done...

Keith

RE: cvs commit: jakarta-tomcat/src/share/org/apache/tomcat/util/testHeader.java HttpRequest.java

Posted by Keith Wannamaker <Ke...@Wannamaker.org>.
Heh, you beat me to it.  I'd like to compare notes.
Can you resend the diff with -u -b?

Keith

| -----Original Message-----
| From: Schreibman, David [mailto:DSchreibman@eTranslate.com]
| Sent: Tuesday, August 28, 2001 11:17 AM
| To: 'tomcat-dev@jakarta.apache.org'
| Subject: RE: cvs commit:
| jakarta-tomcat/src/share/org/apache/tomcat/util/testHeader.java
| HttpRequest.java
| 
| 
| For whatever it's worth, here are some changes I made to the 3.2.2 code base
| to add support for chunked requests.  I just did the work a few days ago and
| am still in the process of moving them to 3.3.  Still, since the topic came