Skip to content

Commit fd3a7d1

Browse files
committed
Explicit handling of upgrade body.
1 parent dfcacca commit fd3a7d1

File tree

2 files changed

+12
-9
lines changed

2 files changed

+12
-9
lines changed

lib/protocol/http1/connection.rb

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,12 @@ def read_tunnel_body
435435
read_remainder_body
436436
end
437437

438+
def read_upgrade_body
439+
# When you have an incoming upgrade request body, we must be extremely careful not to start reading it until the upgrade has been confirmed, otherwise if the upgrade was rejected and we started forwarding the incoming request body, it would desynchronize the connection (potential security issue).
440+
# We mitigate this issue by setting @persistent to false, which will prevent the connection from being reused, even if the upgrade fails (potential performance issue).
441+
read_remainder_body
442+
end
443+
438444
HEAD = "HEAD"
439445
CONNECT = "CONNECT"
440446

@@ -470,14 +476,11 @@ def read_response_body(method, status, headers)
470476
return nil
471477
end
472478

473-
if status >= 100 and status < 200
474-
# At the moment this is returned, the Remainder represents any
475-
# future response on the stream. The Remainder may be used directly
476-
# or discarded, or read_response may be called again.
477-
return read_remainder_body
479+
if status == 101
480+
return read_upgrade_body
478481
end
479482

480-
if status == 204 or status == 304
483+
if (status >= 100 and status < 200) or status == 204 or status == 304
481484
return nil
482485
end
483486

@@ -505,7 +508,7 @@ def read_request_body(method, headers)
505508

506509
# A successful upgrade response implies that the connection will become a tunnel immediately after the empty line that concludes the header fields.
507510
if headers[UPGRADE]
508-
return read_tunnel_body
511+
return read_upgrade_body
509512
end
510513

511514
# 6. If this is a request message and none of the above are true, then

test/protocol/http1/connection.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,8 +205,8 @@
205205
with "GET" do
206206
it "should ignore body for informational responses" do
207207
body = client.read_response_body("GET", 100, {'content-length' => '10'})
208-
expect(body).to be_a(::Protocol::HTTP1::Body::Remainder)
209-
expect(client.persistent).to be == false
208+
expect(body).to be_nil
209+
expect(client.persistent).to be == true
210210
end
211211

212212
it "should ignore body for no content responses" do

0 commit comments

Comments
 (0)