drogonframework / drogonframework/drogon

MultiPartParser fails to find the first boundary if the boundary string starts with dashes (e.g., Firefox/Gecko)

Open
#2,497 1 comment 0 reactions 0 assignees View on GitHub
good first issue
Dominant language
C++
Stars
14.3k
Forks
1.4k
Avg merge
1d 12h
Merged PRs (30d)
15

Description

## 🐛 Bug Report: MultiPartParser Fails with Dash-Prefixed Boundaries (Firefox Compatibility)

### 📄 Description
The `MultiPartParser` fails to correctly identify the first part of a `multipart/form-data` request when the boundary string in the `Content-Type` header begins with multiple dashes.

This is a common scenario with Firefox (Gecko-based) browsers, which generate boundaries like:

```
----geckoformboundary...
```

---

### ⚙️ Technical Root Cause
In `lib/src/MultiPart.cc`, the method:

```cpp
parse(const HttpRequestPtr &req)
```

extracts the raw boundary string from the header and searches for it directly in the request body.

However, per **RFC 7578**, the actual delimiter in the body is:

```
"--" + boundary
```

#### Example:
- Boundary: `----ABC`
- Actual delimiter: `------ABC`

Because of this:
- The parser searches for `----ABC`
- But the body contains `------ABC`
- The match is found starting at **offset +2** instead of index `0`

This introduces a **2-byte offset error**.

As a result:
- `parseEntity()` begins parsing from the wrong position
- Expected `\r\n` sequences are missed
- Header parsing (e.g., `Content-Disposition`) fails
- The entire multipart parsing fails

---

### 🔁 Steps to Reproduce

Save the following as `repro.cc` and run it:

```cpp
#include
#include
#include

int main() {
// 1. Define a boundary starting with dashes (common in Firefox)
std::string boundary = "----geckoformboundary7805dba873e5a74dd0aa7640be4f989";

// 2. Construct valid RFC 7578 body (Delimiter = "--" + boundary)
std::string body =
"--" + boundary + "\r\n"
"Content-Disposition: form-data; name=\"file\"; filename=\"test.txt\"\r\n"
"\r\n"
"Hello World\r\n"
"--" + boundary + "--\r\n";

auto req = drogon::HttpRequest::newHttpRequest();
req->setMethod(drogon::Post);
req->addHeader("Content-Type", "multipart/form-data; boundary=" + boundary);
req->setBody(std::move(body));

drogon::MultiPartParser parser;
if (parser.parse(req) == 0 && parser.getFiles().size() > 0) {
std::cout << "SUCCESS: Parsed " << parser.getFiles().size() << " files." << std::endl;
} else {
std::cerr << "FAILURE: MultiPartParser could not parse the request." << std::endl;
return 1;
}
return 0;
}
```

---

###Expected Behavior
- The parser should correctly detect the delimiter
- `parser.parse(req)` should return `0`
- Uploaded files should be successfully parsed

---

### Actual Behavior
- The parser returns `-1`
- Multipart data is not parsed due to internal offset mismatch

---

### 💡 Suggested Fix
Ensure the parser searches for the **full delimiter** (`"--" + boundary`) instead of the raw boundary string.

#### Proposed Change (`MultiPart.cc`):

```cpp
// Within MultiPartParser::parse(const HttpRequestPtr &req)
// ...

std::string fixed_boundary = "--" + boundary;
return parse(req, fixed_boundary.data(), fixed_boundary.size());
```

---

### Additional Notes
- This issue primarily affects clients like Firefox that prepend dashes to boundary values.
- The fix aligns the implementation with RFC 7578 and improves compatibility across browsers.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.