Skip to content

Commit d4a5be1

Browse files
gesellixswankjesse
andauthored
Fix RequestBody events on upgraded connections (#8970)
* Add SocketSink{Start,End} events * Add SocketSource{Start,End} events * Emit SocketSink/Source events * Ensure same connection * Chore * Chore * There and back again * Update okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/Exchange.kt * Update okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/Exchange.kt --------- Co-authored-by: Jesse Wilson <[email protected]>
1 parent 112a19d commit d4a5be1

File tree

3 files changed

+42
-34
lines changed

3 files changed

+42
-34
lines changed

okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/Exchange.kt

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ class Exchange(
4646
internal var isDuplex: Boolean = false
4747
private set
4848

49+
/** True if the request body should not be used, but the socket, instead. */
50+
internal var isSocket: Boolean = false
51+
private set
52+
4953
/** True if there was an exception on the connection to the peer. */
5054
internal var hasFailure: Boolean = false
5155
private set
@@ -143,11 +147,10 @@ class Exchange(
143147
fun peekTrailers(): Headers? = codec.peekTrailers()
144148

145149
fun upgradeToSocket(): Socket {
150+
isSocket = true
146151
call.timeoutEarlyExit()
147152
(codec.carrier as RealConnection).useAsSocket()
148153

149-
eventListener.requestBodyStart(call)
150-
151154
return object : Socket {
152155
override fun cancel() {
153156
this@Exchange.cancel()
@@ -233,6 +236,7 @@ class Exchange(
233236
) : ForwardingSink(delegate) {
234237
private var completed = false
235238
private var bytesReceived = 0L
239+
private var invokeStartEvent = isSocket
236240
private var closed = false
237241

238242
@Throws(IOException::class)
@@ -247,6 +251,10 @@ class Exchange(
247251
)
248252
}
249253
try {
254+
if (invokeStartEvent) {
255+
invokeStartEvent = false
256+
eventListener.requestBodyStart(call)
257+
}
250258
super.write(source, byteCount)
251259
this.bytesReceived += byteCount
252260
} catch (e: IOException) {

okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/http/CallServerInterceptor.kt

Lines changed: 29 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -44,39 +44,41 @@ object CallServerInterceptor : Interceptor {
4444
try {
4545
exchange.writeRequestHeaders(request)
4646

47-
if (hasRequestBody) {
48-
// If there's a "Expect: 100-continue" header on the request, wait for a "HTTP/1.1 100
49-
// Continue" response before transmitting the request body. If we don't get that, return
50-
// what we did get (such as a 4xx response) without ever transmitting the request body.
51-
if ("100-continue".equals(request.header("Expect"), ignoreCase = true)) {
52-
exchange.flushRequest()
53-
responseBuilder = exchange.readResponseHeaders(expectContinue = true)
54-
exchange.responseHeadersStart()
55-
invokeStartEvent = false
56-
}
57-
if (responseBuilder == null) {
58-
if (requestBody.isDuplex()) {
59-
// Prepare a duplex body so that the application can send a request body later.
47+
if (!isUpgradeRequest) {
48+
if (hasRequestBody) {
49+
// If there's a "Expect: 100-continue" header on the request, wait for a "HTTP/1.1 100
50+
// Continue" response before transmitting the request body. If we don't get that, return
51+
// what we did get (such as a 4xx response) without ever transmitting the request body.
52+
if ("100-continue".equals(request.header("Expect"), ignoreCase = true)) {
6053
exchange.flushRequest()
61-
val bufferedRequestBody = exchange.createRequestBody(request, true).buffer()
62-
requestBody.writeTo(bufferedRequestBody)
54+
responseBuilder = exchange.readResponseHeaders(expectContinue = true)
55+
exchange.responseHeadersStart()
56+
invokeStartEvent = false
57+
}
58+
if (responseBuilder == null) {
59+
if (requestBody.isDuplex()) {
60+
// Prepare a duplex body so that the application can send a request body later.
61+
exchange.flushRequest()
62+
val bufferedRequestBody = exchange.createRequestBody(request, true).buffer()
63+
requestBody.writeTo(bufferedRequestBody)
64+
} else {
65+
// Write the request body if the "Expect: 100-continue" expectation was met.
66+
val bufferedRequestBody = exchange.createRequestBody(request, false).buffer()
67+
requestBody.writeTo(bufferedRequestBody)
68+
bufferedRequestBody.close()
69+
}
6370
} else {
64-
// Write the request body if the "Expect: 100-continue" expectation was met.
65-
val bufferedRequestBody = exchange.createRequestBody(request, false).buffer()
66-
requestBody.writeTo(bufferedRequestBody)
67-
bufferedRequestBody.close()
71+
exchange.noRequestBody()
72+
if (!exchange.connection.isMultiplexed) {
73+
// If the "Expect: 100-continue" expectation wasn't met, prevent the HTTP/1 connection
74+
// from being reused. Otherwise we're still obligated to transmit the request body to
75+
// leave the connection in a consistent state.
76+
exchange.noNewExchangesOnConnection()
77+
}
6878
}
6979
} else {
7080
exchange.noRequestBody()
71-
if (!exchange.connection.isMultiplexed) {
72-
// If the "Expect: 100-continue" expectation wasn't met, prevent the HTTP/1 connection
73-
// from being reused. Otherwise we're still obligated to transmit the request body to
74-
// leave the connection in a consistent state.
75-
exchange.noNewExchangesOnConnection()
76-
}
7781
}
78-
} else if (!isUpgradeRequest) {
79-
exchange.noRequestBody()
8082
}
8183

8284
if (requestBody == null || !requestBody.isDuplex()) {

okhttp/src/jvmTest/kotlin/okhttp3/internal/http/HttpUpgradesTest.kt

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -208,8 +208,8 @@ class HttpUpgradesTest {
208208
"RequestHeadersEnd",
209209
"ResponseHeadersStart",
210210
"ResponseHeadersEnd",
211-
"RequestBodyStart",
212211
"FollowUpDecision",
212+
"RequestBodyStart",
213213
"ResponseBodyStart",
214214
"ResponseBodyEnd",
215215
"RequestBodyEnd",
@@ -233,17 +233,15 @@ class HttpUpgradesTest {
233233
"ConnectionAcquired",
234234
"RequestHeadersStart",
235235
"RequestHeadersEnd",
236-
"RequestBodyStart",
237-
"RequestBodyEnd",
238236
"ResponseHeadersStart",
239237
"ResponseHeadersEnd",
240-
"RequestBodyStart",
241238
"FollowUpDecision",
239+
"RequestBodyStart",
242240
"ResponseBodyStart",
243241
"ResponseBodyEnd",
242+
"RequestBodyEnd",
244243
"ConnectionReleased",
245244
"CallEnd",
246-
"RequestBodyEnd",
247245
)
248246
}
249247

0 commit comments

Comments
 (0)