Skip to content

Commit 316b060

Browse files
Fix POST request body not consumed when returning cached response
When a POST request is made to /patch/<base_rev>/<patch_hash>/schedules with cached data, the response was being sent before consuming the request body. This caused clients to fail with IncompleteRead errors because they were still sending the body while receiving the response. The fix consumes the request body (by accessing request.data) before returning the cached response for POST requests. Added comprehensive tests to verify the fix handles both POST and GET requests correctly with cached results. Co-authored-by: suhaibmujahid <[email protected]>
1 parent 91af0c8 commit 316b060

File tree

2 files changed

+125
-0
lines changed

2 files changed

+125
-0
lines changed

http_service/bugbug_http/app.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1087,6 +1087,10 @@ def patch_schedules(base_rev, patch_hash):
10871087
job = JobInfo(schedule_tests_from_patch, base_rev, patch_hash)
10881088
data = get_result(job)
10891089
if data:
1090+
# Consume the request body for POST requests before returning the cached response
1091+
# to avoid the client receiving a response before finishing sending the body
1092+
if request.method == "POST":
1093+
_ = request.data
10901094
return compress_response(data, 200)
10911095

10921096
if not is_pending(job):
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
# -*- coding: utf-8 -*-
2+
# This Source Code Form is subject to the terms of the Mozilla Public
3+
# License, v. 2.0. If a copy of the MPL was not distributed with this file,
4+
# You can obtain one at http://mozilla.org/MPL/2.0/.
5+
6+
import gzip
7+
8+
import orjson
9+
10+
from bugbug_http.app import API_TOKEN
11+
12+
13+
def retrieve_compressed_reponse(response):
14+
# Response is of type "<class 'flask.wrappers.Response'>" - Flask Client's Response
15+
# Not applicable for "<class 'requests.models.Response'> "
16+
if response.headers["Content-Encoding"] == "gzip":
17+
return orjson.loads(gzip.decompress(response.data))
18+
return response.json
19+
20+
21+
def test_patch_schedules_post_with_cache(client, add_result, jobs):
22+
"""Test that POST requests with cached results still work correctly.
23+
24+
This test verifies the fix for the issue where sending a POST request
25+
with a patch body to a previously cached endpoint would fail with
26+
IncompleteRead error because the response was sent before consuming
27+
the request body.
28+
"""
29+
base_rev = "abc123"
30+
patch_hash = "def456"
31+
patch_content = """diff --git a/test.txt b/test.txt
32+
--- a/test.txt
33+
+++ b/test.txt
34+
@@ -1,1 +1,2 @@
35+
line 1
36+
+line 2
37+
"""
38+
39+
# First POST request - should queue the job
40+
rv = client.post(
41+
f"/patch/{base_rev}/{patch_hash}/schedules",
42+
data=patch_content.encode("utf-8"),
43+
headers={API_TOKEN: "test"},
44+
)
45+
46+
assert rv.status_code == 202
47+
assert rv.json == {"ready": False}
48+
49+
# Simulate job completion with cached result
50+
result = {
51+
"groups": ["foo/mochitest.ini"],
52+
"tasks": ["test-linux/opt-mochitest-1"],
53+
}
54+
keys = next(iter(jobs.values()))
55+
add_result(keys[0], result)
56+
57+
# Second POST request with same parameters - should return cached result
58+
# This is where the bug would occur - sending response before reading body
59+
rv = client.post(
60+
f"/patch/{base_rev}/{patch_hash}/schedules",
61+
data=patch_content.encode("utf-8"),
62+
headers={API_TOKEN: "test"},
63+
)
64+
65+
assert rv.status_code == 200
66+
assert retrieve_compressed_reponse(rv) == result
67+
68+
69+
def test_patch_schedules_get_with_cache(client, add_result, jobs):
70+
"""Test that GET requests work correctly with cached results."""
71+
base_rev = "abc123"
72+
patch_hash = "def456"
73+
patch_content = """diff --git a/test.txt b/test.txt
74+
--- a/test.txt
75+
+++ b/test.txt
76+
@@ -1,1 +1,2 @@
77+
line 1
78+
+line 2
79+
"""
80+
81+
# First POST request to submit the patch
82+
rv = client.post(
83+
f"/patch/{base_rev}/{patch_hash}/schedules",
84+
data=patch_content.encode("utf-8"),
85+
headers={API_TOKEN: "test"},
86+
)
87+
88+
assert rv.status_code == 202
89+
assert rv.json == {"ready": False}
90+
91+
# Simulate job completion with cached result
92+
result = {
93+
"groups": ["foo/mochitest.ini"],
94+
"tasks": ["test-linux/opt-mochitest-1"],
95+
}
96+
keys = next(iter(jobs.values()))
97+
add_result(keys[0], result)
98+
99+
# GET request should return cached result
100+
rv = client.get(
101+
f"/patch/{base_rev}/{patch_hash}/schedules",
102+
headers={API_TOKEN: "test"},
103+
)
104+
105+
assert rv.status_code == 200
106+
assert retrieve_compressed_reponse(rv) == result
107+
108+
109+
def test_patch_schedules_get_without_cache(client):
110+
"""Test that GET requests without cache return 404."""
111+
base_rev = "abc123"
112+
patch_hash = "def456"
113+
114+
# GET request without submitting patch should return 404
115+
rv = client.get(
116+
f"/patch/{base_rev}/{patch_hash}/schedules",
117+
headers={API_TOKEN: "test"},
118+
)
119+
120+
assert rv.status_code == 404
121+
assert rv.json == {"error": "Patch not submitted yet"}

0 commit comments

Comments
 (0)