Skip to content
This repository has been archived by the owner on Jan 17, 2023. It is now read-only.

Commit

Permalink
Fix #3204, give proper response code for request entity too large (#3248
Browse files Browse the repository at this point in the history
)

* Fix #3204, give proper response code for request entity too large

* Fix #3204, give 400 Bad Request when id is invalid
  • Loading branch information
ianb authored and jaredhirsch committed Aug 2, 2017
1 parent d12066c commit 4d53220
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 5 deletions.
23 changes: 21 additions & 2 deletions server/src/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,13 @@ app.param("id", function(req, res, next, id) {
next();
return;
}
next(new Error("invalid id"));
let exc = new Error("invalid id")
exc.isAppError = true;
exc.output = {
statusCode: 400,
payload: "Invalid id"
};
next(exc);
});

app.param("domain", function(req, res, next, domain) {
Expand Down Expand Up @@ -1087,10 +1093,23 @@ app.use(function(err, req, res, next) {
if (err.isAppError) {
let { statusCode, headers, payload } = err.output;
res.status(statusCode);
res.header(headers);
if (headers) {
res.header(headers);
}
res.send(payload);
return;
}
if (err.type === "entity.too.large") {
mozlog.info("entity-too-large", {
length: err.length,
limit: err.limit,
expected: err.expected
});
res.status(err.statusCode);
res.type("text");
res.send(res.message);
return;
}
errorResponse(res, "General error:", err);
});

Expand Down
12 changes: 9 additions & 3 deletions test/server/clientlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,12 @@ def login(self):
resp.raise_for_status()

def delete_account(self):
page = self.session.get(self.backend + "/leave-screenshots/").text
csrf_match = re.search(r'<input.*name="_csrf".*value="([^"]*)"', page)
csrf = csrf_match.group(1)
resp = self.session.post(
urljoin(self.backend, "/leave-screenshots/leave"),
json={})
json={"_csrf": csrf})
resp.raise_for_status()

def create_shot(self, shot_id=None, **example_args):
Expand Down Expand Up @@ -72,12 +75,15 @@ def search_shots(self, q):
resp.raise_for_status()


def make_example_shot(deviceId, **overrides):
def make_example_shot(deviceId, pad_image_to_length=None, **overrides):
image = random.choice(example_images)
text = []
for i in range(10):
text.append(random.choice(text_strings))
text = " ".join(text)
image_url = image["url"]
if pad_image_to_length:
image_url = image_url + "A" * (pad_image_to_length - len(image_url))
return dict(
deviceId=deviceId,
url="http://test.com/?" + make_uuid(),
Expand All @@ -90,7 +96,7 @@ def make_example_shot(deviceId, **overrides):
createdDate=int(time.time() * 1000),
sortOrder=100,
image=dict(
url=image["url"],
url=image_url,
captureType="selection",
text=text,
location=dict(
Expand Down
39 changes: 39 additions & 0 deletions test/server/test_responses.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
#!../../.venv/bin/python

from clientlib import ScreenshotsClient
import random
from requests import HTTPError

# Hack to make this predictable:
random.seed(0)


def test_put_large_image():
user = ScreenshotsClient()
user.login()
try:
try:
user.create_shot(pad_image_to_length=100 * 1000 * 1000)
except HTTPError, e:
if e.response.status_code != 413:
raise
finally:
user.delete_account()


def test_bad_id():
user = ScreenshotsClient()
user.login()
try:
try:
user.create_shot(shot_id="!!!/test.com")
except HTTPError, e:
if e.response.status_code != 400:
raise
finally:
user.delete_account()


if __name__ == "__main__":
test_put_large_image()
test_bad_id()

0 comments on commit 4d53220

Please sign in to comment.