refactor: improve UploadHandler boundary extraction and add utility for quoted values

webserv/handler/UploadHandler.cpp

@@ -44,17 +44,16 @@ void UploadHandler::handle()
         return;
     }
 
-    // TODO: So wtf does this do for non-multipart uploads?
+    // TODO: Tester expects 200 OK for non-multipart uploads - weird but sure, okay
     if (contentType->find("multipart/form-data") == std::string::npos)
     {
-        // For application/x-www-form-urlencoded or other types, just return success
-        // The upload endpoint can accept form data without files
         Log::debug("Upload request with non-multipart Content-Type: " + *contentType);
         response_.setStatus(200);
         response_.addHeader("Content-Type", "application/json");
         response_.setBody("{\"success\": true, \"message\": \"Form data received\"}\n");
         return;
     }
+
     if (!FileUtils::isDirectory(uploadStore_))
     {
         ErrorHandler::createErrorResponse(Http::StatusCode::FORBIDDEN,
@@ -82,37 +81,6 @@ void UploadHandler::handleTimeout()
     ErrorHandler::createErrorResponse(Http::StatusCode::GATEWAY_TIMEOUT, response_);
 }
 
-std::string UploadHandler::extractBoundary(const std::string &contentType) const
-{
-    Log::trace(LOCATION);
-
-    size_t boundaryPos = contentType.find("boundary=");
-    if (boundaryPos == std::string::npos)
-    {
-        throw std::runtime_error("No boundary found in Content-Type");
-    }
-    std::string boundary = contentType.substr(boundaryPos + std::strlen("boundary=")); // "boundary=" is 9 chars
-    // Remove quotes if present
-    if (!boundary.empty() && boundary[0] == '"')
-    {
-        boundary = boundary.substr(1);
-        size_t endQuote = boundary.find('"');
-        if (endQuote == std::string::npos)
-        {
-            throw std::runtime_error("Malformed boundary in Content-Type");
-        }
-        boundary = boundary.substr(0, endQuote);
-    }
-    // Remove any trailing characters after semicolon or whitespace
-    size_t endPos = boundary.find_first_of("; \t\r\n");
-    if (endPos != std::string::npos)
-    {
-        boundary = boundary.substr(0, endPos);
-    }
-    Log::debug("Extracted boundary: " + boundary);
-    return boundary;
-}
-
 void UploadHandler::parseMultipart()
 {
     Log::trace(LOCATION);
@@ -181,6 +149,27 @@ void UploadHandler::parseMultipart()
     Log::info("Parsed " + std::to_string(uploadedFiles_.size()) + " file(s) from multipart form data");
 }
 
+std::string UploadHandler::extractBoundary(const std::string &contentType)
+{
+    Log::trace(LOCATION);
+
+    size_t boundaryPos = contentType.find("boundary=");
+    if (boundaryPos == std::string::npos)
+    {
+        throw std::runtime_error("No boundary found in Content-Type");
+    }
+    std::string boundary = contentType.substr(boundaryPos + std::strlen("boundary=")); // "boundary=" is 9 chars
+    boundary = utils::extractQuotedValue(boundary);
+    if (boundary.empty())
+    {
+        throw std::runtime_error("Malformed boundary in Content-Type");
+    }
+    // Remove any trailing characters after semicolon or whitespace
+    boundary = utils::trim(boundary, "\t\r\n ");
+    Log::debug("Extracted boundary: " + boundary);
+    return boundary;
+}
+
 bool UploadHandler::decodeSection(const std::string &part)
 {
     Log::trace(LOCATION);
@@ -280,7 +269,7 @@ std::string UploadHandler::getHeaderValue(const std::string &headers, const std:
     return "";
 }
 
-std::string UploadHandler::getFileName(const std::string &disposition) const
+std::string UploadHandler::getFileName(const std::string &disposition)
 {
     // Look for filename="..." or filename*=UTF-8''...
     size_t filenamePos = disposition.find("filename=");
@@ -290,30 +279,17 @@ std::string UploadHandler::getFileName(const std::string &disposition) const
     }
     // TODO: strlen is extra function call, but magic number otherwise
     std::string filename = disposition.substr(filenamePos + std::strlen("filename="));
-    // TODO: DRY, this is similar to boundary extraction
+    filename = utils::extractQuotedValue(filename);
-    if (!filename.empty() && filename[0] == '"')
+    if (filename.empty())
     {
-        filename = filename.substr(1);
+        Log::warning("Malformed filename in Content-Disposition");
-        size_t endQuote = filename.find('"');
+        return "";
-        if (endQuote != std::string::npos)
-        {
-            filename = filename.substr(0, endQuote);
-        }
-        else
-        {
-            // Malformed filename
-            Log::warning("Malformed filename in Content-Disposition");
-            return "";
-        }
     }
-    else
+    // Unquoted - take until semicolon or end
+    size_t endPos = filename.find(';');
+    if (endPos != std::string::npos)
     {
-        // Unquoted - take until semicolon or end
+        filename = filename.substr(0, endPos);
-        size_t endPos = filename.find(';');
-        if (endPos != std::string::npos)
-        {
-            filename = filename.substr(0, endPos);
-        }
     }
 
     return utils::trim(filename);

webserv/handler/UploadHandler.hpp

@@ -44,14 +44,12 @@ class UploadHandler : public AHandler
     bool save(UploadedFile &info, const std::vector<uint8_t> &data);
     std::string sanitizeFilename(const std::string &filename) const;
     std::string generateFilename(const std::string &baseFilename) const;
-    // void sendSuccessResponse();
-    // void sendErrorResponse(uint16_t statusCode, const std::string &message);
 
     // Multipart parsing helpers
-    std::string extractBoundary(const std::string &contentType) const;
+    [[nodiscard]] static std::string extractBoundary(const std::string &contentType);
     bool decodeSection(const std::string &part);
-    std::string getHeaderValue(const std::string &headers, const std::string &headerName) const;
+    [[nodiscard]] std::string getHeaderValue(const std::string &headers, const std::string &key) const;
-    std::string getFileName(const std::string &disposition) const;
+    [[nodiscard]] static std::string getFileName(const std::string &disposition);
     std::string getFieldName(const std::string &disposition) const;
 
     static const std::string DEFAULT_UPLOAD_STORE;

webserv/utils/utils.cpp

@@ -51,6 +51,21 @@ std::string trimSemi(const std::string &str)
     return str;
 }
 
+std::string extractQuotedValue(const std::string &str)
+{
+    size_t first = str.find('"');
+    if (first == std::string::npos)
+    {
+        return str;
+    }
+    size_t second = str.find('"', first + 1);
+    if (second == std::string::npos)
+    {
+        return ""; // No closing quote found, return empty string
+    }
+    return str.substr(first + 1, second - first - 1);
+}
+
 size_t findCorrespondingClosingBrace(const std::string &str, size_t openPos)
 {
     int braceCount = 1;

webserv/utils/utils.hpp

@@ -11,8 +11,8 @@ namespace utils
 {
 size_t stoul(const std::string &str, int base = 10);
 std::string trimSemi(const std::string &str);
-
 std::string trim(const std::string &str, const std::string &charset = " \t\n\r");
+std::string extractQuotedValue(const std::string &str);
 size_t findCorrespondingClosingBrace(const std::string &str, size_t openPos);
 void removeEmptyLines(std::string &str);
 void removeComments(std::string &str);