Skip to content

Don't output body for json DELETE requests #650

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

Merged
merged 2 commits into from
Apr 16, 2025
Merged

Don't output body for json DELETE requests #650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
19dfc39
Don't output body for DELETE requests
adam-fowler Apr 15, 2025
6fd28a5
Add test for empty HEAD and DELETE JSON requests
adam-fowler Apr 16, 2025
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
5 changes: 4 additions & 1 deletion Sources/SotoCore/HTTP/AWSHTTPRequest.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ extension AWSHTTPRequest {
encoder.userInfo[.awsRequest] = requestEncoderContainer
encoder.dateEncodingStrategy = .secondsSince1970
let buffer = try encoder.encodeAsByteBuffer(input, allocator: configuration.byteBufferAllocator)
if method == .GET || method == .HEAD, buffer == ByteBuffer(string: "{}") {
// GET, HEAD and DELETE methods should have an empty body
if method == .GET || method == .HEAD || method == .DELETE, buffer == ByteBuffer(string: "{}") {
body = .init()
} else {
body = .init(buffer: buffer)
Expand Down Expand Up @@ -260,6 +261,8 @@ extension AWSHTTPRequest {
guard self.headers["content-type"].first == nil else {
return
}
// in theory we don't need this check as the check for an empty body should skip adding
// a header and a GET or HEAD request should have an empty body
guard self.method != .GET, self.method != .HEAD else {
Copy link
Member

Choose a reason for hiding this comment

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

Do we not want to check for DELTE here as well?

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 trying to add code that has the least effect. I can't trust there isn't a service that includes a body with a delete method. The code above basically removes the body if it is {}.

return
}
Expand Down
24 changes: 20 additions & 4 deletions Tests/SotoCoreTests/AWSRequestTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -501,13 +501,29 @@ class AWSRequestTests: XCTestCase {
}

/// JSON POST request require a body even if there is no data to POST
func testEmptyJsonObject() {
func testEmptyPostJsonObject() throws {
struct Input: AWSEncodableShape {}
let input = Input()
let config = createServiceConfig(serviceProtocol: .json(version: "1.0"), endpoint: "https://test.com")
var request: AWSHTTPRequest?
XCTAssertNoThrow(request = try AWSHTTPRequest(operation: "Test", path: "/", method: .POST, input: input, configuration: config))
XCTAssertEqual(request?.body.asString(), "{}")
let request = try AWSHTTPRequest(operation: "Test", path: "/", method: .POST, input: input, configuration: config)
XCTAssertEqual(request.body.asString(), "{}")
XCTAssertEqual(request.headers["content-type"].first, "application/x-amz-json-1.0")
}

/// JSON GET, HEAD, DELETE requests should not output a body if it is empty ie `{}`
func testEmptyGetJsonObject() throws {
struct Input: AWSEncodableShape {}
let input = Input()
let config = createServiceConfig(serviceProtocol: .json(version: "1.0"), endpoint: "https://test.com")
let request = try AWSHTTPRequest(operation: "Test", path: "/", method: .GET, input: input, configuration: config)
XCTAssertEqual(request.body.asString(), "")
XCTAssertNil(request.headers["content-type"].first)
let request2 = try AWSHTTPRequest(operation: "Test", path: "/", method: .HEAD, input: input, configuration: config)
XCTAssertEqual(request2.body.asString(), "")
XCTAssertNil(request2.headers["content-type"].first)
let request3 = try AWSHTTPRequest(operation: "Test", path: "/", method: .DELETE, input: input, configuration: config)
XCTAssertEqual(request3.body.asString(), "")
XCTAssertNil(request3.headers["content-type"].first)
}

/// Test host prefix
Expand Down