joes 2004/06/19 13:03:59
Modified: . STATUS
src apreq.h apreq_env.c apreq_params.h apreq_parsers.c
apreq_version.h
t parsers.c
Log:
Fully specify parser's behavior wrt input brigade. Parsers will never destroy
it, and sometimes will not even clean it up completely (see comments on
rejected buckets in apreq_params.h).
Revision Changes Path
1.54 +21 -3 httpd-apreq-2/STATUS
Index: STATUS
===================================================================
RCS file: /home/cvs/httpd-apreq-2/STATUS,v
retrieving revision 1.53
retrieving revision 1.54
diff -u -r1.53 -r1.54
--- STATUS 14 Jun 2004 02:19:58 -0000 1.53
+++ STATUS 19 Jun 2004 20:03:59 -0000 1.54
@@ -60,7 +60,16 @@
TODO:
- - Include perl glue for CGI environment.
+ - CuTest needs va_arg to print comments for a failed unit test.
+
+ - Eliminate useless "status" field from apreq_value_t. The only
behavioral
+ change required is the mfd parser - it should wait until the upload has
+ finished before adding it to the r->body table.
+
+ - Add FAQ file.
+
+ - Fix perl glue for CGI environment on Win32. Add a note to
+ version_check.pl that the minimum mp2 version for CGI support is
1.99_15?
- Get env/ (Apache::Test) tests to work for --with-apache2-src option.
@@ -68,12 +77,20 @@
- Write parser/hook API documentation, and add perl glue for the API.
+ - Implement DISABLE_UPLOADS as an upload hook.
+
+ - Implement $upload->fh using APR::PerlIO.
+
- Add XForms logic to the mfd parser.
- symbol exports files:
-# aix needs .exp files
- - Add basic ssl tests to env/t and glue/perl/t.
+ - Get doxygen to generate manpages for libapreq2, and install both the
+ html documentation and the manpages during "make install".
+
+ - Rework glue/perl build system to use apreq2-config instead of
+ relying on paths like "../../src".
OPEN ISSUES:
@@ -90,6 +107,7 @@
- The current API does not handle failed query_string parsing
adequately (apreq_request does log an error message, but
there's no status code in the struct for users to interrogate).
+
WISH LIST:
1.41 +9 -0 httpd-apreq-2/src/apreq.h
Index: apreq.h
===================================================================
RCS file: /home/cvs/httpd-apreq-2/src/apreq.h,v
retrieving revision 1.40
retrieving revision 1.41
diff -u -r1.40 -r1.41
--- apreq.h 14 Jun 2004 02:19:58 -0000 1.40
+++ apreq.h 19 Jun 2004 20:03:59 -0000 1.41
@@ -44,6 +44,10 @@
* @page INSTALL
* @verbinclude INSTALL
*/
+/**
+ * @page FAQ
+ * @include FAQ
+ */
/**
* @defgroup XS Perl
* @ingroup GLUE
@@ -68,6 +72,11 @@
* @defgroup XS_Request Apache::Request
* @ingroup XS
* @htmlinclude Request.html
+ */
+/**
+ * @defgroup XS_Upload Apache::Upload
+ * @ingroup XS
+ * @htmlinclude Upload.html
*/
/**
* @defgroup XS_Cookie Apache::Cookie
1.7 +10 -4 httpd-apreq-2/src/apreq_env.c
Index: apreq_env.c
===================================================================
RCS file: /home/cvs/httpd-apreq-2/src/apreq_env.c,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -r1.6 -r1.7
--- apreq_env.c 16 Apr 2004 16:37:55 -0000 1.6
+++ apreq_env.c 19 Jun 2004 20:03:59 -0000 1.7
@@ -145,7 +145,7 @@
#define APREQ_MODULE_NAME "CGI"
-#define APREQ_MODULE_MAGIC_NUMBER 20040324
+#define APREQ_MODULE_MAGIC_NUMBER 20040619
static apr_pool_t *cgi_pool(void *env)
{
@@ -268,7 +268,9 @@
if (ctx.bytes_read > ctx.max_body)
return ctx.status = APR_ENOSPC;
}
- return ctx.status = apreq_parse_request(req, bb);
+ ctx.status = apreq_parse_request(req, bb);
+ apr_brigade_cleanup(bb);
+ break;
case APR_INCOMPLETE:
bb = ctx.in;
@@ -281,11 +283,15 @@
if (ctx.bytes_read > ctx.max_body)
return ctx.status = APR_ENOSPC;
}
- return ctx.status = apreq_parse_request(req, bb);
+ ctx.status = apreq_parse_request(req, bb);
+ apr_brigade_cleanup(bb);
+ break;
default:
- return ctx.status = s;
+ ctx.status = s;
}
+
+ return ctx.status;
}
1.30 +18 -7 httpd-apreq-2/src/apreq_params.h
Index: apreq_params.h
===================================================================
RCS file: /home/cvs/httpd-apreq-2/src/apreq_params.h,v
retrieving revision 1.29
retrieving revision 1.30
diff -u -r1.29 -r1.30
--- apreq_params.h 5 Jun 2004 22:59:28 -0000 1.29
+++ apreq_params.h 19 Jun 2004 20:03:59 -0000 1.30
@@ -14,8 +14,8 @@
** limitations under the License.
*/
-#ifndef APREQ_PARAM_H
-#define APREQ_PARAM_H
+#ifndef APREQ_PARAMS_H
+#define APREQ_PARAMS_H
#include "apreq.h"
@@ -29,7 +29,8 @@
* @brief Request and param stuff.
*/
/**
- * @defgroup params Request params
+ * @defgroup APREQ_PARAMS_H Request params
+ * @brief Foo
* @ingroup LIBRARY
* @{
*/
@@ -274,7 +275,12 @@
/**
- * Parse the incoming brigade into a table.
+ * Parse the incoming brigade into a table. Parsers normally
+ * consume all the buckets of the brigade during parsing. However
+ * parsers may leave "rejected" data in the brigade, even during a
+ * successful parse, so callers may need to clean up the brigade
+ * themselves (in particular, rejected buckets should not be
+ * passed back to the parser again).
*/
#define APREQ_RUN_PARSER(psr,env,t,bb) (psr)->parser(psr,env,t,bb)
@@ -301,7 +307,9 @@
/**
- * Rfc822 Header parser.
+ * Rfc822 Header parser. It will reject all data
+ * after the first CRLF CRLF sequence (an empty line).
+ * See #APREQ_RUN_PARSER for more info on rejected data.
*/
APREQ_DECLARE_PARSER(apreq_parse_headers);
@@ -311,7 +319,10 @@
APREQ_DECLARE_PARSER(apreq_parse_urlencoded);
/**
- * Rfc2388 multipart/form-data parser.
+ * Rfc2388 multipart/form-data parser. It will reject
+ * any buckets representing preamble and postamble text
+ * (this is normal behavior, not an error condition).
+ * See #APREQ_RUN_PARSER for more info on rejected data.
*/
APREQ_DECLARE_PARSER(apreq_parse_multipart);
@@ -379,7 +390,7 @@
#endif
-#endif /* APREQ_PARAM_H */
+#endif /* APREQ_PARAMS_H */
1.48 +119 -68 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.47
retrieving revision 1.48
diff -u -r1.47 -r1.48
--- apreq_parsers.c 15 Jun 2004 04:18:27 -0000 1.47
+++ apreq_parsers.c 19 Jun 2004 20:03:59 -0000 1.48
@@ -186,7 +186,9 @@
apr_bucket_brigade *bb;
enum {
URL_NAME,
- URL_VALUE
+ URL_VALUE,
+ URL_COMPLETE,
+ URL_ERROR
} status;
};
@@ -198,22 +200,28 @@
struct url_ctx *ctx;
if (parser->ctx == NULL) {
- parser->ctx = apr_pcalloc(pool, sizeof *ctx);
- ctx = parser->ctx;
- ctx->bb = apr_brigade_create(pool, bb->bucket_alloc);
+ apr_bucket_alloc_t *bb_alloc = apr_bucket_alloc_create(pool);
+ ctx = apr_palloc(pool, sizeof *ctx);
+ ctx->bb = apr_brigade_create(pool, bb_alloc);
+ parser->ctx = ctx;
+ ctx->status = URL_NAME;
}
else
ctx = parser->ctx;
+ if (ctx->status == URL_ERROR)
+ return APR_EGENERAL;
+ else if (ctx->status == URL_COMPLETE)
+ return APR_SUCCESS;
+
APR_BRIGADE_CONCAT(ctx->bb, bb);
- bb = ctx->bb;
parse_url_brigade:
ctx->status = URL_NAME;
- for (e = APR_BRIGADE_FIRST(bb), nlen = vlen = 0;
- e != APR_BRIGADE_SENTINEL(bb);
+ for (e = APR_BRIGADE_FIRST(ctx->bb), nlen = vlen = 0;
+ e != APR_BRIGADE_SENTINEL(ctx->bb);
e = APR_BUCKET_NEXT(e))
{
apr_size_t off = 0, dlen;
@@ -222,7 +230,10 @@
if (APR_BUCKET_IS_EOS(e)) {
s = (ctx->status == URL_NAME) ? APR_SUCCESS :
- split_urlword(t, bb, nlen+1, vlen);
+ split_urlword(t, ctx->bb, nlen+1, vlen);
+
+ APR_BRIGADE_CONCAT(bb, ctx->bb);
+ ctx->status = URL_COMPLETE;
return s;
}
s = apr_bucket_read(e, &data, &dlen, APR_BLOCK_READ);
@@ -250,7 +261,7 @@
switch (data[off++]) {
case '&':
case ';':
- s = split_urlword(t, bb, nlen+1, vlen+1);
+ s = split_urlword(t, ctx->bb, nlen+1, vlen+1);
if (s != APR_SUCCESS)
return s;
goto parse_url_brigade;
@@ -259,6 +270,8 @@
}
}
break;
+ default:
+ ; /* not reached */
}
}
APREQ_BRIGADE_SETASIDE(ctx->bb, pool);
@@ -362,12 +375,15 @@
}
struct hdr_ctx {
+ apr_bucket_brigade *bb;
enum {
HDR_NAME,
HDR_GAP,
HDR_VALUE,
HDR_NEWLINE,
- HDR_CONTINUE
+ HDR_CONTINUE,
+ HDR_COMPLETE,
+ HDR_ERROR
} status;
};
@@ -378,33 +394,47 @@
apr_bucket *e;
struct hdr_ctx *ctx;
- if (parser->ctx == NULL)
- parser->ctx = apr_pcalloc(pool, sizeof *ctx);
+ if (parser->ctx == NULL) {
+ apr_bucket_alloc_t *bb_alloc = apr_bucket_alloc_create(pool);
+ ctx = apr_pcalloc(pool, sizeof *ctx);
+ ctx->bb = apr_brigade_create(pool, bb_alloc);
+ parser->ctx = ctx;
+ }
+ else
+ ctx = parser->ctx;
- ctx = parser->ctx;
+ if (ctx->status == HDR_ERROR)
+ return APR_EGENERAL;
+ else if (ctx->status == HDR_COMPLETE)
+ return APR_SUCCESS;
+ APR_BRIGADE_CONCAT(ctx->bb, bb);
parse_hdr_brigade:
+ ctx->status = HDR_NAME;
+
/* parse the brigade for CRLF_CRLF-terminated header block,
* each time starting from the front of the brigade.
*/
- ctx->status = HDR_NAME;
- for (e = APR_BRIGADE_FIRST(bb), nlen = glen = vlen = 0;
- e != APR_BRIGADE_SENTINEL(bb); e = APR_BUCKET_NEXT(e))
+ for (e = APR_BRIGADE_FIRST(ctx->bb), nlen = glen = vlen = 0;
+ e != APR_BRIGADE_SENTINEL(ctx->bb); e = APR_BUCKET_NEXT(e))
{
apr_size_t off = 0, dlen;
const char *data;
apr_status_t s;
- if (APR_BUCKET_IS_EOS(e))
- return ctx->status = APR_EOF;
-
+ if (APR_BUCKET_IS_EOS(e)) {
+ ctx->status = HDR_COMPLETE;
+ APR_BRIGADE_CONCAT(bb, ctx->bb);
+ return APR_SUCCESS;
+ }
s = apr_bucket_read(e, &data, &dlen, APR_BLOCK_READ);
- if ( s != APR_SUCCESS )
+ if ( s != APR_SUCCESS ) {
+ ctx->status = HDR_ERROR;
return s;
-
+ }
if (dlen == 0)
continue;
@@ -419,7 +449,6 @@
switch (ctx->status) {
-
case HDR_NAME:
while (off < dlen) {
@@ -427,15 +456,15 @@
case '\n':
if (off < dlen)
- apr_bucket_split(e, off);
-
+ apr_bucket_split(e, off);
e = APR_BUCKET_NEXT(e);
do {
- apr_bucket *f = APR_BRIGADE_FIRST(bb);
- apr_bucket_delete(f);
- } while (e != APR_BRIGADE_FIRST(bb));
-
+ apr_bucket *f = APR_BRIGADE_FIRST(ctx->bb);
+ apr_bucket_delete(f);
+ } while (e != APR_BRIGADE_FIRST(ctx->bb));
+ APR_BRIGADE_CONCAT(bb, ctx->bb);
+ ctx->status = HDR_COMPLETE;
return APR_SUCCESS;
case ':':
@@ -504,10 +533,11 @@
/* can parse brigade now */
if (off > 0)
apr_bucket_split(e, off);
- s = split_header(t, bb, nlen, glen, vlen);
- if (s != APR_SUCCESS)
+ s = split_header(t, ctx->bb, nlen, glen, vlen);
+ if (s != APR_SUCCESS) {
+ ctx->status = HDR_ERROR;
return s;
-
+ }
goto parse_hdr_brigade;
}
@@ -536,10 +566,11 @@
}
break;
-
+ default:
+ ; /* not reached */
}
}
-
+ APREQ_BRIGADE_SETASIDE(ctx->bb,pool);
return APR_INCOMPLETE;
}
@@ -601,7 +632,8 @@
MFD_HEADER,
MFD_POST_HEADER,
MFD_PARAM,
- MFD_UPLOAD,
+ MFD_UPLOAD,
+ MFD_COMPLETE,
MFD_ERROR
} status;
apr_bucket *eos;
@@ -625,7 +657,6 @@
if (APR_BUCKET_IS_EOS(e))
return APR_EOF;
-
s = apr_bucket_read(e, &buf, &len, APR_BLOCK_READ);
if (s != APR_SUCCESS)
return s;
@@ -764,24 +795,28 @@
struct mfd_ctx *ctx = parser->ctx;
apr_status_t s;
- if (parser->ctx == NULL) {
+ if (ctx == NULL) {
char *ct;
apr_size_t blen;
apr_bucket_alloc_t *bucket_alloc = apr_bucket_alloc_create(pool);
ctx = apr_pcalloc(pool, sizeof *ctx);
+ parser->ctx = ctx;
+ ctx->status = MFD_INIT;
ct = strchr(parser->enctype, ';');
if (ct == NULL) {
+ ctx->status = MFD_ERROR;
return APR_EINIT;
}
*ct++ = 0;
s = apreq_header_attribute(ct, "boundary", 8,
(const char **)&ctx->bdry, &blen);
- if (s != APR_SUCCESS)
+ if (s != APR_SUCCESS) {
+ ctx->status = MFD_ERROR;
return s;
-
+ }
ctx->bdry[blen] = 0;
*--ctx->bdry = '-';
@@ -796,13 +831,14 @@
ctx->bb = apr_brigade_create(pool, bucket_alloc);
ctx->in = apr_brigade_create(pool, bucket_alloc);
ctx->eos = apr_bucket_eos_create(bucket_alloc);
-
- ctx->status = MFD_INIT;
- parser->ctx = ctx;
+ }
+ else if (ctx->status == MFD_COMPLETE) {
+ apreq_log(APREQ_DEBUG APR_SUCCESS, env, "mfd parser is already
complete- "
+ "all further input brigades will be ignored.");
+ return APR_SUCCESS;
}
APR_BRIGADE_CONCAT(ctx->in, bb);
- bb = ctx->in;
mfd_parse_brigade:
@@ -810,45 +846,61 @@
case MFD_INIT:
{
- s = split_on_bdry(ctx->bb, bb, NULL, ctx->bdry + 2);
+ s = split_on_bdry(ctx->bb, ctx->in, NULL, ctx->bdry + 2);
if (s != APR_SUCCESS) {
- APREQ_BRIGADE_SETASIDE(bb, pool);
+ APREQ_BRIGADE_SETASIDE(ctx->in, pool);
APREQ_BRIGADE_SETASIDE(ctx->bb, pool);
return s;
}
ctx->status = MFD_NEXTLINE;
+ /* Be polite and return any preamble text to the caller. */
+ APR_BRIGADE_CONCAT(bb, ctx->bb);
}
+
/* fall through */
case MFD_NEXTLINE:
{
- apr_status_t s;
- s = split_on_bdry(ctx->bb, bb, NULL, CRLF);
+ s = split_on_bdry(ctx->bb, ctx->in, NULL, CRLF);
if (s != APR_SUCCESS) {
- APREQ_BRIGADE_SETASIDE(bb, pool);
+ APREQ_BRIGADE_SETASIDE(ctx->in, pool);
APREQ_BRIGADE_SETASIDE(ctx->bb, pool);
return s;
}
+ if (!APR_BRIGADE_EMPTY(ctx->bb)) {
+ /* ctx->bb probably contains "--", but we'll stop here
+ * without bothering to check, and just
+ * return any postamble text to caller.
+ */
+ APR_BRIGADE_CONCAT(bb, ctx->in);
+ ctx->status = MFD_COMPLETE;
+ return APR_SUCCESS;
+ }
+
ctx->status = MFD_HEADER;
- apr_brigade_cleanup(ctx->bb);
ctx->info = NULL;
}
/* fall through */
case MFD_HEADER:
{
- apr_status_t s;
-
- if (ctx->info == NULL)
+ if (ctx->info == NULL) {
ctx->info = apr_table_make(pool, APREQ_NELTS);
-
- s = APREQ_RUN_PARSER(ctx->hdr_parser, env, ctx->info, bb);
-
- if (s != APR_SUCCESS) {
- APREQ_BRIGADE_SETASIDE(bb, pool);
- return (s == APR_EOF) ? APR_SUCCESS : s;
+ /* flush out old header parser internal structs for reuse */
+ ctx->hdr_parser->ctx = NULL;
+ }
+ s = APREQ_RUN_PARSER(ctx->hdr_parser, env, ctx->info, ctx->in);
+ switch (s) {
+ case APR_SUCCESS:
+ ctx->status = MFD_POST_HEADER;
+ break;
+ case APR_INCOMPLETE:
+ APREQ_BRIGADE_SETASIDE(ctx->in, pool);
+ return APR_INCOMPLETE;
+ default:
+ ctx->status = MFD_ERROR;
+ return s;
}
- ctx->status = MFD_POST_HEADER;
}
/* fall through */
@@ -872,17 +924,17 @@
apr_size_t nlen, flen;
apr_bucket *e;
- switch (brigade_start_string(bb, ctx->bdry + 2)) {
+ switch (brigade_start_string(ctx->in, ctx->bdry + 2)) {
case APR_INCOMPLETE:
- APREQ_BRIGADE_SETASIDE(bb, pool);
+ APREQ_BRIGADE_SETASIDE(ctx->in, pool);
return APR_INCOMPLETE;
case APR_SUCCESS:
/* part has no body- return CRLF to front */
e = apr_bucket_transient_create(CRLF, 2,
ctx->bb->bucket_alloc);
- APR_BRIGADE_INSERT_HEAD(bb,e);
+ APR_BRIGADE_INSERT_HEAD(ctx->in,e);
break;
default:
@@ -934,14 +986,13 @@
apr_size_t len;
apr_off_t off;
apr_bucket *e;
- apr_status_t s;
- s = split_on_bdry(ctx->bb, bb, ctx->pattern, ctx->bdry);
+ s = split_on_bdry(ctx->bb, ctx->in, ctx->pattern, ctx->bdry);
switch (s) {
case APR_INCOMPLETE:
- APREQ_BRIGADE_SETASIDE(bb, pool);
+ APREQ_BRIGADE_SETASIDE(ctx->in, pool);
APREQ_BRIGADE_SETASIDE(ctx->bb,pool);
return s;
@@ -968,6 +1019,7 @@
v->data[v->size] = 0;
apr_table_addn(t, v->name, v->data);
ctx->status = MFD_NEXTLINE;
+ apr_brigade_cleanup(ctx->bb);
goto mfd_parse_brigade;
default:
@@ -981,12 +1033,11 @@
case MFD_UPLOAD:
{
- apr_status_t s = split_on_bdry(ctx->bb, bb,
- ctx->pattern, ctx->bdry);
apreq_param_t *param;
const apr_array_header_t *arr;
apr_table_entry_t *e;
+ s = split_on_bdry(ctx->bb, ctx->in, ctx->pattern, ctx->bdry);
arr = apr_table_elts(t);
e = (apr_table_entry_t *)arr->elts;
param =
apreq_value_to_param(apreq_strtoval(e[arr->nelts-1].val));
@@ -1000,7 +1051,7 @@
return s;
}
APREQ_BRIGADE_SETASIDE(ctx->bb, pool);
- APREQ_BRIGADE_SETASIDE(bb, pool);
+ APREQ_BRIGADE_SETASIDE(ctx->in, pool);
s = apreq_brigade_concat(env, param->bb, ctx->bb);
return (s == APR_SUCCESS) ? APR_INCOMPLETE : s;
@@ -1032,7 +1083,7 @@
}
break; /* not reached */
- case MFD_ERROR:
+ default:
return APR_EGENERAL;
}
1.13 +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.12
retrieving revision 1.13
diff -u -r1.12 -r1.13
--- apreq_version.h 15 Jun 2004 04:18:27 -0000 1.12
+++ apreq_version.h 19 Jun 2004 20:03:59 -0000 1.13
@@ -60,7 +60,7 @@
#define APREQ_MINOR_VERSION 0
/** patch level */
-#define APREQ_PATCH_VERSION 9
+#define APREQ_PATCH_VERSION 10
/**
* This symbol is defined for internal, "development" copies of libapreq.
1.14 +2 -1 httpd-apreq-2/t/parsers.c
Index: parsers.c
===================================================================
RCS file: /home/cvs/httpd-apreq-2/t/parsers.c,v
retrieving revision 1.13
retrieving revision 1.14
diff -u -r1.13 -r1.14
--- parsers.c 14 Jun 2004 02:19:58 -0000 1.13
+++ parsers.c 19 Jun 2004 20:03:59 -0000 1.14
@@ -83,6 +83,7 @@
CuAssertPtrNotNull(tc, req);
CuAssertStrEquals(tc, req->env, apreq_env_content_type(req->env));
+ /* strlen(form_data) == 317 */
for (j = 0; j <= strlen(form_data); ++j) {
apr_bucket *e = apr_bucket_immortal_create(form_data,
strlen(form_data),
@@ -102,7 +103,7 @@
req->parser = NULL;
rv = apreq_parse_request(req,bb);
- CuAssertIntEquals(tc, APR_INCOMPLETE, rv);
+ CuAssertIntEquals(tc, (j < strlen(form_data)) ? APR_INCOMPLETE :
APR_SUCCESS, rv);
rv = apreq_parse_request(req, tail);
CuAssertIntEquals(tc, APR_SUCCESS, rv);
CuAssertPtrNotNull(tc, req->body);
