Unverified Commit ac8c9023 authored by Phillippe Siclait's avatar Phillippe Siclait Committed by GitHub
Browse files

fix: Truncate HttpError message to first 8192 characters (#5020)


Signed-off-by: default avatarPhillippe Siclait <546332+siclait@users.noreply.github.com>
parent 88abc787
...@@ -138,6 +138,12 @@ impl HttpAsyncEngine { ...@@ -138,6 +138,12 @@ impl HttpAsyncEngine {
} }
} }
#[derive(FromPyObject)]
struct HttpError {
code: u16,
message: String,
}
#[async_trait] #[async_trait]
impl<Req, Resp> AsyncEngine<SingleIn<Req>, ManyOut<Annotated<Resp>>, Error> for HttpAsyncEngine impl<Req, Resp> AsyncEngine<SingleIn<Req>, ManyOut<Annotated<Resp>>, Error> for HttpAsyncEngine
where where
...@@ -153,18 +159,10 @@ where ...@@ -153,18 +159,10 @@ where
Err(e) => { Err(e) => {
if let Some(py_err) = e.downcast_ref::<PyErr>() { if let Some(py_err) = e.downcast_ref::<PyErr>() {
Python::with_gil(|py| { Python::with_gil(|py| {
let err_val = py_err.clone_ref(py).into_value(py); // With the Stable ABI, we can't subclass Python's built-in exceptions in PyO3, so instead we
let bound_err = err_val.bind(py); // implement the exception in Python and assume that it's an HttpError if the code and message
// are present.
// check: Py03 exceptions cannot be cross-compiled, so we duck-type by name if let Ok(HttpError { code, message }) = py_err.value(py).extract() {
// and fields.
if let Ok(type_name) = bound_err.get_type().name()
&& type_name.to_string().contains("HttpError")
&& let (Ok(code), Ok(message)) =
(bound_err.getattr("code"), bound_err.getattr("message"))
&& let (Ok(code), Ok(message)) =
(code.extract::<u16>(), message.extract::<String>())
{
// SSE panics if there are carriage returns or newlines // SSE panics if there are carriage returns or newlines
let message = message.replace(['\r', '\n'], ""); let message = message.replace(['\r', '\n'], "");
return Err(http_error::HttpError { code, message })?; return Err(http_error::HttpError { code, message })?;
......
...@@ -3,13 +3,33 @@ ...@@ -3,13 +3,33 @@
# flake8: noqa # flake8: noqa
import logging
logger = logging.getLogger(__name__)
_MAX_MESSAGE_LENGTH = 8192
class HttpError(Exception): class HttpError(Exception):
def __init__(self, code: int, message: str): def __init__(self, code: int, message: str):
if not (isinstance(code, int) and 0 <= code < 600): # These ValueErrors are easier to trace to here than the TypeErrors that
# would be raised otherwise.
if not isinstance(code, int) or isinstance(code, bool):
raise ValueError("HttpError status code must be an integer")
if not isinstance(message, str):
raise ValueError("HttpError message must be a string")
if not (0 <= code < 600):
raise ValueError("HTTP status code must be an integer between 0 and 599") raise ValueError("HTTP status code must be an integer between 0 and 599")
if not (isinstance(message, str) and 0 < len(message) <= 8192):
raise ValueError("HTTP error message must be a string of length <= 8192") if len(message) > _MAX_MESSAGE_LENGTH:
logger.warning(
f"HttpError message length {len(message)} exceeds max length {_MAX_MESSAGE_LENGTH}, truncating..."
)
message = message[: (_MAX_MESSAGE_LENGTH - 3)] + "..."
self.code = code self.code = code
self.message = message self.message = message
super().__init__(f"HTTP {code}: {message}") super().__init__(f"HTTP {code}: {message}")
...@@ -18,3 +18,19 @@ def test_raise_http_error(): ...@@ -18,3 +18,19 @@ def test_raise_http_error():
def test_invalid_http_error_code(): def test_invalid_http_error_code():
with pytest.raises(ValueError): with pytest.raises(ValueError):
HttpError(1700, "Invalid Code") HttpError(1700, "Invalid Code")
def test_invalid_http_error_message():
with pytest.raises(ValueError):
# The second argument must be a string, not bytes.
HttpError(400, b"Bad Request")
def test_long_http_error_message():
message = ("A" * 8192) + "B"
error = HttpError(400, message)
assert len(error.message) == 8192
# Ensure the exception string uses the truncated message too.
assert message[:8189] in str(error)
assert "B" not in str(error)
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment