Skip to content

Fix semicolon parsing in ContentDisposition - #205

Open
kukushechkin wants to merge 2 commits into
apple:mainfrom
kukushechkin:correctly-parse-content-disposition
Open

Fix semicolon parsing in ContentDisposition#205
kukushechkin wants to merge 2 commits into
apple:mainfrom
kukushechkin:correctly-parse-content-disposition

Conversation

@kukushechkin

Copy link
Copy Markdown
Contributor

Motivation

Unconditional split at ; splits filename value into multiple (usually broken) params.

Modifications

Correctly track quotes in ContentDisposition param and inly split params at high-level semicolon.

Result

Filenames with semicolons are parsed correctly.

Test Plan

Unit tests.

@kukushechkin kukushechkin added the 🔨 semver/patch No public API change. label Jul 29, 2026
@czechboy0 czechboy0 self-assigned this Aug 7, 2026

/// Wraps a parameter value in quotes, escaping backslashes and double quotes.
private static func quote(_ value: String) -> String {
"\"" + value.replacingOccurrences(of: "\\", with: "\\\\").replacingOccurrences(of: "\"", with: "\\\"") + "\""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"\"" + value.replacingOccurrences(of: "\\", with: "\\\\").replacingOccurrences(of: "\"", with: "\\\"") + "\""
#"""# + value.replacingOccurrences(of: #"\"#, with: #"\\"#).replacingOccurrences(of: #"""#, with: #"\""#) + #"""#

var iterator = rawValue.makeIterator()
while let character = iterator.next() {
switch character {
case "\\" where isInsideQuotedString:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
case "\\" where isInsideQuotedString:
case #"\"# where isInsideQuotedString:

case "\\" where isInsideQuotedString:
current.append(character)
if let escaped = iterator.next() { current.append(escaped) }
case "\"":

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
case "\"":
case #"""#:


/// Removes surrounding quotes and resolves backslash-escaped characters in a parameter value.
private static func unquote(_ value: String) -> String {
guard value.count >= 2, value.first == "\"", value.last == "\"" else { return value }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
guard value.count >= 2, value.first == "\"", value.last == "\"" else { return value }
guard value.count >= 2, value.first == #"""#, value.last == #"""# else { return value }

Comment on lines +132 to +133
return value.dropFirst().dropLast().replacingOccurrences(of: "\\\"", with: "\"")
.replacingOccurrences(of: "\\\\", with: "\\")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return value.dropFirst().dropLast().replacingOccurrences(of: "\\\"", with: "\"")
.replacingOccurrences(of: "\\\\", with: "\\")
return value.dropFirst().dropLast().replacingOccurrences(of: #"\""#, with: #"""#)
.replacingOccurrences(of: #"\\"#, with: #"\"#)

Comment on lines +113 to +115
case "\\" where isInsideQuotedString:
current.append(character)
if let escaped = iterator.next() { current.append(escaped) }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you help me understand this section better? I don't quite follow - we don't transform the characters, we just add them back as they were.

XCTAssertEqual(stateMachine.next(parts[0]), .emitError(.receivedMultipleValuesForSingleValuePart("name")))
}

func testFilenameContainingQuoteAndSemicolon() throws {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this test on top of what you added to Test_ContentDisposition.swift?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔨 semver/patch No public API change.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants