Skip to content

Commit fbe89eb

Browse files
ioquatixioquatix
committed
Improvements to tests and implementation.
1 parent 11fab9c commit fbe89eb

File tree

7 files changed

+92
-76
lines changed

7 files changed

+92
-76
lines changed

‎lib/protocol/http2/client.rb

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,25 @@ def send_connection_preface(settings = [])
2929
@framer.write_connection_preface
3030

3131
# We don't support RFC7540 priorities:
32-
settings = settings.to_a
33-
settings << [Settings::NO_RFC7540_PRIORITIES, 1]
32+
if settings.is_a?(Hash)
33+
settings = settings.dup
34+
else
35+
settings = settings.to_h
36+
end
37+
38+
unless settings.key?(Settings::NO_RFC7540_PRIORITIES)
39+
settings = settings.dup
40+
settings[Settings::NO_RFC7540_PRIORITIES] = 1
41+
end
3442

3543
send_settings(settings)
3644

3745
yield if block_given?
3846

3947
read_frame do |frame|
40-
raise ProtocolError, "First frame must be #{SettingsFrame}, but got #{frame.class}" unless frame.is_a? SettingsFrame
48+
unless frame.is_a? SettingsFrame
49+
raise ProtocolError, "First frame must be #{SettingsFrame}, but got #{frame.class}"
50+
end
4151
end
4252
else
4353
raise ProtocolError, "Cannot send connection preface in state #{@state}"

‎lib/protocol/http2/server.rb

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,16 @@ def read_connection_preface(settings = [])
2929
@framer.read_connection_preface
3030

3131
# We don't support RFC7540 priorities:
32-
settings = settings.to_a
33-
settings << [Settings::NO_RFC7540_PRIORITIES, 1]
32+
if settings.is_a?(Hash)
33+
settings = settings.dup
34+
else
35+
settings = settings.to_h
36+
end
37+
38+
unless settings.key?(Settings::NO_RFC7540_PRIORITIES)
39+
settings = settings.dup
40+
settings[Settings::NO_RFC7540_PRIORITIES] = 1
41+
end
3442

3543
send_settings(settings)
3644

‎lib/protocol/http2/settings_frame.rb

Lines changed: 26 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,22 @@ class Settings
2727
:maximum_header_list_size=,
2828
nil,
2929
:enable_connect_protocol=,
30+
:no_rfc7540_priorities=,
3031
]
3132

33+
def initialize
34+
# These limits are taken from the RFC:
35+
# https://tools.ietf.org/html/rfc7540#section-6.5.2
36+
@header_table_size = 4096
37+
@enable_push = 1
38+
@maximum_concurrent_streams = 0xFFFFFFFF
39+
@initial_window_size = 0xFFFF # 2**16 - 1
40+
@maximum_frame_size = 0x4000 # 2**14
41+
@maximum_header_list_size = 0xFFFFFFFF
42+
@enable_connect_protocol = 0
43+
@no_rfc7540_priorities = 0
44+
end
45+
3246
# Allows the sender to inform the remote endpoint of the maximum size of the header compression table used to decode header blocks, in octets.
3347
attr_accessor :header_table_size
3448

@@ -91,16 +105,18 @@ def enable_connect_protocol?
91105
@enable_connect_protocol == 1
92106
end
93107

94-
def initialize
95-
# These limits are taken from the RFC:
96-
# https://tools.ietf.org/html/rfc7540#section-6.5.2
97-
@header_table_size = 4096
98-
@enable_push = 1
99-
@maximum_concurrent_streams = 0xFFFFFFFF
100-
@initial_window_size = 0xFFFF # 2**16 - 1
101-
@maximum_frame_size = 0x4000 # 2**14
102-
@maximum_header_list_size = 0xFFFFFFFF
103-
@enable_connect_protocol = 0
108+
attr :no_rfc7540_priorities
109+
110+
def no_rfc7540_priorities= value
111+
if value == 0 or value == 1
112+
@no_rfc7540_priorities = value
113+
else
114+
raise ProtocolError, "Invalid value for no_rfc7540_priorities: #{value}"
115+
end
116+
end
117+
118+
def no_rfc7540_priorities?
119+
@no_rfc7540_priorities == 1
104120
end
105121

106122
def update(changes)
@@ -110,40 +126,6 @@ def update(changes)
110126
end
111127
end
112128
end
113-
114-
def difference(other)
115-
changes = []
116-
117-
if @header_table_size != other.header_table_size
118-
changes << [HEADER_TABLE_SIZE, @header_table_size]
119-
end
120-
121-
if @enable_push != other.enable_push
122-
changes << [ENABLE_PUSH, @enable_push]
123-
end
124-
125-
if @maximum_concurrent_streams != other.maximum_concurrent_streams
126-
changes << [MAXIMUM_CONCURRENT_STREAMS, @maximum_concurrent_streams]
127-
end
128-
129-
if @initial_window_size != other.initial_window_size
130-
changes << [INITIAL_WINDOW_SIZE, @initial_window_size]
131-
end
132-
133-
if @maximum_frame_size != other.maximum_frame_size
134-
changes << [MAXIMUM_FRAME_SIZE, @maximum_frame_size]
135-
end
136-
137-
if @maximum_header_list_size != other.maximum_header_list_size
138-
changes << [MAXIMUM_HEADER_LIST_SIZE, @maximum_header_list_size]
139-
end
140-
141-
if @enable_connect_protocol != other.enable_connect_protocol
142-
changes << [ENABLE_CONNECT_PROTOCOL, @enable_connect_protocol]
143-
end
144-
145-
return changes
146-
end
147129
end
148130

149131
class PendingSettings

