From 94d4a30e8807ab2ee92c829fe59f461c96ce41a4 Mon Sep 17 00:00:00 2001 From: Brad Arant Date: Sat, 22 Aug 2026 21:37:10 -0700 Subject: [PATCH] Fix CGI form/cookie data handling and CGI URL encode/decode - CGIFormData: fix broken setSize()+read() double-length bug that corrupted every parsed body with a leading run of garbage bytes (setSize(contentLength) pre-set length before read()/write() appended on top of it); switch content-type checks from exact equality to startsWith() since a real multipart Content-Type always carries a boundary= parameter; implement the previously empty multipart/form-data branch (ordinary fields only, file uploads deferred to a separate upload mechanism per TODO.txt); restore operator[] real lookup logic (was hardcoded to echo the key back); default-initialize formdata to NULL; fix mismatched include guard (leftover from an IMFFormData.h copy-paste); drop the unused IMFHeader.h include that pulled in a broken, unrelated StreamReader.h dependency. - CGIFormattedData: parameterize the split delimiter (was hardcoded to &, which silently broke cookie parsing since HTTP_COOKIE uses ; not &) and trim whitespace on each split token. - MString::toCGI()/fromCGI(): were unimplemented stubs (always returned an empty string); implemented proper CGI percent-encode / percent-decode + space plus-encoding, and fixed the one caller that discarded fromCGIs return value instead of using it. Co-Authored-By: Claude Sonnet 5 --- CGIFormData.cpp | 56 ++++++++++++++++++++++++++++++++++---------- CGIFormData.h | 4 ++-- CGIFormattedData.cpp | 38 +++++++++++++++++------------- CGIFormattedData.h | 9 ++++++- MString.cpp | 48 +++++++++++++++++++++++++++++++++++-- 5 files changed, 121 insertions(+), 34 deletions(-) diff --git a/CGIFormData.cpp b/CGIFormData.cpp index 589c378..b199b86 100644 --- a/CGIFormData.cpp +++ b/CGIFormData.cpp @@ -1,28 +1,58 @@ #include "CGIFormData.h" -#include "IMFHeader.h" #include "malloc.h" namespace coreutils { CGIFormData::CGIFormData(int fd, ZString contentType, int contentLength, ZString boundary) - : contentType(contentType) { - if(contentType == "application/x-www-form-urlencoded") { - setSize(contentLength); + : contentType(contentType) { + if(contentType.startsWith("application/x-www-form-urlencoded")) { read(fd, contentLength); formdata = new CGIFormattedData(*this); - } else if(contentType == "multipart/form-data") { - + } else if(contentType.startsWith("multipart/form-data")) { + read(fd, contentLength); + formdata = new CGIFormattedData(); + MString delimiter; + delimiter << "--" << boundary; + ZString body(*this); + body.split(delimiter); + + for(int ix = 0; ix < body.getList().size(); ++ix) { + ZString part = body[ix]; + if(part.getLength() == 0 || part.startsWith("--")) + continue; + + int headerEnd = part.find("\r\n\r\n"); + if(headerEnd == -1) + continue; + + ZString headers = part.substring(0, headerEnd); + if(headers.find("filename=\"") != -1) + // file uploads are out of scope here; handled via a separate upload mechanism. + continue; + + int namePos = headers.find("name=\""); + if(namePos == -1) + continue; + + ZString afterName = headers.substring(namePos + 6); + int nameEnd = afterName.find("\""); + if(nameEnd == -1) + continue; + ZString name = afterName.substring(0, nameEnd); + + ZString value = part.substring(headerEnd + 4); + value.trimCRLF(); + + formdata->data[name] = value; + } } - - } MString CGIFormData::operator[](ZString name) { -// if(contentType == "application/x-www-form-urlencoded") -// return formdata[name]; - MString temp(name); - return temp; + if(formdata == NULL) + return MString(); + return (*formdata).data[name]; } - + } diff --git a/CGIFormData.h b/CGIFormData.h index 945817e..860799e 100644 --- a/CGIFormData.h +++ b/CGIFormData.h @@ -1,5 +1,5 @@ #ifndef __CGIFormData_h__ -# define __IMFFormData_h__ +# define __CGIFormData_h__ #include "MString.h" #include "ZString.h" @@ -15,7 +15,7 @@ namespace coreutils { MString operator[](ZString name); private: - CGIFormattedData *formdata; + CGIFormattedData *formdata = NULL; MString contentType; }; diff --git a/CGIFormattedData.cpp b/CGIFormattedData.cpp index 98c4342..f91f94c 100644 --- a/CGIFormattedData.cpp +++ b/CGIFormattedData.cpp @@ -4,27 +4,33 @@ namespace coreutils { CGIFormattedData::CGIFormattedData() {} - + CGIFormattedData::CGIFormattedData(ZString cgiData) { - if(cgiData != "") { - cgiData.split("&"); + parse(cgiData); + } + + CGIFormattedData::CGIFormattedData(ZString cgiData, ZString delimiter) : delimiter(delimiter) { + parse(cgiData); + } + + void CGIFormattedData::setDelimiter(ZString delimiter) { + this->delimiter = delimiter; + } + + void CGIFormattedData::parse(ZString cgiData) { + if(cgiData != "") { + cgiData.split(delimiter); for(int ix = 0; ix < cgiData.getList().size(); ++ix) { - cgiData[ix].split("="); - data[cgiData[ix][0]] = cgiData[ix][1]; - data[cgiData[ix][0]].fromCGI(); + ZString pair = cgiData[ix].trim(); + pair.split("="); + data[pair[0]] = pair[1]; + data[pair[0]] = data[pair[0]].fromCGI(); } - } + } } ZString CGIFormattedData::operator=(ZString cgiData) { - if(cgiData != "") { - cgiData.split("&"); - for(int ix = 0; ix < cgiData.getList().size(); ++ix) { - cgiData[ix].split("="); - data[cgiData[ix][0]] = cgiData[ix][1]; - data[cgiData[ix][0]].fromCGI(); - } - } + parse(cgiData); return cgiData; - } + } } diff --git a/CGIFormattedData.h b/CGIFormattedData.h index e6a3fc8..974f534 100644 --- a/CGIFormattedData.h +++ b/CGIFormattedData.h @@ -17,11 +17,18 @@ namespace coreutils { public: CGIFormattedData(); CGIFormattedData(ZString cgiData); + CGIFormattedData(ZString cgiData, ZString delimiter); ZString operator=(ZString cgiData); + void setDelimiter(ZString delimiter); std::map data; - + + private: + void parse(ZString cgiData); + + ZString delimiter = "&"; + }; } diff --git a/MString.cpp b/MString.cpp index 57bb207..360f91f 100644 --- a/MString.cpp +++ b/MString.cpp @@ -432,13 +432,57 @@ namespace coreutils { MString MString::toCGI() { + push(); + reset(); MString target; + while(!eod()) { + char ch = nextChar(); + if(ch == ' ') { + target.write('+'); + } else if(isalnum((unsigned char)ch) || ch == '-' || ch == '_' || ch == '.' || ch == '~') { + target.write(ch); + } else { + target.write('%'); + target.write(hexChar((ch >> 4) & 15)); + target.write(hexChar(ch & 15)); + } + } + pop(); return target; } - - + + MString MString::fromCGI() { + push(); + reset(); MString target; + while(!eod()) { + char ch = nextChar(); + if(ch == '+') { + target.write(' '); + } else if(ch == '%' && !eod()) { + char ch1 = nextChar(); + ch1 -= 48; + if(ch1 > 9) + ch1 -= 7; + ch1 <<= 4; + ch1 &= 240; + if(!eod()) { + char ch2 = nextChar(); + ch2 -= 48; + if(ch2 > 9) + ch2 -= 7; + ch2 &= 15; + ch1 |= ch2; + target.write(ch1); + } else { + target.write(ch1); + } + } else { + target.write(ch); + } + } + pop(); return target; }