Skip to content

Commit 6146dd1

Browse files
authored
Improve validation of Websocket requests. (#59)
* Verify `connection` and `upgrade` headers for HTTP/1 requests. * Add test for status for HTTP/2.
1 parent 5694b4e commit 6146dd1

File tree

3 files changed

+63
-9
lines changed

3 files changed

+63
-9
lines changed

lib/async/websocket/connect_request.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
module Async
1414
module WebSocket
15-
# This is required for HTTP/1.x to upgrade the connection to the WebSocket protocol.
15+
# This is required for HTTP/2 to establish a connection using the WebSocket protocol.
1616
# See https://tools.ietf.org/html/rfc8441 for more details.
1717
class ConnectRequest < ::Protocol::HTTP::Request
1818
include ::Protocol::WebSocket::Headers

lib/async/websocket/upgrade_request.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
module Async
1818
module WebSocket
1919
# This is required for HTTP/1.x to upgrade the connection to the WebSocket protocol.
20+
# See https://tools.ietf.org/html/rfc6455
2021
class UpgradeRequest < ::Protocol::HTTP::Request
2122
include ::Protocol::WebSocket::Headers
2223

@@ -75,8 +76,9 @@ def call(connection)
7576
raise ProtocolError, "Invalid accept digest, expected #{expected_accept_digest.inspect}, got #{accept_digest.inspect}!"
7677
end
7778
end
79+
verified = accept_digest && Array(response.protocol) == %w(websocket) && response.headers['connection']&.include?('upgrade')
7880

79-
return Wrapper.new(response, verified: !!accept_digest)
81+
return Wrapper.new(response, verified: verified)
8082
end
8183
end
8284
end

test/async/websocket/client.rb

Lines changed: 59 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -105,17 +105,53 @@
105105
end
106106
end
107107

108+
FailedToNegotiate = Sus::Shared("a failed websocket request") do
109+
it 'raises an error' do
110+
expect do
111+
Async::WebSocket::Client.connect(client_endpoint) {}
112+
end.to raise_exception(Async::WebSocket::ProtocolError, message: be =~ /Failed to negotiate connection/)
113+
end
114+
end
115+
108116
describe Async::WebSocket::Client do
109117
include Sus::Fixtures::Async::HTTP::ServerContext
110118

111119
with 'http/1' do
112120
let(:protocol) {Async::HTTP::Protocol::HTTP1}
113121
it_behaves_like ClientExamples
114122

123+
def valid_headers(request)
124+
{
125+
'connection' => 'upgrade',
126+
'upgrade' => 'websocket',
127+
'sec-websocket-accept' => Protocol::WebSocket::Headers::Nounce.accept_digest(request.headers['sec-websocket-key'].first)
128+
}
129+
end
130+
131+
with 'invalid connection header' do
132+
let(:app) do
133+
Protocol::HTTP::Middleware.for do |request|
134+
Protocol::HTTP::Response[101, valid_headers(request).except('connection'), []]
135+
end
136+
end
137+
138+
it_behaves_like FailedToNegotiate
139+
end
140+
141+
with 'invalid upgrade header' do
142+
let(:app) do
143+
Protocol::HTTP::Middleware.for do |request|
144+
Protocol::HTTP::Response[101, valid_headers(request).except('upgrade'), []]
145+
end
146+
end
147+
148+
it_behaves_like FailedToNegotiate
149+
end
150+
115151
with 'invalid sec-websocket-accept header' do
116152
let(:app) do
117153
Protocol::HTTP::Middleware.for do |request|
118-
Protocol::HTTP::Response[101, {'sec-websocket-accept'=>'wrong-digest'}, []]
154+
Protocol::HTTP::Response[101, valid_headers(request).merge('sec-websocket-accept'=>'wrong-digest'), []]
119155
end
120156
end
121157

@@ -125,24 +161,40 @@
125161
end.to raise_exception(Async::WebSocket::ProtocolError, message: be =~ /Invalid accept digest/)
126162
end
127163
end
128-
164+
129165
with 'missing sec-websocket-accept header' do
130166
let(:app) do
131167
Protocol::HTTP::Middleware.for do |request|
132-
Protocol::HTTP::Response[101, {}, []]
168+
Protocol::HTTP::Response[101, valid_headers(request).except('sec-websocket-accept'), []]
133169
end
134170
end
135171

136-
it 'raises an error' do
137-
expect do
138-
Async::WebSocket::Client.connect(client_endpoint) {}
139-
end.to raise_exception(Async::WebSocket::ProtocolError, message: be =~ /Failed to negotiate connection/)
172+
it_behaves_like FailedToNegotiate
173+
end
174+
175+
with 'invalid status' do
176+
let(:app) do
177+
Protocol::HTTP::Middleware.for do |request|
178+
Protocol::HTTP::Response[403, valid_headers(request), []]
179+
end
140180
end
181+
182+
it_behaves_like FailedToNegotiate
141183
end
142184
end
143185

144186
with 'http/2' do
145187
let(:protocol) {Async::HTTP::Protocol::HTTP2}
146188
it_behaves_like ClientExamples
189+
190+
with 'invalid status' do
191+
let(:app) do
192+
Protocol::HTTP::Middleware.for do |request|
193+
Protocol::HTTP::Response[403, {}, []]
194+
end
195+
end
196+
197+
it_behaves_like FailedToNegotiate
198+
end
147199
end
148200
end

0 commit comments

Comments
 (0)