Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update URLEncodedFormEncoder encoding rules #3192

Merged
merged 6 commits into from
May 20, 2024
Merged

Update URLEncodedFormEncoder encoding rules #3192

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9c677c2
Percent-encode `$` in URLEncodedFormEncoder
ptoffy May 13, 2024
1716132
Adopt URL RFC based rule for encoding
ptoffy May 13, 2024
bba1b5f
Remove unused import
ptoffy May 13, 2024
6319404
Update failing test with stricter URL encoding
ptoffy May 14, 2024
cfa4496
Merge branch 'main' into urlencoder-issue
ptoffy May 14, 2024
0ee3bbb
Merge branch 'main' into urlencoder-issue
0xTim May 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions Sources/Vapor/URLEncodedForm/URLEncodedFormSerializer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ import struct Foundation.CharacterSet
struct URLEncodedFormSerializer: Sendable {
let splitVariablesOn: Character
let splitKeyValueOn: Character

/// Create a new form-urlencoded data parser.
init(splitVariablesOn: Character = "&", splitKeyValueOn: Character = "=") {
self.splitVariablesOn = splitVariablesOn
self.splitKeyValueOn = splitKeyValueOn
}

func serialize(_ data: URLEncodedFormData, codingPath: [CodingKey] = []) throws -> String {
var entries: [String] = []
let key = try codingPath.toURLEncodedKey()
Expand All @@ -21,20 +21,20 @@ struct URLEncodedFormSerializer: Sendable {
}
}
for (key, child) in data.children {
entries.append(try serialize(child, codingPath: codingPath + [_CodingKey(stringValue: key) as CodingKey]))
try entries.append(serialize(child, codingPath: codingPath + [_CodingKey(stringValue: key) as CodingKey]))
}
return entries.joined(separator: String(splitVariablesOn))
}

