Skip to content

Commit

Permalink
Added enforcement of protocol handling for out-of-order headers (#136)
Browse files Browse the repository at this point in the history 
Added enforcement of protocol handling for out-of-order headers.

Implemented validation (order and casing) of received headers. Failure will trigger a stream error (PROTOCOL_ERROR). Header compressor now emits pseudo-headers ahead of the rest (by partitioning supplied list).
  • Loading branch information
@byronformwalt @igrigorik
byronformwalt authored and igrigorik committed Jul 23, 2018
1 parent dfd76af commit 1cb2518
Show file tree
Hide file tree
Showing 3 changed files with 140 additions and 29 deletions.
14 changes: 12 additions & 2 deletions lib/http/2/compressor.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,8 @@ def header(h, buffer = Buffer.new)
# @return [Buffer]
def encode(headers)
buffer = Buffer.new

pseudo_headers, regular_headers = headers.partition { |f, _| f.start_with? ':' }
headers = [*pseudo_headers, *regular_headers]
commands = @cc.encode(headers)
commands.each do |cmd|
buffer << header(cmd)
Expand Down Expand Up @@ -549,7 +550,16 @@ def header(buf)
# @return [Array] +[[name, value], ...]+
def decode(buf)
list = []
list << @cc.process(header(buf)) until buf.empty?
decoding_pseudo_headers = true
until buf.empty?
next_header = @cc.process(header(buf))
is_pseudo_header = next_header.first.start_with? ':'
if !decoding_pseudo_headers && is_pseudo_header
fail ProtocolError, 'one or more pseudo headers encountered after regular headers'
end
decoding_pseudo_headers = is_pseudo_header
list << next_header
end
list.compact
end
end
Expand Down
6 changes: 5 additions & 1 deletion lib/http/2/connection.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -606,8 +606,12 @@ def decode_headers(frame)
frame[:payload] = @decompressor.decode(frame[:payload])
end

rescue StandardError => e
rescue CompressionError => e
connection_error(:compression_error, e: e)
rescue ProtocolError => e
connection_error(:protocol_error, e: e)
rescue StandardError => e
connection_error(:internal_error, e: e)
end

# Encode headers payload and update connection compressor state.
Expand Down
149 changes: 123 additions & 26 deletions spec/compressor_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,78 @@
},
],
},
{ title: 'D.4.a. Request Examples with Huffman - Client Handling of Improperly Ordered Headers',
type: :request,
table_size: 4096,
huffman: :always,
streams: [
{ wire: '8286 8441 8cf1 e3c2 e5f2 3a6b a0ab 90f4 ff',
emitted: [
[':method', 'GET'],
[':scheme', 'http'],
[':path', '/'],
[':authority', 'www.example.com'],
],
table: [
[':authority', 'www.example.com'],
],
table_size: 57,
},
{ wire: '8286 84be 5886 a8eb 1064 9cbf',
emitted: [
[':method', 'GET'],
[':scheme', 'http'],
['cache-control', 'no-cache'],
[':path', '/'],
[':authority', 'www.example.com'],
],
table: [
['cache-control', 'no-cache'],
[':authority', 'www.example.com'],
],
table_size: 110,
},
{ wire: "8287 85bf 4088 25a8 49e9 5ba9 7d7f 8925
a849 e95b b8e8 b4bf",
emitted: [
[':method', 'GET'],
[':scheme', 'https'],
['custom-key', 'custom-value'],
[':path', '/index.html'],
[':authority', 'www.example.com'],
],
table: [
['custom-key', 'custom-value'],
['cache-control', 'no-cache'],
[':authority', 'www.example.com'],
],
table_size: 164,
},
],
},
{ title: 'D.4.b. Request Examples with Huffman - Server Handling of Improperly Ordered Headers',
type: :request,
bypass_encoder: true,
table_size: 4096,
huffman: :always,
streams: [
{ wire: '8286408825a849e95ba97d7f8925a849e95bb8e8b4bf84418cf1e3c2e5f23a6ba0ab90f4ff',
emitted: [
[':method', 'GET'],
[':scheme', 'http'],
['custom-key', 'custom-value'],
[':path', '/'],
[':authority', 'www.example.com'],
],
table: [
['custom-key', 'custom-value'],
[':authority', 'www.example.com'],
],
table_size: 111,
has_bad_headers: true,
},
],
},
{ title: 'D.5. Response Examples without Huffman',
type: :response,
table_size: 256,
Expand Down Expand Up @@ -485,25 +557,38 @@
before do
(0...nth).each do |i|
bytes = [ex[:streams][i][:wire].delete(" \n")].pack('H*')
@dc.decode(HTTP2::Buffer.new(bytes))
if ex[:streams][i][:has_bad_headers]
expect { @dc.decode(HTTP2::Buffer.new(bytes)) }.to raise_error ProtocolError
else
@dc.decode(HTTP2::Buffer.new(bytes))
end
end
end
subject do
bytes = [ex[:streams][nth][:wire].delete(" \n")].pack('H*')
@emitted = @dc.decode(HTTP2::Buffer.new(bytes))
end
it 'should emit expected headers' do
subject
# order-perserving compare
expect(@emitted).to eq ex[:streams][nth][:emitted]
end
it 'should update header table' do
subject
expect(@dc.instance_eval { @cc.table }).to eq ex[:streams][nth][:table]
end
it 'should compute header table size' do
subject
expect(@dc.instance_eval { @cc.current_table_size }).to eq ex[:streams][nth][:table_size]
if ex[:streams][nth][:has_bad_headers]
it 'should raise CompressionError' do
bytes = [ex[:streams][nth][:wire].delete(" \n")].pack('H*')
expect { @dc.decode(HTTP2::Buffer.new(bytes)) }.to raise_error ProtocolError
end
else
subject do
bytes = [ex[:streams][nth][:wire].delete(" \n")].pack('H*')
@emitted = @dc.decode(HTTP2::Buffer.new(bytes))
end
it 'should emit expected headers' do
subject
# partitioned compare
pseudo_headers, headers = ex[:streams][nth][:emitted].partition { |f, _| f.start_with? ':' }
partitioned_headers = pseudo_headers + headers
expect(@emitted).to eq partitioned_headers
end
it 'should update header table' do
subject
expect(@dc.instance_eval { @cc.table }).to eq ex[:streams][nth][:table]
end
it 'should compute header table size' do
subject
expect(@dc.instance_eval { @cc.current_table_size }).to eq ex[:streams][nth][:table_size]
end
end
end
end
Expand All @@ -513,6 +598,7 @@

context 'encode' do
spec_examples.each do |ex|
next if ex[:bypass_encoder]
context "spec example #{ex[:title]}" do
ex[:streams].size.times do |nth|
context "request #{nth + 1}" do
Expand All @@ -522,22 +608,33 @@
end
before do
(0...nth).each do |i|
@cc.encode(ex[:streams][i][:emitted])
if ex[:streams][i][:has_bad_headers]
@cc.encode(ex[:streams][i][:emitted], ensure_proper_ordering: false)
else
@cc.encode(ex[:streams][i][:emitted])
end
end
end
subject do
@cc.encode(ex[:streams][nth][:emitted])
if ex[:streams][nth][:has_bad_headers]
@cc.encode(ex[:streams][nth][:emitted], ensure_proper_ordering: false)
else
@cc.encode(ex[:streams][nth][:emitted])
end
end
it 'should emit expected bytes on wire' do
puts subject.unpack('H*').first
expect(subject.unpack('H*').first).to eq ex[:streams][nth][:wire].delete(" \n")
end
it 'should update header table' do
subject
expect(@cc.instance_eval { @cc.table }).to eq ex[:streams][nth][:table]
end
it 'should compute header table size' do
subject
expect(@cc.instance_eval { @cc.current_table_size }).to eq ex[:streams][nth][:table_size]
unless ex[:streams][nth][:has_bad_headers]
it 'should update header table' do
subject
expect(@cc.instance_eval { @cc.table }).to eq ex[:streams][nth][:table]
end
it 'should compute header table size' do
subject
expect(@cc.instance_eval { @cc.current_table_size }).to eq ex[:streams][nth][:table_size]
end
end
end
end
Expand Down

0 comments on commit 1cb2518

Please sign in to comment.