Skip to content

Commit d2f9299

Browse files
committed
Prevent invalid window bit sizes
This is specifically to deal with the case whereby the window size is set to 8. As zlib dropped support for 256 (8 bit) window sizes in 1.2.9, https://www.zlib.net/ChangeLog.txt, onwards. 8 bit windows were broken before as well, madler/zlib#171. As autobahn tests 8 bit windows the tox test would fail based on the zlib version installed - hence the relevant autobahn tests are also disabled. 8 bit windows are rarely used in practice, so this should be ok practically. If not zlib needs to be fixed, see madler/zlib#87 (comment)
1 parent 9fe1b84 commit d2f9299

File tree

3 files changed

+42
-22
lines changed

3 files changed

+42
-22
lines changed

compliance/run-autobahn-tests.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
"outdir": "./reports/clients",
3636
"webport": 8080,
3737
"cases": ["*"],
38-
"exclude-cases": [],
38+
"exclude-cases": ["13.3.*", "13.5.*"],
3939
"exclude-agent-cases": {},
4040
}
4141

src/wsproto/extensions.py

+39-19
Original file line numberDiff line numberDiff line change
@@ -71,13 +71,13 @@ def __init__(
7171
server_max_window_bits: Optional[int] = None,
7272
) -> None:
7373
self.client_no_context_takeover = client_no_context_takeover
74-
if client_max_window_bits is None:
75-
client_max_window_bits = self.DEFAULT_CLIENT_MAX_WINDOW_BITS
76-
self.client_max_window_bits = client_max_window_bits
7774
self.server_no_context_takeover = server_no_context_takeover
78-
if server_max_window_bits is None:
79-
server_max_window_bits = self.DEFAULT_SERVER_MAX_WINDOW_BITS
80-
self.server_max_window_bits = server_max_window_bits
75+
self._client_max_window_bits = self.DEFAULT_CLIENT_MAX_WINDOW_BITS
76+
self._server_max_window_bits = self.DEFAULT_SERVER_MAX_WINDOW_BITS
77+
if client_max_window_bits is not None:
78+
self.client_max_window_bits = client_max_window_bits
79+
if server_max_window_bits is not None:
80+
self.server_max_window_bits = server_max_window_bits
8181

8282
self._compressor: Optional[zlib._Compress] = None # noqa
8383
self._decompressor: Optional[zlib._Decompress] = None # noqa
@@ -90,6 +90,26 @@ def __init__(
9090

9191
self._enabled = False
9292

93+
@property
94+
def client_max_window_bits(self) -> int:
95+
return self._client_max_window_bits
96+
97+
@client_max_window_bits.setter
98+
def client_max_window_bits(self, value: int) -> None:
99+
if value < 9 or value > 15:
100+
raise ValueError("Window size must be between 9 and 15 inclusive")
101+
self._client_max_window_bits = value
102+
103+
@property
104+
def server_max_window_bits(self) -> int:
105+
return self._server_max_window_bits
106+
107+
@server_max_window_bits.setter
108+
def server_max_window_bits(self, value: int) -> None:
109+
if value < 9 or value > 15:
110+
raise ValueError("Window size must be between 9 and 15 inclusive")
111+
self._server_max_window_bits = value
112+
93113
def _compressible_opcode(self, opcode: Opcode) -> bool:
94114
return opcode in (Opcode.TEXT, Opcode.BINARY, Opcode.CONTINUATION)
95115

@@ -127,7 +147,6 @@ def _parse_params(self, params: str) -> Tuple[Optional[int], Optional[int]]:
127147
client_max_window_bits = None
128148
server_max_window_bits = None
129149

130-
print(params)
131150
bits = [b.strip() for b in params.split(";")]
132151
for bit in bits[1:]:
133152
if bit.startswith("client_no_context_takeover"):
@@ -147,25 +166,27 @@ def _parse_params(self, params: str) -> Tuple[Optional[int], Optional[int]]:
147166

148167
return client_max_window_bits, server_max_window_bits
149168

150-
def accept(self, offer: str) -> Union[bool, str]:
169+
def accept(self, offer: str) -> Union[bool, None, str]:
151170
client_max_window_bits, server_max_window_bits = self._parse_params(offer)
152171

153-
self._enabled = True
154-
155172
parameters = []
156173

157174
if self.client_no_context_takeover:
158175
parameters.append("client_no_context_takeover")
159-
if client_max_window_bits is not None:
160-
parameters.append("client_max_window_bits=%d" % client_max_window_bits)
161-
self.client_max_window_bits = client_max_window_bits
162176
if self.server_no_context_takeover:
163177
parameters.append("server_no_context_takeover")
164-
if server_max_window_bits is not None:
165-
parameters.append("server_max_window_bits=%d" % server_max_window_bits)
166-
self.server_max_window_bits = server_max_window_bits
167-
168-
return "; ".join(parameters)
178+
try:
179+
if client_max_window_bits is not None:
180+
parameters.append("client_max_window_bits=%d" % client_max_window_bits)
181+
self.client_max_window_bits = client_max_window_bits
182+
if server_max_window_bits is not None:
183+
parameters.append("server_max_window_bits=%d" % server_max_window_bits)
184+
self.server_max_window_bits = server_max_window_bits
185+
except ValueError:
186+
return None
187+
else:
188+
self._enabled = True
189+
return "; ".join(parameters)
169190

170191
def frame_inbound_header(
171192
self,
@@ -257,7 +278,6 @@ def frame_outbound(
257278
bits = self.client_max_window_bits
258279
else:
259280
bits = self.server_max_window_bits
260-
print(bits, self.client_max_window_bits, self.server_max_window_bits)
261281
self._compressor = zlib.compressobj(
262282
zlib.Z_DEFAULT_COMPRESSION, zlib.DEFLATED, -int(bits)
263283
)

test/test_permessage_deflate.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,12 @@ class TestPerMessageDeflate:
4545
},
4646
{
4747
"client_no_context_takeover": True,
48-
"client_max_window_bits": 8,
48+
"client_max_window_bits": 9,
4949
"server_no_context_takeover": True,
5050
"server_max_window_bits": 9,
5151
},
5252
{"client_no_context_takeover": True, "server_max_window_bits": 9},
53-
{"server_no_context_takeover": True, "client_max_window_bits": 8},
53+
{"server_no_context_takeover": True, "client_max_window_bits": 9},
5454
{"client_max_window_bits": None, "server_max_window_bits": None},
5555
{},
5656
]

0 commit comments

Comments
 (0)