From 9689e87ed54246598b154750aa2afc1735283ba7 Mon Sep 17 00:00:00 2001 From: Wolfgang Draxinger Date: Fri, 19 Jul 2013 14:24:02 +0200 Subject: multipart boundary separation totally broken with runs of s; I must approach this differently. --- picohttp.c | 131 ++++++++++++++++++++----------------------------------- picohttp.h | 10 ++--- test/Makefile | 9 +++- test/bsdsocket.c | 31 +++++++++---- 4 files changed, 84 insertions(+), 97 deletions(-) diff --git a/picohttp.c b/picohttp.c index 9b28b92..7b3dfbc 100644 --- a/picohttp.c +++ b/picohttp.c @@ -453,7 +453,7 @@ static size_t picohttpMatchURL( return j; } -static int8_t picohttpMatchRoute( +static int picohttpMatchRoute( struct picohttpRequest * const req, struct picohttpURLRoute const * const routes ) { @@ -1113,6 +1113,25 @@ int picohttpResponseWrite ( return len; } +static void picohttpMultipartSync( + struct picohttpMultipart * const mp) +{ + if( '\r' == mp->req->query.prev_ch[0] ) { + mp->replayhead = 0; + mp->in_boundary = 1; + debug_printf("mpsync \n"); + } else + if( '\n' == mp->req->query.prev_ch[0] && + '\r' == mp->req->query.prev_ch[1] ) { + mp->replayhead = 1; + mp->in_boundary = 2; + debug_printf("mpsync \n"); + } else { + mp->replayhead = -1; + mp->in_boundary = 0; + } +} + int picohttpMultipartGetch( struct picohttpMultipart * const mp) { @@ -1124,18 +1143,17 @@ int picohttpMultipartGetch( if( mp->replayhead < mp->replay ) { ch = mp->req->query.multipartboundary[mp->replayhead]; mp->replayhead++; - debug_printf("replay head: %0.2x\n", ch); + debug_printf(" >> : %0.2x\n", ch); return ch; } else { - ch = mp->req->query.prev_ch[0]; mp->replay = 0; - mp->replayhead = 0; - debug_printf("replay prev: %0.2x\n", ch); + ch = mp->req->query.prev_ch[0]; + picohttpMultipartSync(mp); + debug_printf(" >>|: %0.2x\n", ch); return ch; } } else { - mp->replay = 0; - +end_of_replay_was_CR: ch = picohttpGetch(mp->req); /* picohttp's query and header parsing is forgiving @@ -1145,40 +1163,17 @@ int picohttpMultipartGetch( * a sequence. */ - switch(ch) { - case '\r': - debug_printf("(CR)", ch); break; - case '\n': - debug_printf("(LF)", ch); break; - default: - debug_printf("(%c)", ch); - } - if( '\r' == ch ) { - mp->replayhead = - mp->in_boundary = 0; - if( '\r' == mp->req->query.prev_ch[1] ) { - debug_printf(""); - return '\r'; - } else { - debug_printf(""); + while( 0 <= ch ) { + + switch(ch) { + case '\r': + debug_printf("(CR)", ch); break; + case '\n': + debug_printf("(LF)", ch); break; + default: + debug_printf("(%c)", ch); } - } else - if( '\n' == ch && - '\r' == mp->req->query.prev_ch[1] ) { - mp->replayhead = - mp->in_boundary = 1; - debug_printf(""); - } else - if( '-' == ch && - '\n' == mp->req->query.prev_ch[1] && - '\r' == mp->req->query.prev_ch[2] - ) { - mp->replayhead = - mp->in_boundary = 2; - debug_printf("-"); - } - while( 0 <= ch ) { if( mp->req->query.multipartboundary[mp->in_boundary] == ch ) { if( 0 == mp->req->query.multipartboundary[mp->in_boundary+1] ) { @@ -1197,9 +1192,10 @@ int picohttpMultipartGetch( mp->finished = 1; } - /* TODO: Technically the last boundary is followed by - * a last sequence... we should check for this - * as well, just for completeness */ + /* TODO: Technically the last boundary is followed by a + * terminating sequence... we should check for + * this as well, just for completeness + */ if(trail[0] == '-' && trail[1] == '-') mp->finished = 2; @@ -1208,55 +1204,22 @@ int picohttpMultipartGetch( debug_printf("{#}", ch); mp->in_boundary++; } else { - debug_printf("{_}", ch); + debug_printf("{_|%0.2x", ch); if( mp->in_boundary ) { - - /* In case the mismatch was due to a or - * or '-' character, it must be checked, if this may be - * preceeded by some * sequence that would - * allow to be a valid multipart boundary. - * In that case the exact replay parameters depend - * on the specific combination. - * - * The replay code above also emits the last character - * read from IO, but in this case that character is - * far ahead of what should actually be returned. - * So we maniulate the content of the prev_ch[0] - * to be the final character of the replay and - * have the replay from the boundary buffer be one - * position short - */ - - /* dw: I really hate how deep this nests, but - * unfortunately HTTP and multipart body parsing - * is a nasty, convoluted state machine - */ + mp->replay = mp->in_boundary; if( '\r' == ch ) { - mp->replay = mp->in_boundary - 1; + mp->replayhead = 0; mp->in_boundary = 1; - mp->req->query.prev_ch[0] = - mp->req->query.multipartboundary[mp->replay]; - } else - if( '\n' == ch && - '\r' == mp->req->query.prev_ch[1] ) { - mp->replay = mp->in_boundary - 2; - mp->in_boundary = 2; - mp->req->query.prev_ch[0] = - mp->req->query.multipartboundary[mp->replay]; - } else - if( '-' == ch && - '\n' == mp->req->query.prev_ch[1] && - '\r' == mp->req->query.prev_ch[2] - ) { - mp->replay = mp->in_boundary - 4; - mp->in_boundary = 2; } else { - mp->replay = mp->in_boundary; + mp->replayhead = 1; mp->in_boundary = 0; } - ch = mp->req->query.multipartboundary[mp->replayhead++]; + + debug_printf("|%d..%d", mp->replayhead, mp->replay); } + debug_putc('}'); + return ch; } ch = picohttpGetch(mp->req); @@ -1368,6 +1331,8 @@ struct picohttpMultipart picohttpMultipartStart( .replayhead = 0 }; + picohttpMultipartSync(&mp); + return mp; } diff --git a/picohttp.h b/picohttp.h index 380584c..d3931ab 100644 --- a/picohttp.h +++ b/picohttp.h @@ -47,10 +47,10 @@ #define PICOHTTP_STATUS_505_HTTP_VERSION_NOT_SUPPORTED 505 struct picohttpIoOps { - int (*read)(size_t /*count*/, char* /*buf*/, void*); - int (*write)(size_t /*count*/, char const* /*buf*/, void*); + int (*read)(size_t /*count*/, void* /*buf*/, void*); + int (*write)(size_t /*count*/, void const* /*buf*/, void*); int (*getch)(void*); // returns negative value on error - int (*putch)(char, void*); + int (*putch)(int, void*); int (*flush)(void*); void *data; }; @@ -120,8 +120,8 @@ struct picohttpRequest { size_t contentlength; uint8_t contentencoding; uint8_t transferencoding; - char multipartboundary[PICOHTTP_MULTIPARTBOUNDARY_MAX_LEN+1]; - char prev_ch[5]; + unsigned char multipartboundary[PICOHTTP_MULTIPARTBOUNDARY_MAX_LEN+1]; + unsigned char prev_ch[5]; size_t chunklength; } query; struct { diff --git a/test/Makefile b/test/Makefile index 2a831d4..e96cb0f 100644 --- a/test/Makefile +++ b/test/Makefile @@ -1,2 +1,9 @@ -bsdsocket: bsdsocket.c ../picohttp.c +.PHONY: all + +all: bsdsocket bsdsocket_nhd + +bsdsocket: bsdsocket.c ../picohttp.c ../picohttp.h $(CC) -std=c99 -DHOST_DEBUG -O0 -g3 -I../ -o bsdsocket ../picohttp.c bsdsocket.c + +bsdsocket_nhd: bsdsocket.c ../picohttp.c ../picohttp.h + $(CC) -std=c99 -O0 -g3 -I../ -o bsdsocket_nhd ../picohttp.c bsdsocket.c diff --git a/test/bsdsocket.c b/test/bsdsocket.c index 1d5f8d5..5350db9 100644 --- a/test/bsdsocket.c +++ b/test/bsdsocket.c @@ -15,14 +15,14 @@ #include "../picohttp.h" -int bsdsock_read(size_t count, char *buf, void *data) +int bsdsock_read(size_t count, void *buf, void *data) { int fd = *((int*)data); ssize_t rb = 0; ssize_t r = 0; do { - r = read(fd, buf+rb, count-rb); + r = read(fd, (unsigned char*)buf + rb, count-rb); if( 0 < r ) { rb += r; continue; @@ -41,14 +41,14 @@ int bsdsock_read(size_t count, char *buf, void *data) return rb; } -int bsdsock_write(size_t count, char const *buf, void *data) +int bsdsock_write(size_t count, void const *buf, void *data) { int fd = *((int*)data); ssize_t wb = 0; ssize_t w = 0; do { - w = write(fd, buf+wb, count-wb); + w = write(fd, (unsigned char*)buf + wb, count-wb); if( 0 < w ) { wb += w; continue; @@ -69,16 +69,17 @@ int bsdsock_write(size_t count, char const *buf, void *data) int bsdsock_getch(void *data) { - char ch; + unsigned char ch; int err; if( 1 != (err = bsdsock_read(1, &ch, data)) ) return err; return ch; } -int bsdsock_putch(char ch, void *data) +int bsdsock_putch(int ch, void *data) { - return bsdsock_write(1, &ch, data); + char ch_ = ch; + return bsdsock_write(1, &ch_, data); } int bsdsock_flush(void* data) @@ -144,11 +145,21 @@ void rhUpload(struct picohttpRequest *req) struct picohttpMultipart mp = picohttpMultipartStart(req); + chdir("/tmp/uploadtest"); while( !picohttpMultipartNext(&mp) ) { fprintf(stderr, "\nprocessing form field \"%s\"\n", mp.disposition.name); - for(int16_t ch = picohttpMultipartGetch(&mp); + FILE *fil = fopen(mp.disposition.name, "wb"); + if(!fil) { + continue; + } + + for(int ch = picohttpMultipartGetch(&mp); 0 <= ch; ch = picohttpMultipartGetch(&mp) ) { + fputc(ch, fil); + + #if HOST_DEBUG + fputs("\e[32m", stderr); switch(ch) { case '\r': fputs("[CR]", stderr); break; @@ -158,7 +169,11 @@ void rhUpload(struct picohttpRequest *req) default: fputc(ch, stderr); } + fputs("\e[0m", stderr); + #endif/*HOST_DEBUG*/ } + + fclose(fil); if( !mp.finished ) { break; } -- cgit v1.2.3