Skip to content

Commit 9ad8a89

Browse files
author
Stuart Espey
committed
Fix body-writer exception causes hang
A sufficiently complex body-writer may not be able to complete writing the body, and the only practical approach is to throw an exception. Exceptions are normally thrown from body-writers when the socket is closed by the remote peer. If a Raw response body-writer throws an exception before completing the body the http session will be left out of sync and will hang indefinitely waiting to read the next request. The fix results in the socket being closed and not attempting to read the next request when an error occurs. If a content-length is provided then the network error can be detected by the remote peer, otherwise a truncated body is returned to the client.
1 parent 9483a5d commit 9ad8a89

16 files changed

+152
-0
lines changed

XCode/Sources/HttpServerIO.swift

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,16 @@ open class HttpServerIO {
130130
}
131131
} catch {
132132
print("Failed to send response: \(error)")
133+
/*
134+
The bodyWriter threw an exception, this could be because the socket is closed, in
135+
which case we fail when we attempt to read from the socket again, or it could be
136+
because the bodyWriter was unable to finish writing the body, session will be out
137+
of sync. If we kept the connection alive, then the client will be waiting for the
138+
remainder of the body until it gives up, and we'll never get the next request.
139+
140+
Ideal action is to possibly abruptly close the connection.
141+
*/
142+
break
133143
}
134144
if let session = response.socketSession() {
135145
delegate?.socketConnectionReceived(socket)

XCode/Swifter.xcodeproj/project.pbxproj

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,9 @@
110110
7CDAB8131BE2A1D400C8A977 /* Main.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = 7CDAB80D1BE2A1D400C8A977 /* Main.storyboard */; };
111111
7CDAB8141BE2A1D400C8A977 /* Images.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 7CDAB80F1BE2A1D400C8A977 /* Images.xcassets */; };
112112
7CDAB8161BE2A1D400C8A977 /* ViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7CDAB8111BE2A1D400C8A977 /* ViewController.swift */; };
113+
AB83D32D2919F84500D4D434 /* ServerRawResponseTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = AB83D32C2919F84500D4D434 /* ServerRawResponseTests.swift */; };
114+
AB83D32E2919F84500D4D434 /* ServerRawResponseTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = AB83D32C2919F84500D4D434 /* ServerRawResponseTests.swift */; };
115+
AB83D32F2919F84500D4D434 /* ServerRawResponseTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = AB83D32C2919F84500D4D434 /* ServerRawResponseTests.swift */; };
113116
/* End PBXBuildFile section */
114117

115118
/* Begin PBXContainerItemProxy section */
@@ -222,6 +225,7 @@
222225
7CDAB80F1BE2A1D400C8A977 /* Images.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; path = Images.xcassets; sourceTree = "<group>"; };
223226
7CDAB8101BE2A1D400C8A977 /* Info.plist */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
224227
7CDAB8111BE2A1D400C8A977 /* ViewController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ViewController.swift; sourceTree = "<group>"; };
228+
AB83D32C2919F84500D4D434 /* ServerRawResponseTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ServerRawResponseTests.swift; sourceTree = "<group>"; };
225229
/* End PBXFileReference section */
226230

