Skip to content

Commit 177dc72

Browse files
committed
Refactor to address code review comments
1 parent c80e8b3 commit 177dc72

File tree

3 files changed

+101
-99
lines changed

3 files changed

+101
-99
lines changed

cors/cors-safelisted-request-header.any.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,4 +77,4 @@ function safelist(headers, expectPreflight = false) {
7777
["012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678", true],
7878
].forEach(([value, preflight = false]) => {
7979
safelist({"last-event-id": value}, preflight);
80-
});
80+
});
Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,35 @@
11
// META: title=EventSource: cross-origin preflight
22
// META: script=/common/utils.js
33

4-
const crossdomain = location.href.replace('://', '://élève.').replace(/\/[^\/]*$/, '/')
5-
const origin = location.origin.replace('://', '://xn--lve-6lad.')
4+
const crossdomain = location.href.replace('://', '://élève.').replace(/\/[^\/]*$/, '/');
5+
const origin = location.origin.replace('://', '://xn--lve-6lad.');
66

7-
;[
7+
[
88
['safe `last-event-id` (no preflight)', 'safe'],
99
['unsafe `last-event-id` (too long)', 'long'],
1010
['unsafe `last-event-id` (unsafe characters)', 'unsafe']
1111
].forEach(([name, fixture]) => {
12-
async_test(document.title + ' - ' + name).step(function() {
13-
const uuid = token()
14-
const url = crossdomain + 'resources/cors-unsafe-last-event-id.py?fixture=' + fixture + '&token=' + uuid
12+
async_test(document.title + ' - ' + name).step(function () {
13+
const uuid = token();
14+
const url = crossdomain + 'resources/cors-unsafe-last-event-id.py?fixture=' + fixture + '&token=' + uuid;
1515

16-
const source = new EventSource(url)
16+
const source = new EventSource(url);
1717

1818
// Make sure to close the EventSource after the test is done.
19-
this.add_cleanup(() => source.close())
19+
this.add_cleanup(() => source.close());
2020

2121
// 1. Event will be a `message` with `id` set to a CORS-safe value, then disconnects.
22-
source.addEventListener('message', this.step_func(e => assert_equals(e.data, fixture)))
22+
source.addEventListener(
23+
'message',
24+
this.step_func((evt) => assert_equals(evt.data, fixture)),
25+
);
2326

2427
// 2. Will emit either `success` or `failure` event. We expect `success`,
2528
// which is the case if `last-event-id` is set to the same value as received above,
2629
// and a preflight request has been sent for the unsafe `last-event-id` headers.
27-
source.addEventListener('success', this.step_func_done())
28-
source.addEventListener('failure', (evt) => {
29-
this.step(() => assert_unreached(evt.data))
30-
this.done()
31-
})
32-
})
33-
})
30+
source.addEventListener('success', this.step_func_done());
31+
source.addEventListener('failure', this.step_func_done((evt) => {
32+
assert_unreached(evt.data);
33+
}));
34+
});
35+
});

eventsource/resources/cors-unsafe-last-event-id.py

Lines changed: 82 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -10,87 +10,87 @@
1010
unsafe_id_value = b"e5p3n<3k0k0s"
1111

