joes 2004/07/09 16:00:39
Modified: . CHANGES
src apreq.c apreq.h apreq_parsers.c apreq_version.h
t parsers.c testall.c
Log:
Add apreq_decodev to fix bug in url parser reported by Ken Burcham.
Revision Changes Path
1.56 +5 -0 httpd-apreq-2/CHANGES
Index: CHANGES
===================================================================
RCS file: /home/cvs/httpd-apreq-2/CHANGES,v
retrieving revision 1.55
retrieving revision 1.56
diff -u -r1.55 -r1.56
--- CHANGES 8 Jul 2004 19:55:56 -0000 1.55
+++ CHANGES 9 Jul 2004 23:00:38 -0000 1.56
@@ -4,6 +4,11 @@
@section v2_04_dev Changes with libapreq2-2.04-dev
+- C API [Ken Burcham, joes]
+ Fix bug in url parser that occurs when a %XX-encoded sequence
+ is split across multiple buckets. Added apreq_decode_decodev
+ to make this problem less inconvenient.
+
- Perl API [joes]
Exception objects inherit from the object which raised it,
which allows $@ to invoke its methods with impunity (exceptions
1.40 +93 -26 httpd-apreq-2/src/apreq.c
Index: apreq.c
===================================================================
RCS file: /home/cvs/httpd-apreq-2/src/apreq.c,v
retrieving revision 1.39
retrieving revision 1.40
diff -u -r1.39 -r1.40
--- apreq.c 29 Jun 2004 18:34:48 -0000 1.39
+++ apreq.c 9 Jul 2004 23:00:38 -0000 1.40
@@ -278,25 +278,13 @@
return (digit);
}
-APREQ_DECLARE(apr_ssize_t) apreq_decode(char *d, const char *s,
- const apr_size_t slen)
+APR_INLINE
+static apr_status_t url_decode(char *d, apr_size_t *dlen,
+ const char *s, apr_size_t *slen)
{
- register int badesc = 0;
+ const char *src = s;
char *start = d;
- const char *end = s + slen;
-
- if (s == NULL || d == NULL)
- return -1;
-
- if (s == (const char *)d) { /* optimize for src = dest case */
- for ( ; s < end; ++s) {
- if (*s == '%' || *s == '+')
- break;
- else if (*s == 0)
- return s - (const char *)d;
- }
- d = (char *)s;
- }
+ const char *end = s + *slen;
for (; s < end; ++d, ++s) {
switch (*s) {
@@ -306,12 +294,12 @@
break;
case '%':
- if (apr_isxdigit(s[1]) && apr_isxdigit(s[2]))
+ if (s + 2 < end && apr_isxdigit(s[1]) && apr_isxdigit(s[2]))
{
*d = x2c(s + 1);
s += 2;
}
- else if ((s[1] == 'u' || s[1] == 'U') &&
+ else if (s + 5 < end && (s[1] == 'u' || s[1] == 'U') &&
apr_isxdigit(s[2]) && apr_isxdigit(s[3]) &&
apr_isxdigit(s[4]) && apr_isxdigit(s[5]))
{
@@ -352,14 +340,23 @@
s += 5;
}
else {
- badesc = 1;
- *d = '%';
+ *dlen = d - start;
+ *slen = s - src;
+ if (s + 5 < end || (s + 2 < end && s[1] != 'u' && s[1] !=
'U')) {
+ *d = 0;
+ return APR_EGENERAL;
+ }
+ memcpy(d, s, end - s);
+ d[end - s] = 0;
+ return APR_INCOMPLETE;
}
break;
case 0:
*d = 0;
- return -1;
+ *dlen = d - start;
+ *slen = s - src;
+ return APR_BADCH;
default:
*d = *s;
@@ -367,11 +364,81 @@
}
*d = 0;
+ *dlen = d - start;
+ *slen = s - src;
+ return APR_SUCCESS;
+}
+
+APREQ_DECLARE(apr_ssize_t) apreq_decode(char *d, const char *s,
+ apr_size_t slen)
+{
+ apr_size_t dlen;
+ const char *end = s + slen;
+
+ if (s == NULL || d == NULL)
+ return -1;
+
+ if (s == (const char *)d) { /* optimize for src = dest case */
+ for ( ; s < end; ++s) {
+ if (*s == '%' || *s == '+')
+ break;
+ else if (*s == 0)
+ return s - (const char *)d;
+ }
+ d = (char *)s;
+ }
- if (badesc)
- return -2;
- else
- return d - start;
+ switch (url_decode(d, &dlen, s, &slen)) {
+ case APR_SUCCESS:
+ return dlen;
+ case APR_INCOMPLETE:
+ return -2;
+ default:
+ return -1;
+ }
+}
+
+
+APREQ_DECLARE(apr_status_t) apreq_decodev(char *d, struct iovec *v, int
nelts,
+ apr_size_t *bytes_written)
+{
+ apr_status_t status = APR_SUCCESS;
+ int n = 0;
+
+ *bytes_written = 0;
+ for (n = 0; n < nelts; ++n) {
+ apr_size_t slen = v[n].iov_len;
+ apr_size_t dlen;
+
+ switch (status = url_decode(d,&dlen,v[n].iov_base, &slen)) {
+
+ case APR_SUCCESS:
+ d += dlen;
+ *bytes_written += dlen;
+ continue;
+
+ case APR_INCOMPLETE:
+ d += dlen;
+ *bytes_written += dlen;
+
+ if (++n == nelts)
+ return APR_INCOMPLETE;
+
+ dlen = v[n-1].iov_len - slen;
+ memcpy(d + dlen, v[n].iov_base, v[n].iov_len);
+ v[n].iov_len += dlen;
+ v[n].iov_base = d;
+
+ status = apreq_decodev(d, v + n, nelts - n, &dlen);
+ *bytes_written += dlen;
+ return status;
+
+ default:
+ *bytes_written = dlen;
+ return status;
+ }
+ }
+ return APR_SUCCESS;
}
1.45 +4 -1 httpd-apreq-2/src/apreq.h
Index: apreq.h
===================================================================
RCS file: /home/cvs/httpd-apreq-2/src/apreq.h,v
retrieving revision 1.44
retrieving revision 1.45
diff -u -r1.44 -r1.45
--- apreq.h 29 Jun 2004 18:34:48 -0000 1.44
+++ apreq.h 9 Jul 2004 23:00:38 -0000 1.45
@@ -239,7 +239,10 @@
* @return Length of url-decoded string in dest, or < 0 on decoding (bad
data) error.
*/
-APREQ_DECLARE(apr_ssize_t) apreq_decode(char *dest, const char *src, const
apr_size_t slen);
+APREQ_DECLARE(apr_ssize_t) apreq_decode(char *dest, const char *src,
apr_size_t slen);
+
+APREQ_DECLARE(apr_status_t) apreq_decodev(char *d, struct iovec *v,
+ int nelts, apr_size_t
*bytes_written);
/**
* Returns an url-encoded copy of a string.
1.56 +44 -52 httpd-apreq-2/src/apreq_parsers.c
Index: apreq_parsers.c
===================================================================
RCS file: /home/cvs/httpd-apreq-2/src/apreq_parsers.c,v
retrieving revision 1.55
retrieving revision 1.56
diff -u -r1.55 -r1.56
--- apreq_parsers.c 7 Jul 2004 23:53:01 -0000 1.55
+++ apreq_parsers.c 9 Jul 2004 23:00:38 -0000 1.56
@@ -114,80 +114,69 @@
{
apr_pool_t *pool = apr_table_elts(t)->pool;
apreq_param_t *param = apr_palloc(pool, nlen + vlen + 1 + sizeof *param);
- apr_size_t total, off;
apreq_value_t *v = ¶m->v;
+ apr_bucket *e, *end;
+ apr_status_t s;
+ struct iovec vec[APREQ_NELTS];
+ apr_array_header_t arr = { pool, sizeof(struct iovec), 0,
+ APREQ_NELTS, (char *)vec };
param->bb = NULL;
param->info = NULL;
v->name = v->data + vlen + 1;
- off = 0;
- total = 0;
- while (total < nlen) {
+ apr_brigade_partition(bb, nlen+1, &end);
+
+ for (e = APR_BRIGADE_FIRST(bb); e != end; e = APR_BUCKET_NEXT(e)) {
apr_size_t dlen;
const char *data;
- apr_bucket *f = APR_BRIGADE_FIRST(bb);
- apr_status_t s = apr_bucket_read(f, &data, &dlen, APR_BLOCK_READ);
- apr_ssize_t decoded_len;
-
- if ( s != APR_SUCCESS )
+ struct iovec *iov;
+ s = apr_bucket_read(e, &data, &dlen, APR_BLOCK_READ);
+ if (s != APR_SUCCESS)
return s;
- total += dlen;
-
- if (total >= nlen) {
- dlen -= total - nlen;
- apr_bucket_split(f, dlen);
- if (data[dlen-1] == '=')
- --dlen;
- }
+ iov = apr_array_push(&arr);
+ iov->iov_base = (char *)data;
+ iov->iov_len = dlen;
+ }
- decoded_len = apreq_decode((char *)v->name + off, data, dlen);
+ ((struct iovec *)arr.elts)[arr.nelts - 1].iov_len--; /* drop '=' sign */
- if (decoded_len < 0) {
- return APR_EGENERAL;
- }
+ s = apreq_decodev((char *)v->name, (struct iovec *)arr.elts,
+ arr.nelts, &v->size);
+ if (s != APR_SUCCESS)
+ return s;
- off += decoded_len;
- apr_bucket_delete(f);
- }
+ while ((e = APR_BRIGADE_FIRST(bb)) != end)
+ apr_bucket_delete(e);
- ((char *)v->name)[off] = 0;
+ arr.nelts = 0;
+ apr_brigade_partition(bb, vlen + 1, &end);
- off = 0;
- total = 0;
- while (total < vlen) {
+ for (e = APR_BRIGADE_FIRST(bb); e != end; e = APR_BUCKET_NEXT(e)) {
apr_size_t dlen;
const char *data;
- apr_bucket *f = APR_BRIGADE_FIRST(bb);
- apr_status_t s = apr_bucket_read(f, &data, &dlen, APR_BLOCK_READ);
- apr_ssize_t decoded_len;
-
- if ( s != APR_SUCCESS )
+ struct iovec *vec;
+ s = apr_bucket_read(e, &data, &dlen, APR_BLOCK_READ);
+ if (s != APR_SUCCESS)
return s;
- total += dlen;
-
- if (total >= vlen) {
- dlen -= total - vlen;
- apr_bucket_split(f, dlen);
- if (data[dlen-1] == '&' || data[dlen-1] == ';')
- --dlen;
- }
+ vec = apr_array_push(&arr);
+ vec->iov_base = (char *)data;
+ vec->iov_len = dlen;
+ }
- decoded_len = apreq_decode(v->data + off, data, dlen);
+ if (end != APR_BRIGADE_SENTINEL(bb))
+ ((struct iovec *)arr.elts)[arr.nelts - 1].iov_len--; /* drop '='
sign */
- if (decoded_len < 0) {
- return APR_EGENERAL;
- }
+ s = apreq_decodev(v->data, (struct iovec *)arr.elts, arr.nelts,
&v->size);
+ if (s != APR_SUCCESS)
+ return s;
- off += decoded_len;
- apr_bucket_delete(f);
- }
+ while ((e = APR_BRIGADE_FIRST(bb)) != end)
+ apr_bucket_delete(e);
- v->data[off] = 0;
- v->size = off;
apr_table_addn(t, v->name, v->data);
return APR_SUCCESS;
}
@@ -236,14 +225,16 @@
if (APR_BUCKET_IS_EOS(e)) {
s = (ctx->status == URL_NAME) ? APR_SUCCESS :
- split_urlword(t, ctx->bb, nlen+1, vlen);
+ split_urlword(t, ctx->bb, nlen, vlen);
APR_BRIGADE_CONCAT(bb, ctx->bb);
ctx->status = (s == APR_SUCCESS) ? URL_COMPLETE : URL_ERROR;
+ apreq_log(APREQ_DEBUG s, env, "url parser saw EOS");
return s;
}
s = apr_bucket_read(e, &data, &dlen, APR_BLOCK_READ);
if ( s != APR_SUCCESS ) {
ctx->status = URL_ERROR;
+ apreq_log(APREQ_ERROR s, env, "apr_bucket_read failed");
return s;
}
parse_url_bucket:
@@ -267,9 +258,10 @@
switch (data[off++]) {
case '&':
case ';':
- s = split_urlword(t, ctx->bb, nlen+1, vlen+1);
+ s = split_urlword(t, ctx->bb, nlen, vlen);
if (s != APR_SUCCESS) {
ctx->status = URL_ERROR;
+ apreq_log(APREQ_ERROR s, env, "split_urlword
failed");
return s;
}
goto parse_url_brigade;
1.20 +1 -1 httpd-apreq-2/src/apreq_version.h
Index: apreq_version.h
===================================================================
RCS file: /home/cvs/httpd-apreq-2/src/apreq_version.h,v
retrieving revision 1.19
retrieving revision 1.20
diff -u -r1.19 -r1.20
--- apreq_version.h 7 Jul 2004 13:49:53 -0000 1.19
+++ apreq_version.h 9 Jul 2004 23:00:38 -0000 1.20
@@ -61,7 +61,7 @@
#define APREQ_MINOR_VERSION 0
/** patch level */
-#define APREQ_PATCH_VERSION 15
+#define APREQ_PATCH_VERSION 16
/**
* This symbol is defined for internal, "development" copies of libapreq.
1.17 +10 -4 httpd-apreq-2/t/parsers.c
Index: parsers.c
===================================================================
RCS file: /home/cvs/httpd-apreq-2/t/parsers.c,v
retrieving revision 1.16
retrieving revision 1.17
diff -u -r1.16 -r1.17
--- parsers.c 27 Jun 2004 18:07:35 -0000 1.16
+++ parsers.c 9 Jul 2004 23:00:39 -0000 1.17
@@ -22,7 +22,7 @@
#define CRLF "\015\012"
-static char url_data[] = "alpha=one&beta=two;omega=last";
+static char url_data[] = "alpha=one&beta=two;omega=last%2";
static char form_data[] =
"--AaB03x" CRLF
"content-disposition: form-data; name=\"field1\"" CRLF
@@ -50,11 +50,17 @@
APR_BRIGADE_INSERT_HEAD(bb,
apr_bucket_immortal_create(url_data,strlen(url_data),
bb->bucket_alloc));
+
+ rv = apreq_parse_request(req,bb);
+ CuAssertIntEquals(tc, APR_INCOMPLETE, rv);
+
+ APR_BRIGADE_INSERT_HEAD(bb,
+ apr_bucket_immortal_create("blast",5,
+ bb->bucket_alloc));
APR_BRIGADE_INSERT_TAIL(bb,
apr_bucket_eos_create(bb->bucket_alloc));
- do rv = apreq_parse_request(req,bb);
- while (rv == APR_INCOMPLETE);
+ rv = apreq_parse_request(req,bb);
CuAssertIntEquals(tc, APR_SUCCESS, rv);
@@ -64,7 +70,7 @@
val = apr_table_get(req->body,"beta");
CuAssertStrEquals(tc, "two", val);
val = apr_table_get(req->body,"omega");
- CuAssertStrEquals(tc, "last", val);
+ CuAssertStrEquals(tc, "last+last", val);
}
1.15 +2 -1 httpd-apreq-2/t/testall.c
Index: testall.c
===================================================================
RCS file: /home/cvs/httpd-apreq-2/t/testall.c,v
retrieving revision 1.14
retrieving revision 1.15
diff -u -r1.14 -r1.15
--- testall.c 2 Jul 2004 02:52:45 -0000 1.14
+++ testall.c 9 Jul 2004 23:00:39 -0000 1.15
@@ -97,8 +97,9 @@
apr_status_t status, void *env, const char *fmt,
va_list vp)
{
+ char buf[256];
if (level < APREQ_LOG_ERR)
- fprintf(stderr, "[%s(%d)] %s\n", file, line,
apr_pvsprintf(p,fmt,vp));
+ fprintf(stderr, "[%s(%d)]%s (%s)\n", file, line,
apr_strerror(status,buf,255), apr_pvsprintf(p,fmt,vp));
}
static apr_status_t test_read(void *env, apr_read_type_e block,