227231
/* Begin PBXFrameworksBuildPhase section */
@@ -429,6 +433,7 @@
429433
0858E7F31D68BB2600491CD1 /* IOSafetyTests.swift */,
430434
6A0D4511204E9988000A0726 /* MimeTypesTests.swift */,
431435
0C1F3CAC2265FC470076B6F5 /* SwifterTestsHttpResponseBody.swift */,
436+
AB83D32C2919F84500D4D434 /* ServerRawResponseTests.swift */,
432437
7B55EC94226E0E4F00042D23 /* ServerThreadingTests.swift */,
433438
);
434439
path = Tests;
@@ -781,6 +786,7 @@
781786
043660CD21FED35200497989 /* SwifterTestsHttpRouter.swift in Sources */,
782787
047F1F02226AB9AD00909B95 /* XCTestManifests.swift in Sources */,
783788
043660CE21FED35500497989 /* SwifterTestsHttpParser.swift in Sources */,
789+
AB83D32E2919F84500D4D434 /* ServerRawResponseTests.swift in Sources */,
784790
043660D521FED36C00497989 /* MimeTypesTests.swift in Sources */,
785791
0C1F3CAE2265FC470076B6F5 /* SwifterTestsHttpResponseBody.swift in Sources */,
786792
7B55EC96226E0E4F00042D23 /* ServerThreadingTests.swift in Sources */,
@@ -793,6 +799,7 @@
793799
files = (
794800
043660E721FED51600497989 /* PingServer.swift in Sources */,
795801
043660E921FED51B00497989 /* SwifterTestsWebSocketSession.swift in Sources */,
802+
AB83D32F2919F84500D4D434 /* ServerRawResponseTests.swift in Sources */,
796803
043660EA21FED51E00497989 /* IOSafetyTests.swift in Sources */,
797804
043660E821FED51900497989 /* SwifterTestsStringExtensions.swift in Sources */,
798805
043660E521FED51100497989 /* SwifterTestsHttpRouter.swift in Sources */,
@@ -912,6 +919,7 @@
912919
7C4785E91C71D15600A9FE73 /* SwifterTestsWebSocketSession.swift in Sources */,
913920
043660D421FED36900497989 /* IOSafetyTests.swift in Sources */,
914921
7CCD87721C660B250068099B /* SwifterTestsStringExtensions.swift in Sources */,
922+
AB83D32D2919F84500D4D434 /* ServerRawResponseTests.swift in Sources */,
915923
7B11AD4B21C9A8A6002F8820 /* SwifterTestsHttpRouter.swift in Sources */,
916924
0C1F3CAD2265FC470076B6F5 /* SwifterTestsHttpResponseBody.swift in Sources */,
917925
7B55EC95226E0E4F00042D23 /* ServerThreadingTests.swift in Sources */,
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
//
2+
// ServerRawResponseTests.swift
3+
// Swifter
4+
//
5+
// Created by Stuart Espey on 11/08/22.
6+
7+
import XCTest
8+
#if os(Linux)
9+
import FoundationNetworking
10+
#endif
11+
@testable import Swifter
12+
13+
class ServerRawResponseTests: XCTestCase {
14+
15+
public enum TestError: Error {
16+
case aborted
17+
}
18+
19+
var server: HttpServer!
20+
21+
override func setUp() {
22+
super.setUp()
23+
server = HttpServer()
24+
}
25+
26+
override func tearDown() {
27+
if server.operating {
28+
server.stop()
29+
}
30+
server = nil
31+
super.tearDown()
32+
}
33+
34+
/**
35+
Adds the handler, at the teststring path, then expects that XXX-CustomHeader is equal to testString, and the
36+
body. Test should pass instantly-ish.
37+
38+
If expectError is true, then an error is expected, and response and body should be nil
39+
*/
40+
func doHandlerTest( handler: @escaping (HttpRequest) -> HttpResponse, testString: String, expectError: Bool = false) {
41+
let path = "/\(testString)"
42+
server.GET[path] = handler
43+
44+
var requestExpectation: XCTestExpectation? = expectation(description: "Should handle the request quickly")
45+
46+
do {
47+
try server.start()
48+
49+
DispatchQueue.global().async {
50+
51+
let task = URLSession.shared.executeAsyncTask(path: path) { (body, response, error ) in
52+
53+
if expectError {
54+
XCTAssertNil(body)
55+
XCTAssertNil(response)
56+
XCTAssertNotNil(error)
57+
} else {
58+
XCTAssertNil(error)
59+
let response = response as? HTTPURLResponse
60+
XCTAssertNotNil(response)
61+
62+
let statusCode = response?.statusCode
63+
XCTAssertNotNil(statusCode)
64+
XCTAssertEqual(statusCode, 200)
65+
#if !os(Linux)
66+
if #available(iOS 13.0, macOS 10.15, tvOS 13.0, *) {
67+
let header = response?.value(forHTTPHeaderField: "XXX-Custom-Header") ?? ""
68+
XCTAssertEqual(header, testString)
69+
}
70+
#endif
71+
72+
XCTAssertEqual(body, testString.data(using: .utf8))
73+
}
74+
75+
requestExpectation?.fulfill()
76+
requestExpectation = nil
77+
}
78+
79+
task.resume()
80+
}
81+
82+
} catch let error {
83+
XCTFail("\(error)")
84+
}
85+
86+
waitForExpectations(timeout: 5, handler: nil)
87+
}
88+
89+
func testRawResponseWithBodyWriter() {
90+
91+
let testString = "normal"
92+
93+
let handler: ((HttpRequest) -> HttpResponse) = { _ in
94+
return HttpResponse.raw(200, "OK", ["XXX-Custom-Header": testString], {
95+
try $0.write([UInt8]((testString) .utf8))
96+
})
97+
}
98+
99+
doHandlerTest(handler: handler, testString: testString)
100+
}
101+
102+
func testFailedUnboundedResponseShouldNotHang() {
103+
104+
let testString = "unbounded"
105+
let handler: ((HttpRequest) -> HttpResponse) = { _ in
106+
return HttpResponse.raw(200, "OK", ["XXX-Custom-Header": testString], {
107+
try $0.write([UInt8]((testString) .utf8))
108+
109+
// simulates the body writer not being able to finish the body.
110+
throw TestError.aborted
111+
})
112+
}
113+
114+
doHandlerTest(handler: handler, testString: testString)
115+
}
116+
117+
func testFailedBoundedResponseShouldNotHang() {
118+
119+
let testString = "bounded" // content-length: 7
120+
let handler: ((HttpRequest) -> HttpResponse) = { _ in
121+
return HttpResponse.raw(200, "OK", [
122+
"XXX-Custom-Header": testString,
123+
"Content-Length": "\(testString.utf8.count+1)" // we'll be missing one byte
124+
], {
125+
try $0.write([UInt8]((testString) .utf8))
126+
127+
throw TestError.aborted
128+
})
129+
}
130+
131+
// because the length is known, we should trigger a network failure
132+
doHandlerTest(handler: handler, testString: testString, expectError: true)
133+
}
134+
}

0 commit comments

Comments
 (0)