‎test/protocol/http2/client.rb

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434

3535
client_settings_frame = framer.read_frame
3636
expect(client_settings_frame).to be_a Protocol::HTTP2::SettingsFrame
37-
expect(client_settings_frame.unpack).to be == settings
37+
expect(client_settings_frame.unpack).to be == settings + [[Protocol::HTTP2::Settings::NO_RFC7540_PRIORITIES, 1]]
3838

3939
# Fake (empty) server settings:
4040
server_settings_frame = Protocol::HTTP2::SettingsFrame.new
@@ -52,6 +52,27 @@
5252
expect(client.local_settings.header_table_size).to be == 1024
5353
end
5454

55+
it "should fail if the server does not reply with settings frame" do
56+
data_frame = Protocol::HTTP2::DataFrame.new
57+
data_frame.pack("Hello, World!")
58+
59+
expect do
60+
client.send_connection_preface(settings) do
61+
framer.write_frame(data_frame)
62+
end
63+
end.to raise_exception(Protocol::HTTP2::ProtocolError, message: be =~ /First frame must be Protocol::HTTP2::SettingsFrame/)
64+
end
65+
66+
it "should send connection preface with no RFC7540 priorities" do
67+
server_settings_frame = client.send_connection_preface({}) do
68+
client_settings_frame = server.read_connection_preface({})
69+
70+
expect(client_settings_frame.unpack).to be == [[Protocol::HTTP2::Settings::NO_RFC7540_PRIORITIES, 1]]
71+
end
72+
73+
expect(server_settings_frame.unpack).to be == [[Protocol::HTTP2::Settings::NO_RFC7540_PRIORITIES, 1]]
74+
end
75+
5576
it "can generate a stream id" do
5677
id = client.next_stream_id
5778
expect(id).to be == 1

‎test/protocol/http2/priority_update_frame.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,4 +62,4 @@ def before
6262
expect(server_stream.priority).to be == ["u=1", "i"]
6363
end
6464
end
65-
end
65+
end

‎test/protocol/http2/server.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@
7878
# The server immediately sends its own settings frame...
7979
frame = framer.read_frame
8080
expect(frame).to be_a Protocol::HTTP2::SettingsFrame
81-
expect(frame.unpack).to be == server_settings
81+
expect(frame.unpack).to be == server_settings + [[Protocol::HTTP2::Settings::NO_RFC7540_PRIORITIES, 1]]
8282

8383
# And then it acknowledges the client settings:
8484
frame = framer.read_frame

‎test/protocol/http2/settings.rb

Lines changed: 19 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,23 @@
7373
end
7474
end
7575

76+
with "#no_rfc7540_priorities" do
77+
it "is disabled by default" do
78+
expect(settings.no_rfc7540_priorities).to be == 0
79+
expect(settings).not.to be(:no_rfc7540_priorities?)
80+
end
81+
82+
it "can be enabled" do
83+
settings.no_rfc7540_priorities = 1
84+
expect(settings.no_rfc7540_priorities).to be == 1
85+
expect(settings).to be(:no_rfc7540_priorities?)
86+
end
87+
88+
it "only accepts valid values" do
89+
expect{settings.no_rfc7540_priorities = 2}.to raise_exception(Protocol::HTTP2::ProtocolError)
90+
end
91+
end
92+
7693
it "has the expected defaults" do
7794
expect(settings).to have_attributes(
7895
header_table_size: be == 4096,
@@ -81,7 +98,8 @@
8198
initial_window_size: be == 0xFFFF,
8299
maximum_frame_size: be == 0x4000,
83100
maximum_header_list_size: be == 0xFFFFFFFF,
84-
enable_connect_protocol: be == 0
101+
enable_connect_protocol: be == 0,
102+
no_rfc7540_priorities: be == 0,
85103
)
86104
end
87105

@@ -106,29 +124,6 @@
106124
enable_connect_protocol: be == 1
107125
)
108126
end
109-
110-
it "can compute the difference between two settings" do
111-
other = subject.new
112-
other.update({
113-
Protocol::HTTP2::Settings::HEADER_TABLE_SIZE => 100,
114-
Protocol::HTTP2::Settings::ENABLE_PUSH => 0,
115-
Protocol::HTTP2::Settings::MAXIMUM_CONCURRENT_STREAMS => 10,
116-
Protocol::HTTP2::Settings::INITIAL_WINDOW_SIZE => 1000,
117-
Protocol::HTTP2::Settings::MAXIMUM_FRAME_SIZE => 20000,
118-
Protocol::HTTP2::Settings::MAXIMUM_HEADER_LIST_SIZE => 100000,
119-
Protocol::HTTP2::Settings::ENABLE_CONNECT_PROTOCOL => 1
120-
})
121-
122-
expect(other.difference(settings)).to be == [
123-
[Protocol::HTTP2::Settings::HEADER_TABLE_SIZE, 100],
124-
[Protocol::HTTP2::Settings::ENABLE_PUSH, 0],
125-
[Protocol::HTTP2::Settings::MAXIMUM_CONCURRENT_STREAMS, 10],
126-
[Protocol::HTTP2::Settings::INITIAL_WINDOW_SIZE, 1000],
127-
[Protocol::HTTP2::Settings::MAXIMUM_FRAME_SIZE, 20000],
128-
[Protocol::HTTP2::Settings::MAXIMUM_HEADER_LIST_SIZE, 100000],
129-
[Protocol::HTTP2::Settings::ENABLE_CONNECT_PROTOCOL, 1]
130-
]
131-
end
132127
end
133128

134129
describe Protocol::HTTP2::PendingSettings do

0 commit comments

Comments
 (0)