1212
def main(request, response):
13-
origin = request.headers.get(b"Origin")
14-
cors_request_headers = request.headers.get(b"Access-Control-Request-Headers")
15-
16-
# Allow any CORS origin
17-
if origin is not None:
18-
response.headers.set(b"Access-Control-Allow-Origin", origin)
19-
20-
# Allow any CORS request headers
21-
if cors_request_headers is not None:
22-
response.headers.set(b"Access-Control-Allow-Headers", cors_request_headers)
23-
24-
# Expect a `token` in the query string
25-
if b"token" not in request.GET:
26-
headers = [(b"Content-Type", b"text/plain")]
27-
return 400, headers, b"ERROR: `token` query parameter!"
28-
29-
# Expect a `fixture` in the query string
30-
if b"fixture" not in request.GET:
31-
headers = [(b"Content-Type", b"text/plain")]
32-
return 400, headers, b"ERROR: `fixture` query parameter!"
33-
34-
# Prepare state
35-
fixture = request.GET.first(b"fixture")
36-
token = request.GET.first(b"token")
37-
last_event_id = request.headers.get(b"Last-Event-ID", b"")
38-
expect_preflight = fixture == b"unsafe" or fixture == b"long"
39-
40-
# Preflight handling
41-
if request.method == u"OPTIONS":
42-
# The first request (without any `Last-Event-ID` header) should _never_ be a
43-
# preflight request, since it should be considered a "safe" request.
44-
# If we _do_ send a preflight for these requests, error early.
13+
origin = request.headers.get(b"Origin")
14+
cors_request_headers = request.headers.get(b"Access-Control-Request-Headers")
15+
16+
# Allow any CORS origin
17+
if origin is not None:
18+
response.headers.set(b"Access-Control-Allow-Origin", origin)
19+
20+
# Allow any CORS request headers
21+
if cors_request_headers is not None:
22+
response.headers.set(b"Access-Control-Allow-Headers", cors_request_headers)
23+
24+
# Expect a `token` in the query string
25+
if b"token" not in request.GET:
26+
headers = [(b"Content-Type", b"text/plain")]
27+
return 400, headers, b"ERROR: `token` query parameter!"
28+
29+
# Expect a `fixture` in the query string
30+
if b"fixture" not in request.GET:
31+
headers = [(b"Content-Type", b"text/plain")]
32+
return 400, headers, b"ERROR: `fixture` query parameter!"
33+
34+
# Prepare state
35+
fixture = request.GET.first(b"fixture")
36+
token = request.GET.first(b"token")
37+
last_event_id = request.headers.get(b"Last-Event-ID", b"")
38+
expect_preflight = fixture == b"unsafe" or fixture == b"long"
39+
40+
# Preflight handling
41+
if request.method == u"OPTIONS":
42+
# The first request (without any `Last-Event-ID` header) should _never_ be a
43+
# preflight request, since it should be considered a "safe" request.
44+
# If we _do_ send a preflight for these requests, error early.
45+
if last_event_id == b"":
46+
headers = [(b"Content-Type", b"text/plain")]
47+
return 400, headers, b"ERROR: No Last-Event-ID header in preflight!"
48+
49+
# We keep track of the different "tokens" we see, in order to tell whether or not
50+
# a client has done a preflight request. If the "stash" does not contain a token,
51+
# no preflight request was made.
52+
request.server.stash.put(token, cors_request_headers)
53+
54+
# We can return with an empty body on preflight requests
55+
return b""
56+
57+
# This will be a SSE endpoint
58+
response.headers.set(b"Content-Type", b"text/event-stream")
59+
response.headers.set(b"Cache-Control", b"no-store")
60+
61+
# If we do not have a `Last-Event-ID` header, we're on the initial request
62+
# Respond with the fixture corresponding to the `fixture` query parameter
4563
if last_event_id == b"":
46-
headers = [(b"Content-Type", b"text/plain")]
47-
return 400, headers, b"ERROR: No Last-Event-ID header in preflight!"
48-
49-
# We keep track of the different "tokens" we see, in order to tell whether or not
50-
# a client has done a preflight request. If the "stash" does not contain a token,
51-
# no preflight request was made.
52-
request.server.stash.put(token, cors_request_headers)
53-
54-
# We can return with an empty body on preflight requests
55-
return b""
56-
57-
# This will be a SSE endpoint
58-
response.headers.set(b"Content-Type", b"text/event-stream")
59-
response.headers.set(b"Cache-Control", b"no-store")
60-
61-
# If we do not have a `Last-Event-ID` header, we're on the initial request
62-
# Respond with the fixture corresponding to the `fixture` query parameter
63-
if last_event_id == b"":
64+
if fixture == b"safe":
65+
return b"id: " + safe_id_value + b"\nretry: 200\ndata: safe\n\n"
66+
if fixture == b"unsafe":
67+
return b"id: " + unsafe_id_value + b"\nretry: 200\ndata: unsafe\n\n"
68+
if fixture == b"long":
69+
return b"id: " + long_string + b"\nretry: 200\ndata: long\n\n"
70+
return b"event: failure\ndata: unknown fixture\n\n"
71+
72+
# If we have a `Last-Event-ID` header, we're on a reconnect.
73+
# If fixture is "unsafe", eg requires a preflight, check to see that we got one.
74+
preflight_headers = request.server.stash.take(token)
75+
saw_preflight = preflight_headers is not None
76+
if saw_preflight and not expect_preflight:
77+
return b"event: failure\ndata: saw preflight, did not expect one\n\n"
78+
elif not saw_preflight and expect_preflight:
79+
return b"event: failure\ndata: expected preflight, did not get one\n\n"
80+
81+
if saw_preflight and preflight_headers.lower() != b"last-event-id":
82+
data = b"preflight `access-control-request-headers` was not `last-event-id`"
83+
return b"event: failure\ndata: " + data + b"\n\n"
84+
85+
# Expect to have the same ID in the header as the one we sent.
86+
expected = b"<unknown>"
6487
if fixture == b"safe":
65-
return b"id: " + safe_id_value + b"\nretry: 200\ndata: safe\n\n"
66-
if fixture == b"unsafe":
67-
return b"id: " + unsafe_id_value + b"\nretry: 200\ndata: unsafe\n\n"
68-
if fixture == b"long":
69-
return b"id: " + long_string + b"\nretry: 200\ndata: long\n\n"
70-
return b"event: failure\ndata: unknown fixture\n\n"
71-
72-
# If we have a `Last-Event-ID` header, we're on a reconnect.
73-
# If fixture is "unsafe", eg requires a preflight, check to see that we got one.
74-
preflight_headers = request.server.stash.take(token)
75-
saw_preflight = preflight_headers is not None
76-
if saw_preflight and not expect_preflight:
77-
return b"event: failure\ndata: saw preflight, did not expect one\n\n"
78-
elif not saw_preflight and expect_preflight:
79-
return b"event: failure\ndata: expected preflight, did not get one\n\n"
80-
81-
if saw_preflight and preflight_headers.lower() != b"last-event-id":
82-
data = b"preflight `access-control-request-headers` was not `last-event-id`"
83-
return b"event: failure\ndata: " + data + b"\n\n"
84-
85-
# Expect to have the same ID in the header as the one we sent.
86-
expected = b"<unknown>"
87-
if fixture == b"safe":
88-
expected = safe_id_value
89-
elif fixture == b"unsafe":
90-
expected = unsafe_id_value
91-
elif fixture == b"long":
92-
expected = long_string
93-
94-
event = last_event_id == expected and b"success" or b"failure"
95-
data = b"got " + last_event_id + b", expected " + expected
96-
return b"event: " + event + b"\ndata: " + data + b"\n\n"
88+
expected = safe_id_value
89+
elif fixture == b"unsafe":
90+
expected = unsafe_id_value
91+
elif fixture == b"long":
92+
expected = long_string
93+
94+
event = last_event_id == expected and b"success" or b"failure"
95+
data = b"got " + last_event_id + b", expected " + expected
96+
return b"event: " + event + b"\ndata: " + data + b"\n\n"

0 commit comments

Comments
 (0)