struct _CodingKey: CodingKey {
var stringValue: String

init(stringValue: String) {
self.stringValue = stringValue
}

var intValue: Int?

init?(intValue: Int) {
self.intValue = intValue
self.stringValue = intValue.description
Expand All @@ -47,9 +47,9 @@ extension Array where Element == CodingKey {
if count < 1 {
return ""
}
return try self[0].stringValue.urlEncoded(codingPath: self) + self[1...].map({ (key: CodingKey) -> String in
return try "[" + key.stringValue.urlEncoded(codingPath: self) + "]"
}).joined()
return try self[0].stringValue.urlEncoded(codingPath: self) + self[1...].map { (key: CodingKey) -> String in
try "[" + key.stringValue.urlEncoded(codingPath: self) + "]"
}.joined()
}
}

Expand All @@ -72,10 +72,10 @@ extension String {

/// Characters allowed in form-urlencoded data.
private enum Characters {
// https://url.spec.whatwg.org/#application-x-www-form-urlencoded-percent-encode-set
static let allowedCharacters: CharacterSet = {
var allowed = CharacterSet.urlQueryAllowed
// these symbols are reserved for url-encoded form
allowed.remove(charactersIn: "?&=[];+")
var allowed = CharacterSet.alphanumerics
allowed.insert(charactersIn: "*-._")
Comment on lines +77 to +78
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm honestly not sure why this is allowing [ and ]

Copy link
Member

Choose a reason for hiding this comment

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

Originally because of Vapor's support for "array" URL parameters, but that doesn't matter for this usage.

return allowed
}()
}
84 changes: 43 additions & 41 deletions Tests/VaporTests/ContentTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import XCTVapor
import XCTest
import Vapor
import NIOCore
import NIOHTTP1
import NIOEmbedded
import NIOHTTP1
import Vapor
import XCTest
import XCTVapor

final class ContentTests: XCTestCase {
func testContent() throws {
Expand Down Expand Up @@ -69,7 +69,7 @@ final class ContentTests: XCTestCase {
let app = Application(.testing)
defer { app.shutdown() }

app.routes.get("decode_error") { req -> String in
app.routes.get("decode_error") { _ -> String in
struct Foo: Decodable {
var name: String
var bar: Int
Expand All @@ -95,7 +95,7 @@ final class ContentTests: XCTestCase {
let app = Application(.testing)
defer { app.shutdown() }

app.routes.get("encode") { req -> Response in
app.routes.get("encode") { _ -> Response in
let res = Response()
try res.content.encode(FooContent())
try res.content.encode(FooContent(), as: .json)
Expand Down Expand Up @@ -153,7 +153,7 @@ final class ContentTests: XCTestCase {
XCTAssertContains(res.body.string, "decoded!")
}
}

func testMultipartDecode() throws {
let data = """
--123\r
Expand Down Expand Up @@ -199,7 +199,7 @@ final class ContentTests: XCTestCase {
XCTAssertEqualJSON(res.body.string, expected)
}
}

func testMultipartDecodedEmptyMultipartForm() throws {
let data = """
--123\r
Expand Down Expand Up @@ -254,7 +254,7 @@ final class ContentTests: XCTestCase {
XCTAssertEqual(res.status, .unprocessableEntity)
}
}

func testMultipartDecodeUnicode() throws {
let data = """
--123\r
Expand Down Expand Up @@ -312,8 +312,8 @@ final class ContentTests: XCTestCase {
let app = Application(.testing)
defer { app.shutdown() }

app.get("multipart") { req -> User in
return User(
app.get("multipart") { _ -> User in
User(
name: "Vapor",
age: 4,
image: File(data: "<contents of image>", filename: "droplet.png")
Expand All @@ -328,7 +328,7 @@ final class ContentTests: XCTestCase {
XCTAssertContains(res.body.string, "name=\"image\"")
}
}

func testMultiPartEncodeUnicode() throws {
struct User: Content {
static let defaultContentType: HTTPMediaType = .formData
Expand All @@ -340,8 +340,8 @@ final class ContentTests: XCTestCase {
let app = Application(.testing)
defer { app.shutdown() }

app.get("multipart") { req -> User in
return User(
app.get("multipart") { _ -> User in
User(
name: "Vapor",
age: 4,
image: File(data: "<contents of image>", filename: "UTF-8\'\'%E5%A5%B9%E5%9C%A8%E5%90%83%E6%B0%B4%E6%9E%9C.png")
Expand Down Expand Up @@ -396,8 +396,8 @@ final class ContentTests: XCTestCase {
let app = Application(.testing)
defer { app.shutdown() }

app.get("urlencodedform") { req -> User in
return User(name: "Vapor", age: 3, luckyNumbers: [5, 7])
app.get("urlencodedform") { _ -> User in
User(name: "Vapor", age: 3, luckyNumbers: [5, 7])
}
try app.testable().test(.GET, "/urlencodedform") { res in
XCTAssertEqual(res.status.code, 200)
Expand All @@ -414,7 +414,7 @@ final class ContentTests: XCTestCase {
defer { app.shutdown() }

app.get("check") { (req: Request) -> String in
return "\(req.headers.first(name: .init("X-Test-Value")) ?? "MISSING").\(req.headers.first(name: .contentType) ?? "?")"
"\(req.headers.first(name: .init("X-Test-Value")) ?? "MISSING").\(req.headers.first(name: .contentType) ?? "?")"
}

try app.test(.GET, "/check", headers: ["X-Test-Value": "PRESENT"], beforeRequest: { req in
Expand All @@ -429,16 +429,16 @@ final class ContentTests: XCTestCase {
defer { app.shutdown() }

app.get("check") { (req: Request) -> String in
return "\(req.headers.first(name: .init("X-Test-Value")) ?? "MISSING").\(req.headers.first(name: .contentType) ?? "?")"
"\(req.headers.first(name: .init("X-Test-Value")) ?? "MISSING").\(req.headers.first(name: .contentType) ?? "?")"
}
// Me and my sadistic sense of humor.
ContentConfiguration.global.use(decoder: try! ContentConfiguration.global.requireDecoder(for: .json), for: .xml)

try app.testable().test(.GET, "/check", headers: [
"X-Test-Value": "PRESENT"
], beforeRequest: { req in
try req.content.encode(["foo": "bar"], as: .json)
req.headers.contentType = .xml
], beforeRequest: { req in
try req.content.encode(["foo": "bar"], as: .json)
req.headers.contentType = .xml
}) { res in
XCTAssertEqual(res.body.string, "PRESENT.application/xml; charset=utf-8")
}
Expand Down Expand Up @@ -473,7 +473,7 @@ final class ContentTests: XCTestCase {
let content = try request.content.decode(SampleContent.self)
XCTAssertEqual(content.name, "new name after decode")
}

func testSupportsJsonApi() throws {
let app = Application()
defer { app.shutdown() }
Expand All @@ -499,7 +499,7 @@ final class ContentTests: XCTestCase {

let request = Request(
application: app,
collectedBody: .init(string:""),
collectedBody: .init(string: ""),
on: app.eventLoopGroup.any()
)
request.url.query = "name=before+decode"
Expand Down Expand Up @@ -531,17 +531,17 @@ final class ContentTests: XCTestCase {
func testEncodePercentEncodedQuery() throws {
let app = Application()
defer { app.shutdown() }

struct Foo: Content {
var status: String
}

var request = ClientRequest(url: .init(scheme: "https", host: "example.com", path: "/api"))
try request.query.encode(Foo(status:
"⬆️ taylorswift just released swift-mongodb v0.10.1 – use BSON and MongoDB in pure Swift\n\nhttps://swiftpackageindex.com/tayloraswift/swift-mongodb#releases"
))

XCTAssertEqual(request.url.string, "https://example.com/api?status=%E2%AC%86%EF%B8%8F%20taylorswift%20just%20released%20swift-mongodb%20v0.10.1%20%E2%80%93%20use%20BSON%20and%20MongoDB%20in%20pure%20Swift%0A%0Ahttps://swiftpackageindex.com/tayloraswift/swift-mongodb%23releases")
XCTAssertEqual(request.url.string, "https://example.com/api?status=%E2%AC%86%EF%B8%8F%20taylorswift%20just%20released%20swift-mongodb%20v0.10.1%20%E2%80%93%20use%20BSON%20and%20MongoDB%20in%20pure%20Swift%0A%0Ahttps%3A%2F%2Fswiftpackageindex.com%2Ftayloraswift%2Fswift-mongodb%23releases")
}

func testSnakeCaseCodingKeyError() throws {
Expand All @@ -557,6 +557,7 @@ final class ContentTests: XCTestCase {
enum CodingKeys: String, CodingKey {
case id, title, isFree = "is_free"
}

let id: UUID?
let title: String
let isFree: Bool
Expand All @@ -572,7 +573,7 @@ final class ContentTests: XCTestCase {
func testDataCorruptionError() throws {
let app = Application()
defer { app.shutdown() }

let req = Request(
application: app,
method: .GET,
Expand All @@ -581,7 +582,7 @@ final class ContentTests: XCTestCase {
collectedBody: ByteBuffer(string: #"{"badJson: "Key doesn't have a trailing quote"}"#),
on: app.eventLoopGroup.any()
)

struct DecodeModel: Content {
let badJson: String
}
Expand All @@ -596,12 +597,12 @@ final class ContentTests: XCTestCase {
func testValueNotFoundError() throws {
let app = Application()
defer { app.shutdown() }

let req = Request(application: app, on: app.eventLoopGroup.any())
try req.content.encode([
"items": ["1"]
], as: .json)

struct DecodeModel: Content {
struct Item: Content {
init(from decoder: Decoder) throws {
Expand All @@ -611,7 +612,7 @@ final class ContentTests: XCTestCase {
fatalError()
}
}

let items: Item
}
XCTAssertThrowsError(try req.content.decode(DecodeModel.self)) { error in
Expand All @@ -625,18 +626,19 @@ final class ContentTests: XCTestCase {
func testTypeMismatchError() throws {
let app = Application()
defer { app.shutdown() }

let req = Request(application: app, on: app.eventLoopGroup.any())
try req.content.encode([
"item": [
"title": "The title"
]
], as: .json)

struct DecodeModel: Content {
struct Item: Content {
let title: Int
}

let item: Item
}
XCTAssertThrowsError(try req.content.decode(DecodeModel.self)) { error in
Expand All @@ -652,13 +654,13 @@ final class ContentTests: XCTestCase {
let app = Application(.testing)
defer { app.shutdown() }

app.routes.get("plaintext") { (req) -> Response in
app.routes.get("plaintext") { _ -> Response in
let res = Response()
try res.content.encode(data, as: .plainText)
return res
}

app.routes.get("empty-plaintext") { (req) -> Response in
app.routes.get("empty-plaintext") { _ -> Response in
let res = Response()
try res.content.encode("", as: .plainText)
return res
Expand Down Expand Up @@ -704,15 +706,15 @@ final class ContentTests: XCTestCase {
XCTAssertEqual(res.status, .badRequest)
}
}

func testContentIsBool() throws {
let app = Application(.testing)
defer { app.shutdown() }
app.routes.get("success") { req in
return true

app.routes.get("success") { _ in
true
}

try app.testable().test(.GET, "/success") { res in
XCTAssertEqual(try res.content.decode(Bool.self), true)
}
Expand All @@ -733,7 +735,7 @@ private struct SampleContent: Content {

private struct JsonApiContent: Content {
struct Meta: Codable {}

var data: [String]
var meta = Meta()
}
Loading
Loading