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/09/05 21:09:50 UTC
Chunked input in 3.3-b2
I downloaded the 3.3-b2 sources to investigate the issue below further.
However, I am unable to get chunked input to work at all. It's almost as if
the Java side is declaring eof before reading all the data available on the
apache side. I have confirmed that mod_jk thinks the request is chunked.
I'm going to keep digging to get at the root of the problem, but thought I
would send in a report in case anyone has any ideas (btw, I'm running Apache
1.3.20).
>From looking over the code, I'm now pretty sure the issue discussed below is
also going to be a problem. I won't know for sure until I get past the
current problem and get to do my stress tests.
-David
-----Original Message-----
From: Schreibman, David
Sent: Tuesday, September 04, 2001 11:27 AM
To: 'tomcat-dev@jakarta.apache.org'
Subject: RE: cvs commit:
jakarta-tomcat/src/share/org/apache/tomcat/modules/server Ajp13.java
Oops. Sorry for the typo.
About
>... "chunk-size [ chunk-ext ] CRLF", as defined in rfc 2069. That's ...
I meant rfc 2068!
-David
-----Original Message-----
From: Schreibman, David
Sent: Tuesday, September 04, 2001 11:03 AM
To: 'tomcat-dev@jakarta.apache.org'; jakarta-tomcat-cvs@apache.org
Subject: RE: cvs commit:
jakarta-tomcat/src/share/org/apache/tomcat/modules/server Ajp13.java
It's great to see this work on chunked input progressing!
It's hard for me to see if it's covered here, so I wanted to double check
whether this code addresses a subtle bug that I ran into while getting
chunked input working on 3.2.2
For me, the problem came up when read_fully_from_server() would call read()
with a very small value of len.
The bug would not manifest itself all the time so I didn't notice it for a
while. The problem occurs only when you happen to call
ap_get_client_block() with a very small value of len and
ap_get_client_block() needs to start a new chunk. By small value, I mean
anything less than the length of "chunk-size [ chunk-ext ] CRLF", as defined
in rfc 2069. That's because ap_get_client_block() actually uses the buffer
that is passed in, to read the chunk-size data.
The comments for ap_get_client_block in http_protocol.c indicate the
requirement for the caller to provide enough buffer space to deal with this.
You can luck out a lot of the time, but if you hit this case
ap_get_client_block() will fail. I only spotted this while doing a stress
test that was repeatedly posting 100MB.
It know it was heavy handed but this is what lead me to set up a separate
read buffer for each endpoint. That way I would be sure to always call
ap_get_client_block() with a generously sized buffer. I don't blame you for
avoiding this approach.
Again, my apologies is this is obviously covered. Just wanted to make sure
this was addressed in the latest changes (and bring it up while my memory on
the topic is still fresh).
Thanks,
-David
-----Original Message-----
From: keith@apache.org [mailto:keith@apache.org]
Sent: Friday, August 31, 2001 6:53 PM
To: jakarta-tomcat-cvs@apache.org
Subject: cvs commit:
jakarta-tomcat/src/share/org/apache/tomcat/modules/server Ajp13.java
keith 01/08/31 18:53:25
Modified: src/native/mod_jk/apache1.3 mod_jk.c
src/native/mod_jk/common jk_ajp13_worker.c jk_service.h
jk_util.c
src/share/org/apache/tomcat/modules/server Ajp13.java
Log:
Upgrade mod_jk to support chunked input from Apache 1.3.
TODO: port to Apache 2.0 module. (And forward port to j-t-c)
Revision Changes Path
1.10 +22 -11 jakarta-tomcat/src/native/mod_jk/apache1.3/mod_jk.c
Index: mod_jk.c
===================================================================
RCS file: /home/cvs/jakarta-tomcat/src/native/mod_jk/apache1.3/mod_jk.c,v
retrieving revision 1.9
retrieving revision 1.10
diff -u -r1.9 -r1.10
--- mod_jk.c 2001/06/01 08:41:13 1.9
+++ mod_jk.c 2001/09/01 01:53:25 1.10
@@ -267,16 +267,19 @@
if(s && s->ws_private && b && actually_read) {
apache_private_data_t *p = s->ws_private;
if(!p->read_body_started) {
- if(!ap_setup_client_block(p->r, REQUEST_CHUNKED_DECHUNK)) {
- if(ap_should_client_block(p->r)) {
- p->read_body_started = JK_TRUE;
- }
+ if(ap_should_client_block(p->r)) {
+ p->read_body_started = JK_TRUE;
}
}
- if(p->read_body_started) {
- *actually_read = ap_get_client_block(p->r, b, len);
- return JK_TRUE;
+ if(p->read_body_started) {
+ long rv;
+ if ((rv = ap_get_client_block(p->r, b, len)) < 0) {
+ *actually_read = 0;
+ } else {
+ *actually_read = (unsigned) rv;
+ }
+ return JK_TRUE;
}
}
return JK_FALSE;
@@ -454,7 +457,9 @@
s->server_software = (char *)ap_get_server_version();
s->method = (char *)r->method;
- s->content_length = get_content_length(r);
+ s->content_length = get_content_length(r);
+ s->is_chunked = r->read_chunked;
+ s->no_more_chunks = 0;
s->query_string = r->args;
s->req_uri = r->uri;
@@ -784,10 +789,16 @@
{
/* Retrieve the worker name stored by jk_translate() */
const char *worker_name = ap_table_get(r->notes, JK_WORKER_ID);
+ int rc;
if(r->proxyreq) {
return HTTP_INTERNAL_SERVER_ERROR;
- }
+ }
+
+ /* Set up r->read_chunked flags for chunked encoding, if present */
+ if(rc = ap_setup_client_block(r, REQUEST_CHUNKED_DECHUNK)) {
+ return rc;
+ }
if(worker_name) {
jk_server_conf_t *conf =
@@ -820,12 +831,12 @@
l,
&is_recoverable_error);
- if (s.content_read < s.content_length) {
+ if (s.content_read < s.content_length ||
s.is_chunked) {
/*
* If the servlet engine didn't consume all of
the
* request data, consume and discard all further
* characters left to read from client
- */
+ */
char *buff = ap_palloc(r->pool, 2048);
if (buff != NULL) {
int rd;
1.11 +40 -21
jakarta-tomcat/src/native/mod_jk/common/jk_ajp13_worker.c
Index: jk_ajp13_worker.c
===================================================================
RCS file:
/home/cvs/jakarta-tomcat/src/native/mod_jk/common/jk_ajp13_worker.c,v
retrieving revision 1.10
retrieving revision 1.11
diff -u -r1.10 -r1.11
--- jk_ajp13_worker.c 2001/08/23 15:41:20 1.10
+++ jk_ajp13_worker.c 2001/09/01 01:53:25 1.11
@@ -263,6 +263,10 @@
unsigned len)
{
unsigned rdlen = 0;
+
+ if (s->is_chunked && s->no_more_chunks) {
+ return 0;
+ }
while(rdlen < len) {
unsigned this_time = 0;
@@ -270,45 +274,60 @@
return -1;
}
- if(0 == this_time) {
+ if(0 == this_time) {
+ if (s->is_chunked) {
+ s->no_more_chunks = 1; /* read no more */
+ }
break;
}
- rdlen += this_time;
+ rdlen += this_time;
}
return (int)rdlen;
}
-
+
+/* Returns -1 on error, else number of bytes read */
static int read_into_msg_buff(ajp13_endpoint_t *ep,
jk_ws_service_t *r,
jk_msg_buf_t *msg,
jk_logger_t *l,
unsigned len)
{
- unsigned char *read_buf = jk_b_get_buff(msg);
+ unsigned char *read_buf = jk_b_get_buff(msg);
jk_b_reset(msg);
read_buf += AJP13_HEADER_LEN; /* leave some space for the buffer
headers */
read_buf += AJP13_HEADER_SZ_LEN; /* leave some space for the read
length */
-
- if(read_fully_from_server(r, read_buf, len) < 0) {
+
+ /* Pick the max size since we don't know the content_length */
+ if (r->is_chunked && len == 0) {
+ len = MAX_SEND_BODY_SZ;
+ }
+
+ if((len = read_fully_from_server(r, read_buf, len)) < 0) {
jk_log(l, JK_LOG_ERROR,
"read_into_msg_buff: Error - read_fully_from_server
failed\n");
- return JK_FALSE;
+ return -1;
}
-
- ep->left_bytes_to_send -= len;
-
- if(0 != jk_b_append_int(msg, (unsigned short)len)) {
- jk_log(l, JK_LOG_ERROR,
- "read_into_msg_buff: Error - jk_b_append_int failed\n");
- return JK_FALSE;
+
+ if (!r->is_chunked) {
+ ep->left_bytes_to_send -= len;
}
- jk_b_set_len(msg, jk_b_get_len(msg) + len);
+ if (len > 0) {
+ /* Recipient recognizes empty packet as end of stream, not
+ an empty body packet */
+ if(0 != jk_b_append_int(msg, (unsigned short)len)) {
+ jk_log(l, JK_LOG_ERROR,
+ "read_into_msg_buff: Error - jk_b_append_int
failed\n");
+ return -1;
+ }
+ }
+
+ jk_b_set_len(msg, jk_b_get_len(msg) + len);
- return JK_TRUE;
+ return len;
}
static int ajp13_process_callback(jk_msg_buf_t *msg,
@@ -370,7 +389,7 @@
}
/* the right place to add file storage for upload */
- if(read_into_msg_buff(ep, r, pmsg, l, len)) {
+ if((len = read_into_msg_buff(ep, r, pmsg, l, len)) >= 0) {
r->content_read += len;
return JK_AJP13_HAS_RESPONSE;
}
@@ -636,11 +655,11 @@
* for resend if the remote Tomcat is down, a fact we will
learn only
* doing a read (not yet)
*/
- if(p->left_bytes_to_send > 0) {
+ if(s->is_chunked || p->left_bytes_to_send > 0) {
unsigned len = p->left_bytes_to_send;
- if(len > MAX_SEND_BODY_SZ)
+ if(len > MAX_SEND_BODY_SZ)
len = MAX_SEND_BODY_SZ;
- if(!read_into_msg_buff(p, s, op->post, l, len)) {
+ if((len = read_into_msg_buff(p, s, op->post, l,
len)) < 0) {
/* the browser stop sending data, no need to
recover */
op->recoverable = JK_FALSE;
return JK_FALSE;
1.5 +5 -3 jakarta-tomcat/src/native/mod_jk/common/jk_service.h
Index: jk_service.h
===================================================================
RCS file: /home/cvs/jakarta-tomcat/src/native/mod_jk/common/jk_service.h,v
retrieving revision 1.4
retrieving revision 1.5
diff -u -r1.4 -r1.5
--- jk_service.h 2001/03/06 20:21:58 1.4
+++ jk_service.h 2001/09/01 01:53:25 1.5
@@ -142,8 +142,10 @@
unsigned server_port;
char *server_software;
unsigned content_length; /* integer that represents the content
*/
- /* length should be 0 if unknown.
*/
- unsigned content_read;
+ /* length should be 0 if unknown.
*/
+ unsigned is_chunked; /* 1 if content length is unknown
(chunked rq) */
+ unsigned no_more_chunks; /* 1 if last chunk has been read */
+ unsigned content_read; /* number of bytes read */
/*
* SSL information
1.7 +4 -2 jakarta-tomcat/src/native/mod_jk/common/jk_util.c
Index: jk_util.c
===================================================================
RCS file: /home/cvs/jakarta-tomcat/src/native/mod_jk/common/jk_util.c,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -r1.6 -r1.7
--- jk_util.c 2001/04/23 10:45:05 1.6
+++ jk_util.c 2001/09/01 01:53:25 1.7
@@ -736,7 +736,9 @@
s->server_name = NULL;
s->server_port = 80;
s->server_software = NULL;
- s->content_length = 0;
+ s->content_length = 0;
+ s->is_chunked = 0;
+ s->no_more_chunks = 0;
s->content_read = 0;
s->is_ssl = JK_FALSE;
s->ssl_cert = NULL;
1.24 +11 -4
jakarta-tomcat/src/share/org/apache/tomcat/modules/server/Ajp13.java
Index: Ajp13.java
===================================================================
RCS file:
/home/cvs/jakarta-tomcat/src/share/org/apache/tomcat/modules/server/Ajp13.ja
va,v
retrieving revision 1.23
retrieving revision 1.24
diff -u -r1.23 -r1.24
--- Ajp13.java 2001/08/29 05:08:07 1.23
+++ Ajp13.java 2001/09/01 01:53:25 1.24
@@ -201,6 +201,8 @@
int blen; // Length of current chunk of body data in buffer
int pos; // Current read position within that buffer
+ boolean end_of_stream; // true if we've received an empty packet
+
public Ajp13()
{
super();
@@ -211,6 +213,7 @@
// This is a touch cargo-cultish, but I think wise.
blen = 0;
pos = 0;
+ end_of_stream = false;
if( dL>0 ) d( "recycle()");
headersWriter.recycle();
}
@@ -384,7 +387,7 @@
MessageBytes clB=headers.getValue("content-length");
int contentLength = (clB==null) ? -1 : clB.getInt();
if( dL > 0 ) d("Content-Length: " + contentLength );
- if(contentLength > 0) {
+ if(contentLength != 0) {
req.setContentLength( contentLength );
/* Read present data */
int err = receive(inBuf);
@@ -455,7 +458,7 @@
return len;
}
- // Not enough data (blen < pos + len)
+ // Not enough data (blen < pos + len) or chunked encoded
int toCopy = len;
while(toCopy > 0) {
int bytesRemaining = blen - pos;
@@ -465,8 +468,8 @@
System.arraycopy(bodyBuff, pos, b, off, c);
if( dL > 0 ) d("doRead2: " + pos + " " + len + " " + blen + " "
+ c +
- " " + new String( b, off, len ) + " " +
- new String( bodyBuff, pos, len ));
+ " " + new String( b, off, c ) + " " +
+ new String( bodyBuff, pos, c ));
toCopy -= c;
@@ -492,6 +495,9 @@
{
// If the server returns an empty packet, assume that that end of
// the stream has been reached (yuck -- fix protocol??).
+ if (end_of_stream) {
+ return false;
+ }
// Why not use outBuf??
inBuf.reset();
@@ -509,6 +515,7 @@
if( inBuf.getLen() == 0 ) {
pos=0;
blen=0;
+ end_of_stream = true;
return false;
}
blen = inBuf.peekInt();