Skip to content
New issue

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

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

Already on GitHub? # to your account

Remove chars "@()" from s3PathAllowedCharacters #604

Merged
merged 3 commits into from
Jun 17, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
4 changes: 2 additions & 2 deletions Sources/Soto/Extensions/S3/S3RequestMiddleware.swift
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,10 @@ public struct S3RequestMiddleware: AWSServiceMiddleware {
}
}

static let pathAllowedCharacters = CharacterSet.urlPathAllowed.subtracting(.init(charactersIn: "+"))
static let s3PathAllowedCharacters = CharacterSet.urlPathAllowed.subtracting(.init(charactersIn: "+@()"))
/// percent encode path value.
private static func urlEncodePath(_ value: String) -> String {
return value.addingPercentEncoding(withAllowedCharacters: Self.pathAllowedCharacters) ?? value
return value.addingPercentEncoding(withAllowedCharacters: Self.s3PathAllowedCharacters) ?? value
}

func createBucketFixup(request: inout AWSRequest) {
Expand Down
7 changes: 7 additions & 0 deletions Tests/SotoTests/Services/S3/S3Tests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,13 @@ class S3Tests: XCTestCase {
XCTAssertEqual(response.body?.asString(), contents)
XCTAssertNotNil(response.lastModified)
}
.flatMap { _ -> EventLoopFuture<S3.ListObjectsV2Output> in
return Self.s3.listObjectsV2(.init(bucket: name))
}
.map { result in
let contents = result.contents
XCTAssertNotNil(contents?.first(where: { $0.key == filename }))
}
.flatAlways { _ in
return Self.deleteBucket(name: name, s3: Self.s3)
}
Expand Down