Hi
I gave up trying to figure out the purpose of
the changes made on apache_multipart_buffer.c
and in order to have multipart/form-data messages
parsed correctly I (almost) reverted it back to
the code for apache-1.
IMO the level of maturity of the code in
apache_multipart_buffer.c is not satisfactory
and (as Valery wanted to do) should evolve
(read: rewritten) to exploit libapreq2 parsers.
I can take up the task but I have to put it
off for a while now
Here is the changelog entry for today's patches
(patches are attached to the message)
* INSTALL: instructions for builing Rivet expanded with
examples taken from real builds for Debian and Slackware.
* src/apache-2/apache_request.[c|h]: removed TODO comments
concerning ap_hard_timeout and ap_kill_timeout as these functions
doesn't not exist anymore in the apache 2.x api.
Added the Content-type as a new argumento to
ApacheRequest_parse_multipart for parts boundary determination.
The correctness of the content type has to be done before calling
in order to do the check only once. Commented declarations of variables
not used. The 'tempname' string is allocated now from the apache pool.
Changed template for temporary file name generation.
If apr_file_mktemp fails a more descriptive error is logged.
* src/apache-2/apache_multipart_buffer.[c|h]: removed line in
multipart_buffer_read that lead the buffer pointer to fail
following the parsing process. Input buffer size changed to a
larger and more common size of 8192 bytes
-- Massimo
Index: INSTALL
===================================================================
--- INSTALL (revision 690556)
+++ INSTALL (working copy)
@@ -3,19 +3,75 @@
For more detailed instructions, see the docs/html/ directory.
-Rivet is now based on the auto tools - autoconf, automake and libtool,
+Rivet is now based on the autotools - autoconf, automake and libtool,
and so compilation is simple. For example:
-./configure --with-tcl=/usr/lib/tcl8.4/ --with-apxs=/usr/bin/apxs \
- --with-tclsh=/usr/bin/tclsh8.4
+if you have a clean source tree you have to prepare it for autotools
+
+ aclocal
+ autoreconf
+
+and then run 'configure' with the switches that suit your setup
+
+./configure --with-tcl=/usr/lib/tcl8.4/ \
+ --with-apxs=/usr/bin/apxs \
+ --with-tclsh=/usr/bin/tclsh8.4 \
+ --with-apache-version=1
make
make install
-Basic Apache configuration directives that are needed:
+Along with the usual 'configure' variables Rivet's configure script
+handles other specific environmental variables.
+--with-tclinclude=TCL_INCLUDES_DIR Directory containing include
+ files for compiling code based
+ on the Tcl C library.
+--with-apache-version=VER Values are 1 or 2 depending
+ on the apache server you're
+ builing the module for.
+--with-apache=DIR Apache server's root directory.
+--with-apxs=FILE Path to the apxs program to
+ be used in the compilation
+ process.
+--with-apache-include=DIR Apache's include files path.
+--with-apr-config=FILE Apache Portable Runtime
+ metainformation program
+--with-rivet-target-dir=DIR Rivet Tcl library installation
+ directory.
+
+These are the basic Apache configuration directives that are needed:
+
# Loads the module.
LoadModule rivet_module /path/to/your/copy/of/mod_rivet.so
# Let the module handle .rvt and .tcl files.
-AddType application/x-httpd-rivet .rvt
-AddType application/x-rivet-tcl .tcl
\ No newline at end of file
+AddType application/x-httpd-rivet rvt
+AddType application/x-rivet-tcl tcl
+
+# The default charset can be specified in the configuration
+
+AddType "application/x-httpd-rivet; charset=utf-8" rvt
+
+For Apache 2 a typical configure requesting rivet's library in
+a different location might be
+
+./configure --with-tcl=/usr/lib/tcl8.4 --with-apache-version=2
+ --with-tclsh=/usr/bin/tclsh8.4 --with-apache=/usr
+ --with-rivet-target-dir=/usr/lib/myrivetlib
+
+If Apache1.x and Apache2.x coexist on the same system you must
+tell configure where the right apxs (apache extension tool) script is
+located. E.g.: on a Debian system apxs for Apache2.x is named apxs2
+
+./configure --with-apache-include=/usr/include/apr-1.0 \
+ --with-apache-version=2 --with-apxs=/usr/bin/apxs2 \
+ --with-tcl=/usr/lib/tcl8.4 --with-apache=/usr
+
+Here is another example reported by a user who successfully built
+Rivet on Slackware 12.1
+
+aclocal
+autoreconf
+./configure --with-apache-version=2 --with-tcl=/usr/lib --with-apxs=/usr/bin/apxs
+make
+
Index: src/apache-2/apache_multipart_buffer.c
===================================================================
--- src/apache-2/apache_multipart_buffer.c (revision 690556)
+++ src/apache-2/apache_multipart_buffer.c (working copy)
@@ -40,7 +40,7 @@
while( (ptr = memchr(ptr, needle[0], len)) ) {
/* calculate length after match */
len = haystacklen - (ptr - (char *)haystack);
- ;
+
/* done if matches up to capacity of buffer */
if(memcmp(needle, ptr, needlen) == 0 &&
(partial || len >= needlen))
@@ -64,6 +64,7 @@
/* shift the existing data if necessary */
if(self->bytes_in_buffer > 0 && self->buf_begin != self->buffer)
memmove(self->buffer, self->buf_begin, self->bytes_in_buffer);
+
self->buf_begin = self->buffer;
/* calculate the free space in the buffer */
@@ -79,11 +80,8 @@
/* read the required number of bytes */
if(bytes_to_read > 0) {
char *buf = self->buffer + self->bytes_in_buffer;
- //TODO: fix ap_hard_timeout
- //ap_hard_timeout("[libapreq] multipart_buffer.c:fill_buffer", self->r);
+
actual_read = ap_get_client_block(self->r, buf, bytes_to_read);
- //TODO: fix ap_kill_timeout()
- //ap_kill_timeout(self->r);
/* update the buffer length */
if(actual_read > 0)
@@ -190,7 +188,7 @@
self->bufsize = minsize;
self->request_length = length;
self->boundary = (char*) apr_pstrcat(r->pool, "--", boundary, NULL);
- self->boundary_next = (char*) apr_pstrcat(r->pool, "\n", self->boundary+2, NULL);
+ self->boundary_next = (char*) apr_pstrcat(r->pool, "\n", self->boundary, NULL);
self->buf_begin = self->buffer;
self->bytes_in_buffer = 0;
@@ -201,15 +199,15 @@
//table *multipart_buffer_headers(multipart_buffer *self)
apr_table_t *multipart_buffer_headers(multipart_buffer *self)
{
- //table *tab;
apr_table_t *tab;
char *line;
/* didn't find boundary, abort */
+
if(!find_boundary(self, self->boundary)) return NULL;
/* get lines of text, or CRLF_CRLF */
- //tab = ap_make_table(self->r->pool, 10);
+
tab = apr_table_make(self->r->pool, 10);
while( (line = get_line(self)) && strlen(line) > 0 ) {
/* add header to table */
@@ -218,9 +216,9 @@
if(value) {
*value = 0;
- do {
- value++;
- } while(apr_isspace(*value));
+ do {
+ value++;
+ } while (apr_isspace(*value));
#ifdef DEBUG
ap_log_rerror(MPB_ERROR,
@@ -229,8 +227,7 @@
#endif
apr_table_add(tab, key, value);
- }
- else {
+ } else {
#ifdef DEBUG
ap_log_rerror(MPB_ERROR,
"multipart_buffer_headers: '%s' = ''", key);
@@ -255,8 +252,7 @@
/* look for a potential boundary match, only read data up to that point */
if( (bound = my_memstr(self->buf_begin, self->bytes_in_buffer,
self->boundary_next, 1)) ) {
- max = bound - self->buf_begin - 1 ;
- self->bytes_in_buffer=max+1;
+ max = bound - self->buf_begin;
} else {
max = self->bytes_in_buffer;
}
Index: src/apache-2/apache_multipart_buffer.h
===================================================================
--- src/apache-2/apache_multipart_buffer.h (revision 690556)
+++ src/apache-2/apache_multipart_buffer.h (working copy)
@@ -1,3 +1,20 @@
+/* Copyright 2008 The Apache Software Foundation
+
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+/* $Id: $ */
+
#ifndef _APACHE_MULTIPART_BUFFER_H
#define _APACHE_MULTIPART_BUFFER_H
@@ -4,7 +21,7 @@
#include "apache_request.h"
/* #define DEBUG 1 */
-#define FILLUNIT (1024 * 5)
+#define FILLUNIT (1024 * 8)
#define MPB_ERROR APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, self->r
#ifdef __cplusplus
@@ -32,6 +49,7 @@
///*table*/apr_table_t *multipart_buffer_headers(multipart_buffer *self);
int multipart_buffer_read(multipart_buffer *self, char *buf, int bytes);
char *multipart_buffer_read_body(multipart_buffer *self);
+apr_table_t *multipart_buffer_headers(multipart_buffer *self);
int multipart_buffer_eof(multipart_buffer *self);
#ifdef __cplusplus
Index: src/apache-2/apache_request.c
===================================================================
--- src/apache-2/apache_request.c (revision 690556)
+++ src/apache-2/apache_request.c (working copy)
@@ -56,9 +56,6 @@
*rbuf = apr_pcalloc(r->pool, length + 1);
- //TODO: fix ap_hard_timeout
- //ap_hard_timeout("[libapreq] util_read", r);
-
while ((len_read =
ap_get_client_block(r, buff, sizeof(buff))) > 0) {
if ((rpos + len_read) > length) {
@@ -70,9 +67,6 @@
memcpy((char*)*rbuf + rpos, buff, rsize);
rpos += rsize;
}
-
- //TODO: fix ap_kill_timeout()
- //ap_kill_timeout(r);
}
return rc;
@@ -226,8 +220,7 @@
register char digit;
#ifndef CHARSET_EBCDIC
- digit = ((what[0] >= 'A') ? ((what[0] & 0xdf) - 'A') + 10 : (what[0] - '0'))
-;
+ digit = ((what[0] >= 'A') ? ((what[0] & 0xdf) - 'A') + 10 : (what[0] - '0'));
digit *= 16;
digit += (what[1] >= 'A' ? ((what[1] & 0xdf) - 'A') + 10 : (what[1] - '0'));
#else /*CHARSET_EBCDIC*/
@@ -337,7 +330,7 @@
return OK;
}
-static int urlword_dlm[] = {'&', ';', 0};
+//static int urlword_dlm[] = {'&', ';', 0};
static char *my_urlword(apr_pool_t *p, const char **line)
{
@@ -346,14 +339,14 @@
char ch;
while ( (ch = *pos) != '\0' && ch != ';' && ch != '&') {
- ++pos;
+ ++pos;
}
res = (char*) apr_pstrndup(p, *line, pos - *line);
while (ch == ';' || ch == '&') {
- ++pos;
- ch = *pos;
+ ++pos;
+ ch = *pos;
}
*line = pos;
@@ -376,7 +369,6 @@
ap_unescape_url_u((char*)val);
apr_table_add(req->parms, key, val);
}
-
}
int ApacheRequest___parse(ApacheRequest *req)
@@ -391,15 +383,16 @@
if (r->method_number == M_POST) {
const char *ct = apr_table_get(r->headers_in, "Content-type");
+ if (ct) ap_log_rerror(REQ_INFO, "content-type: `%s'", ct);
if (ct && strncaseEQ(ct, DEFAULT_ENCTYPE, DEFAULT_ENCTYPE_LENGTH)) {
result = ApacheRequest_parse_urlencoded(req);
} else if (ct && strncaseEQ(ct, TEXT_XML_ENCTYPE, TEXT_XML_ENCTYPE_LENGTH)) {
result = ApacheRequest_parse_urlencoded(req);
} else if (ct && strncaseEQ(ct, MULTIPART_ENCTYPE, MULTIPART_ENCTYPE_LENGTH)) {
- result = ApacheRequest_parse_multipart(req);
+ result = ApacheRequest_parse_multipart(req,ct);
} else {
//TODO: fix logging apr_log_rerror
- ap_log_rerror(REQ_ERROR,0, r->server, "unknown content-type: `%s'", ct);
+ ap_log_rerror(REQ_ERROR, "unknown content-type: `%s'", ct);
result = HTTP_INTERNAL_SERVER_ERROR;
}
}
@@ -425,6 +418,7 @@
!strncaseEQ(type, TEXT_XML_ENCTYPE, TEXT_XML_ENCTYPE_LENGTH)) {
return DECLINED;
}
+
if ((rc = util_read(req, &data)) != OK) {
return rc;
}
@@ -437,10 +431,10 @@
return OK;
}
-static void remove_tmpfile(void *data)
+static apr_status_t remove_tmpfile(void *data)
{
ApacheUpload *upload = (ApacheUpload *) data;
- ApacheRequest *req = upload->req;
+// ApacheRequest *req = upload->req;
//TODO: fix ap_pfclose
//if( ap_pfclose(req->r->pool, upload->fp) )
@@ -448,11 +442,14 @@
//apr_log_rerror(REQ_ERROR,"[libapreq] close error on '%s'", upload->tempname);
#ifndef DEBUG
if( remove(upload->tempname) )
+ {
//TODO: fix logging apr_log_rerror
//apr_log_rerror(REQ_ERROR,"[libapreq] remove error on '%s'", upload->tempname);
+ }
#endif
- free(upload->tempname);
+// free(upload->tempname);
+ return 0;
}
apr_file_t *ApacheRequest_tmpfile(ApacheRequest *req, ApacheUpload *upload)
@@ -461,52 +458,49 @@
apr_file_t *fp = NULL;
char *name = NULL;
char *file = NULL ;
- const char *tempdir;
- apr_status_t rv;
+ const char *tempdir;
+ apr_status_t rv;
- tempdir = req->temp_dir;
+ tempdir = req->temp_dir;
/* file = (char *)apr_palloc(r->pool,sizeof(apr_time_t)); */
- file = apr_psprintf(r->pool,"%u", r->request_time);
- rv = apr_temp_dir_get(&tempdir,r->pool);
- if (rv != APR_SUCCESS) {
- ap_log_perror(APLOG_MARK, APLOG_ERR, rv, r->pool, "No temp dir!");
- return NULL;
- }
- rv = apr_filepath_merge(&name,tempdir,file,APR_FILEPATH_NATIVE,r->pool);
- if (rv != APR_SUCCESS) {
- ap_log_perror(APLOG_MARK, APLOG_ERR, rv, r->pool, "File path error!");
- return NULL;
- }
- rv = apr_file_mktemp(&fp,name,0,r->pool);
- if (rv != APR_SUCCESS) {
- ap_log_perror(APLOG_MARK, APLOG_ERR, rv, r->pool, "Failed to open temp file!");
- return NULL;
- }
+ file = apr_psprintf(r->pool,"%u.XXXXXX", (unsigned int)r->request_time);
+ rv = apr_temp_dir_get(&tempdir,r->pool);
+ if (rv != APR_SUCCESS) {
+ ap_log_perror(APLOG_MARK, APLOG_ERR, rv, r->pool, "No temp dir!");
+ return NULL;
+ }
+
+ rv = apr_filepath_merge(&name,tempdir,file,APR_FILEPATH_NATIVE,r->pool);
+ if (rv != APR_SUCCESS) {
+ ap_log_perror(APLOG_MARK, APLOG_ERR, rv, r->pool, "File path error!");
+ return NULL;
+ }
+
+ rv = apr_file_mktemp(&fp,name,0,r->pool);
+ if (rv != APR_SUCCESS) {
+ char* errorBuffer = (char*) apr_palloc(r->pool,256);
+ ap_log_perror(APLOG_MARK, APLOG_ERR, rv, r->pool, "Failed to open temp file: %s",apr_strerror(rv,errorBuffer,256));
+ return NULL;
+ }
+
upload->fp = fp;
upload->tempname = name;
- apr_pool_cleanup_register (r->pool, (void *)upload,
- remove_tmpfile, apr_pool_cleanup_null);
+ apr_pool_cleanup_register (r->pool, (void *)upload, remove_tmpfile, apr_pool_cleanup_null);
return fp;
}
int
-ApacheRequest_parse_multipart(ApacheRequest *req)
+ApacheRequest_parse_multipart(ApacheRequest *req,const char* ct)
{
request_rec *r = req->r;
int rc = OK;
- const char *ct = apr_table_get(r->headers_in, "Content-Type");
long length;
char *boundary;
multipart_buffer *mbuff;
ApacheUpload *upload = NULL;
- apr_status_t status;
- char *error[1024];
- if (!ct) {
- //TODO: fix logging apr_log_rerror
- //apr_log_rerror(REQ_ERROR, "[libapreq] no Content-type header!");
- return HTTP_INTERNAL_SERVER_ERROR;
- }
+ apr_status_t status;
+ char *error[1024];
if ((rc = ap_setup_client_block(r, REQUEST_CHUNKED_ERROR))) {
return rc;
@@ -518,7 +512,7 @@
if ((length = r->remaining) > req->post_max && req->post_max > 0) {
//TODO: fix logging apr_log_rerror
- //apr_log_rerror(REQ_ERROR, "[libapreq] entity too large (%d, max=%d)",
+ //apr_log_rerror(REQ_ERROR, "entity too large (%d, max=%d)",
// (int)length, req->post_max);
return HTTP_REQUEST_ENTITY_TOO_LARGE;
}
@@ -544,21 +538,15 @@
apr_table_t *header = (apr_table_t*) multipart_buffer_headers(mbuff);
const char *cd, *param=NULL, *filename=NULL;
char buff[FILLUNIT];
- int blen, wlen;
+ int blen;
if (!header) {
#ifdef DEBUG
- //TODO: fix logging apr_log_rerror
- //apr_log_rerror(REQ_ERROR,
- // "[libapreq] silently drop remaining '%ld' bytes", r->remaining);
+ apr_log_rerror(REQ_ERROR,"Silently dropping remaining '%ld' bytes", r->remaining);
#endif
- //TODO: fix ap_hard_timeout
- //ap_hard_timeout("[libapreq] parse_multipart", r);
- while ( ap_get_client_block(r, buff, sizeof(buff)) > 0 )
- /* wait for more input to ignore */ ;
- //TODO: fix ap_kill_timeout()
- //ap_kill_timeout(r);
- return OK;
+ do { } while ( ap_get_client_block(r, buff, sizeof(buff)) > 0 );
+
+ return OK;
}
if ((cd = apr_table_get(header, "Content-Disposition"))) {
@@ -621,16 +609,16 @@
}
while ((blen = multipart_buffer_read(mbuff, buff, sizeof(buff)))) {
- status = apr_file_write(upload->fp,buff,&blen);
- if (status != 0) {
- apr_strerror(status,error,1024);
- return HTTP_INTERNAL_SERVER_ERROR;
- }
-
+ apr_size_t bytes_to_write = (apr_size_t) blen;
+ status = apr_file_write(upload->fp,buff,&bytes_to_write);
+ if (status != 0) {
+ apr_strerror(status,error,1024);
+ return HTTP_INTERNAL_SERVER_ERROR;
+ }
upload->size += blen;
}
}
- }
+ }
return OK;
}
Index: src/apache-2/apache_request.h
===================================================================
--- src/apache-2/apache_request.h (revision 690556)
+++ src/apache-2/apache_request.h (working copy)
@@ -95,7 +95,7 @@
ApacheRequest *ApacheRequest_new(request_rec *r);
/* int ApacheRequest_save_post_data(request_rec *r, int flag);
char *ApacheRequest_fetch_post_data(request_rec *r); */
-int ApacheRequest_parse_multipart(ApacheRequest *req);
+int ApacheRequest_parse_multipart(ApacheRequest *req,const char* ct);
int ApacheRequest_parse_urlencoded(ApacheRequest *req);
char *ApacheRequest_script_name(ApacheRequest *req);
char *ApacheRequest_script_path(ApacheRequest *req);
@@ -141,7 +141,8 @@
}
#endif
-#define REQ_ERROR APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, req->r
+#define REQ_ERROR APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, APR_EGENERAL, req->r
+#define REQ_INFO APLOG_MARK, APLOG_INFO, APR_EGENERAL, req->r
#ifdef REQDEBUG
#define REQ_DEBUG(a) (a)
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
