wrowe 02/03/17 10:40:46
Modified: buckets apr_brigade.c
include apr_buckets.h
Log:
Resolve type saftey that the author couldn't resolve... the emits could
not be resolved because len describes a memory buffer, and memory buffer
sizes are expressed in apr_size_t.
This commit identifies a host of potential bugs in the flatten() APIs.
Extensive review of the use cases is required to clean up the API.
Revision Changes Path
1.36 +27 -7 apr-util/buckets/apr_brigade.c
Index: apr_brigade.c
===================================================================
RCS file: /home/cvs/apr-util/buckets/apr_brigade.c,v
retrieving revision 1.35
retrieving revision 1.36
diff -u -r1.35 -r1.36
--- apr_brigade.c 13 Mar 2002 20:40:46 -0000 1.35
+++ apr_brigade.c 17 Mar 2002 18:40:45 -0000 1.36
@@ -225,9 +225,9 @@
}
APU_DECLARE(apr_status_t) apr_brigade_flatten(apr_bucket_brigade *bb,
- char *c, apr_off_t *len)
+ char *c, apr_size_t *len)
{
- apr_off_t actual = 0;
+ apr_size_t actual = 0;
apr_bucket *b;
APR_BRIGADE_FOREACH(b, bb) {
@@ -241,16 +241,22 @@
}
/* If we would overflow. */
- if (*len < actual + str_len) {
+ if (str_len + actual > *len) {
str_len = *len - actual;
}
+ /* XXX: It appears that overflow of the final bucket
+ * is DISCARDED without any warning to the caller.
+ */
memcpy(c, str, str_len);
c += str_len;
actual += str_len;
- if (*len < actual) {
+ /* XXX: Is this a bug in actual == *len or did we intend to
+ * flatten all trailing 0 byte buckets?
+ */
+ if (actual > *len) {
break;
}
}
@@ -261,14 +267,28 @@
APU_DECLARE(apr_status_t) apr_brigade_pflatten(apr_bucket_brigade *bb,
char **c,
- apr_off_t *len,
+ apr_size_t *len,
apr_pool_t *pool)
{
- apr_off_t total;
+ apr_off_t actual;
+ apr_size_t total;
apr_status_t rv;
- apr_brigade_length(bb, 1, &total);
+ apr_brigade_length(bb, 1, &actual);
+ /* XXX: This is dangerous beyond belief. At least in the
+ * apr_brigade_flatten case, the user explicitly stated their
+ * buffer length - so we don't up and palloc 4GB for a single
+ * file bucket. This API must grow a useful max boundry,
+ * either compiled-in or preset via the *len value.
+ *
+ * Shouldn't both fn's grow an additional return value for
+ * the case that the brigade couldn't be flattened into the
+ * provided or allocated buffer (such as APR_EMOREDATA?)
+ * Not a failure, simply an advisory result.
+ */
+ total = (apr_size_t)actual;
+
*c = apr_palloc(pool, total);
rv = apr_brigade_flatten(bb, *c, &total);
1.132 +2 -2 apr-util/include/apr_buckets.h
Index: apr_buckets.h
===================================================================
RCS file: /home/cvs/apr-util/include/apr_buckets.h,v
retrieving revision 1.131
retrieving revision 1.132
diff -u -r1.131 -r1.132
--- apr_buckets.h 13 Mar 2002 20:40:48 -0000 1.131
+++ apr_buckets.h 17 Mar 2002 18:40:45 -0000 1.132
@@ -688,7 +688,7 @@
*/
APU_DECLARE(apr_status_t) apr_brigade_flatten(apr_bucket_brigade *bb,
char *c,
- apr_off_t *len);
+ apr_size_t *len);
/**
* Creates a pool-allocated string representing a flat bucket brigade
@@ -699,7 +699,7 @@
*/
APU_DECLARE(apr_status_t) apr_brigade_pflatten(apr_bucket_brigade *bb,
char **c,
- apr_off_t *len,
+ apr_size_t *len,
apr_pool_t *pool);
/**
