actix / actix/actix-web

Filename escaping in multipart/form-data Content-Disposition

Open
#2,741 5 comments 0 reactions 0 assignees View on GitHub
A-web needs-investigation
Dominant language
Rust
Stars
24.8k
Forks
1.9k
Avg merge
23h 10m
Merged PRs (30d)
26

Description

(Un)escaping of the filename in the Content-Disposition "header" of multipart/form-data streams does not match the other implementations I tested.

## Expected Behavior
In particular, the filename `\"` is escaped as `\%22` in curl, firefox and chrome.
There is an existing discussion of this at https://github.com/curl/curl/issues/7789.
actix-web should decode this as `\"`.

## Current Behavior
actix-web currently implements RFC 6266 for filename encoding and decodes this as `%22`, dropping the `\` and not unescaping the `"`.

## Possible Solution
Use RFC 7578 for quoted strings in the DispositionParam (un)escaping code.
I am not sure if this conflicts with Content-Disposition escaping for normal HTTP-Response headers.
Also, as the comment in `test_from_raw_unnecessary_percent_decode` states, other implementations do not percent-escape higher UTF-8 codepoints, but they do percent-escape `"` (and as far as i can see, only `"` and no other bytes).

## Steps to Reproduce (for bugs)

```
ncat -l 8000 &
touch "\\\"¢"
curl -v -F "file=@\\\"¢" localhost:8000
^C
```
which produces a body similar to this (not escaping the `\` or `¢` and percent escaping the `"`):
```
--------------------------a80b394c70a9c219
Content-Disposition: form-data; name="file"; filename="\%22¢"
Content-Type: application/octet-stream

--------------------------a80b394c70a9c219--
```

Adapted from `http::header::content_disposition::test_from_raw_unnecessary_percent_decode`
```
let a = HeaderValue::from_static(
"form-data; name=\"file\"; filename=\"\\%22\"¢",
);
let a: ContentDisposition = ContentDisposition::from_raw(&a).unwrap();
let b = ContentDisposition {
disposition: DispositionType::FormData,
parameters: vec![
DispositionParam::Name("file".to_owned()),
DispositionParam::Filename(String::from("\\\"¢")),
],
};
assert_eq!(a, b);
```

## Context

I am building a temporary file-uploading service, and tried uploading a file which has a `"` in its name, which works but writes a wrong name to the database.

## Your Environment

- Rust Version: rustc 1.59.0 (9d1b2106e 2022-02-23)
- Actix Web Version: 4.0.1
- curl Version: 7.82.0
- Firefox Version: Mozilla Firefox 99.0.1

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.