Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

Commit 4244b9b

Browse files
author
David Robertson
committed
Improve type hinting of RawHeaders
to detect the problem that #14301 fixed.
1 parent 8756d5c commit 4244b9b

File tree

2 files changed

+23
-8
lines changed

2 files changed

+23
-8
lines changed

synapse/app/generic_worker.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -178,14 +178,14 @@ async def on_POST(
178178
# Proxy headers from the original request, such as the auth headers
179179
# (in case the access token is there) and the original IP /
180180
# User-Agent of the request.
181-
headers = {
182-
header: request.requestHeaders.getRawHeaders(header, [])
181+
headers: Dict[bytes, List[bytes]] = {
182+
header: list(request.requestHeaders.getRawHeaders(header, []))
183183
for header in (b"Authorization", b"User-Agent")
184184
}
185185
# Add the previous hop to the X-Forwarded-For header.
186-
x_forwarded_for = request.requestHeaders.getRawHeaders(
186+
x_forwarded_for = list(request.requestHeaders.getRawHeaders(
187187
b"X-Forwarded-For", []
188-
)
188+
))
189189
# we use request.client here, since we want the previous hop, not the
190190
# original client (as returned by request.getClientAddress()).
191191
if isinstance(request.client, (address.IPv4Address, address.IPv6Address)):

synapse/http/client.py

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,14 +90,29 @@
9090
"synapse_http_client_responses", "", ["method", "code"]
9191
)
9292

93-
# the type of the headers list, to be passed to the t.w.h.Headers.
94-
# Actually we can mix str and bytes keys, but Mapping treats 'key' as invariant so
95-
# we simplify.
93+
# the type of the headers map, to be passed to the t.w.h.Headers.
94+
#
95+
# The actual type accepted by Twisted is
96+
# Mapping[Union[str, bytes], Sequence[Union[str, bytes]] ,
97+
# allowing us to mix and match str and bytes freely. However: any str is also a
98+
# Sequence[str]; passing a header string value which is a
99+
# standalone str is interpreted as a sequence of 1-codepoint strings. This is a disastrous footgun.
100+
# We use a narrower value type (RawHeaderValue) to avoid this footgun.
101+
#
102+
# We also simplify the keys to be either all str or all bytes. This helps because
103+
# Dict[K, V] is invariant in K (and indeed V).
96104
RawHeaders = Union[Mapping[str, "RawHeaderValue"], Mapping[bytes, "RawHeaderValue"]]
97105

98106
# the value actually has to be a List, but List is invariant so we can't specify that
99107
# the entries can either be Lists or bytes.
100-
RawHeaderValue = Sequence[Union[str, bytes]]
108+
RawHeaderValue = Union[
109+
List[str],
110+
List[bytes],
111+
List[Union[str, bytes]],
112+
Tuple[str, ...],
113+
Tuple[bytes, ...],
114+
Tuple[Union[str, bytes], ...],
115+
]
101116

102117

103118
def check_against_blacklist(

0 commit comments

Comments
 (0)