Skip to content

Commit b329274

Browse files
jplattedavidpdrsnyanns
committed
Fix IntoResponse for tuples overriding error response codes
Co-authored-by: David Pedersen <david.pdrsn@gmail.com> Co-authored-by: Yann Simon <yann.simon@commercetools.com>
1 parent 81e727f commit b329274

File tree

10 files changed

+479
-38
lines changed

10 files changed

+479
-38
lines changed

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎axum-core/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ axum = { path = "../axum", features = ["__private"] }
5555
axum-extra = { path = "../axum-extra", features = ["typed-header"] }
5656
axum-macros = { path = "../axum-macros", features = ["__private"] }
5757
hyper = "1.0.0"
58+
serde = { version = "1.0.200", features = ["derive"] }
5859
tokio = { version = "1.25.0", features = ["macros"] }
5960
tower-http = { version = "0.6.0", features = ["limit"] }
6061

‎axum-core/src/response/into_response.rs‎

Lines changed: 57 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use super::{IntoResponseParts, Response, ResponseParts};
1+
use super::{ForceStatusCode, IntoResponseFailed, IntoResponseParts, Response, ResponseParts};
22
use crate::{body::Body, BoxError};
33
use bytes::{buf::Chain, Buf, Bytes, BytesMut};
44
use http::{
@@ -329,7 +329,9 @@ where
329329
{
330330
fn into_response(self) -> Response {
331331
let mut res = self.1.into_response();
332-
*res.status_mut() = self.0;
332+
if res.extensions().get::<IntoResponseFailed>().is_none() {
333+
*res.status_mut() = self.0;
334+
}
333335
res
334336
}
335337
}
@@ -405,18 +407,16 @@ macro_rules! impl_into_response {
405407
let ($($ty),*, res) = self;
406408

407409
let res = res.into_response();
408-
let parts = ResponseParts { res };
409-
410-
$(
411-
let parts = match $ty.into_response_parts(parts) {
410+
if res.extensions().get::<IntoResponseFailed>().is_none() {
411+
let parts = ResponseParts { res };
412+
let parts = match ($($ty,)*).into_response_parts(parts) {
412413
Ok(parts) => parts,
413-
Err(err) => {
414-
return err.into_response();
415-
}
414+
Err(err) => return err.into_response(),
416415
};
417-
)*
418-
419-
parts.res
416+
parts.res
417+
} else {
418+
res
419+
}
420420
}
421421
}
422422

@@ -430,16 +430,40 @@ macro_rules! impl_into_response {
430430
let (status, $($ty),*, res) = self;
431431

432432
let res = res.into_response();
433-
let parts = ResponseParts { res };
434-
435-
$(
436-
let parts = match $ty.into_response_parts(parts) {
433+
if res.extensions().get::<IntoResponseFailed>().is_none() {
434+
let parts = ResponseParts { res };
435+
let mut parts = match ($($ty,)*).into_response_parts(parts) {
437436
Ok(parts) => parts,
438-
Err(err) => {
439-
return err.into_response();
440-
}
437+
Err(err) => return err.into_response(),
441438
};
442-
)*
439+
440+
// Don't call `(status, parts.res).into_response()` since that checks for
441+
// `IntoResponseFailed` and skips setting the status. We've already done that
442+
// check here so overriding the status is required if returning
443+
// `(IntoResponseFailed, StatusCode::INTERNAL_SERVER_ERROR)`
444+
*parts.res.status_mut() = status;
445+
parts.res
446+
} else {
447+
res
448+
}
449+
}
450+
}
451+
452+
#[allow(non_snake_case)]
453+
impl<R, $($ty,)*> IntoResponse for (ForceStatusCode, $($ty),*, R)
454+
where
455+
$( $ty: IntoResponseParts, )*
456+
R: IntoResponse,
457+
{
458+
fn into_response(self) -> Response {
459+
let (status, $($ty),*, res) = self;
460+
461+
let res = res.into_response();
462+
let parts = ResponseParts { res };
463+
let parts = match ($($ty,)*).into_response_parts(parts) {
464+
Ok(parts) => parts,
465+
Err(err) => return err.into_response(),
466+
};
443467

444468
(status, parts.res).into_response()
445469
}
@@ -455,17 +479,22 @@ macro_rules! impl_into_response {
455479
let (outer_parts, $($ty),*, res) = self;
456480

457481
let res = res.into_response();
458-
let parts = ResponseParts { res };
459-
$(
460-
let parts = match $ty.into_response_parts(parts) {
482+
if res.extensions().get::<IntoResponseFailed>().is_none() {
483+
let parts = ResponseParts { res };
484+
let mut parts = match ($($ty,)*).into_response_parts(parts) {
461485
Ok(parts) => parts,
462-
Err(err) => {
463-
return err.into_response();
464-
}
486+
Err(err) => return err.into_response(),
465487
};
466-
)*
467488

468-
(outer_parts, parts.res).into_response()
489+
// Don't call `(outer_parts, parts.res).into_response()` for the same reason we
490+
// don't call `(status, parts.res).into_response()` in the above impl.
491+
*parts.res.status_mut() = outer_parts.status;
492+
parts.res.headers_mut().extend(outer_parts.headers);
493+
parts.res.extensions_mut().extend(outer_parts.extensions);
494+
parts.res
495+
} else {
496+
res
497+
}
469498
}
470499
}
471500

‎axum-core/src/response/into_response_parts.rs‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,9 @@ macro_rules! impl_into_response_parts {
241241
let res = match $ty.into_response_parts(res) {
242242
Ok(res) => res,
243243
Err(err) => {
244-
return Err(err.into_response());
244+
let mut err_res = err.into_response();
245+
err_res.extensions_mut().insert(super::IntoResponseFailed);
246+
return Err(err_res);
245247
}
246248
};
247249
)*
@@ -270,3 +272,19 @@ impl IntoResponseParts for () {
270272
Ok(res)
271273
}
272274
}
275+
276+
#[cfg(test)]
277+
mod tests {
278+
use http::StatusCode;
279+
280+
use crate::response::IntoResponse;
281+
282+
#[test]
283+
fn failed_into_response_parts() {
284+
let response = (StatusCode::CREATED, [("\n", "\n")]).into_response();
285+
assert_eq!(response.status(), StatusCode::INTERNAL_SERVER_ERROR);
286+
287+
let response = (StatusCode::CREATED, [("\n", "\n")], ()).into_response();
288+
assert_eq!(response.status(), StatusCode::INTERNAL_SERVER_ERROR);
289+
}
290+
}

‎axum-core/src/response/mod.rs‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44
//!
55
//! [`axum::response`]: https://docs.rs/axum/0.8/axum/response/index.html
66
7+
use std::convert::Infallible;
8+
9+
use http::StatusCode;
10+
711
use crate::body::Body;
812

913
mod append_headers;
@@ -128,3 +132,87 @@ where
128132
Self(value.into_response())
129133
}
130134
}
135+
136+
/// Response part that stops status code overrides.
137+
///
138+
/// This type should be used by types implementing [`IntoResponseParts`] or
139+
/// [`IntoResponse`] when they fail to produce the response usually expected of
140+
/// them and return some sort of error response instead.
141+
///
142+
/// It is checked used by the tuple impls of [`IntoResponse`] that have a
143+
/// [`StatusCode`] as their first element to ignore that status code.
144+
/// Consider the following example:
145+
///
146+
/// ```no_run
147+
/// # use axum::Json;
148+
/// # use http::StatusCode;
149+
/// # #[derive(serde::Serialize)]
150+
/// # struct CreatedResponse { }
151+
/// fn my_handler(/* ... */) -> (StatusCode, Json<CreatedResponse>) {
152+
/// // This response type's serialization may fail
153+
/// let response = CreatedResponse { /* ... */ };
154+
/// (StatusCode::CREATED, Json(response))
155+
/// }
156+
/// ```
157+
///
158+
/// When `response` serialization succeeds, the server responds with a status
159+
/// code of 201 Created (overwriting `Json`s default status code of 200 OK),
160+
/// and the expected JSON payload.
161+
///
162+
/// When `response` serialization fails hoewever, `impl IntoResponse for Json`
163+
/// return a response with status code 500 Internal Server Error, and
164+
/// `IntoResponseFailed` as a response extension, and the 201 Created override
165+
/// is ignored.
166+
///
167+
/// This is a behavior introduced with axum 0.9.\
168+
/// To force a status code override even when an inner [`IntoResponseParts`] /
169+
/// [`IntoResponse`] failed, use [`ForceStatusCode`].
170+
#[derive(Copy, Clone, Debug)]
171+
pub struct IntoResponseFailed;
172+
173+
impl IntoResponseParts for IntoResponseFailed {
174+
type Error = Infallible;
175+
176+
fn into_response_parts(self, mut res: ResponseParts) -> Result<ResponseParts, Self::Error> {
177+
res.extensions_mut().insert(self);
178+
Ok(res)
179+
}
180+
}
181+
182+
/// Not sure it makes sense to return `IntoResponseFailed` as the whole response. You should
183+
/// probably at least combine it with a status code.
184+
///
185+
/// ```compile_fail
186+
/// fn foo()
187+
/// where
188+
/// axum_core::response::IntoResponseFailed: axum_core::response::IntoResponse,
189+
/// {}
190+
/// ```
191+
#[allow(dead_code)]
192+
fn into_response_failed_doesnt_impl_into_response() {}
193+
194+
/// Set the status code regardless of whether [`IntoResponseFailed`] is used or not.
195+
///
196+
/// See the docs for [`IntoResponseFailed`] for more details.
197+
#[derive(Debug, Copy, Clone, Default)]
198+
pub struct ForceStatusCode(pub StatusCode);
199+
200+
impl IntoResponse for ForceStatusCode {
201+
fn into_response(self) -> Response {
202+
let mut res = ().into_response();
203+
*res.status_mut() = self.0;
204+
res
205+
}
206+
}
207+
208+
impl<R> IntoResponse for (ForceStatusCode, R)
209+
where
210+
R: IntoResponse,
211+
{
212+
fn into_response(self) -> Response {
213+
let (ForceStatusCode(status), res) = self;
214+
let mut res = res.into_response();
215+
*res.status_mut() = status;
216+
res
217+
}
218+
}

‎axum-extra/src/protobuf.rs‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use axum_core::__composite_rejection as composite_rejection;
44
use axum_core::__define_rejection as define_rejection;
55
use axum_core::{
66
extract::{rejection::BytesRejection, FromRequest, Request},
7-
response::{IntoResponse, Response},
7+
response::{IntoResponse, IntoResponseFailed, Response},
88
RequestExt,
99
};
1010
use bytes::BytesMut;
@@ -131,7 +131,12 @@ where
131131
let mut buf = BytesMut::with_capacity(self.0.encoded_len());
132132
match &self.0.encode(&mut buf) {
133133
Ok(()) => buf.into_response(),
134-
Err(err) => (StatusCode::INTERNAL_SERVER_ERROR, err.to_string()).into_response(),
134+
Err(err) => (
135+
StatusCode::INTERNAL_SERVER_ERROR,
136+
IntoResponseFailed,
137+
err.to_string(),
138+
)
139+
.into_response(),
135140
}
136141
}
137142
}

‎axum-extra/src/response/erased_json.rs‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::sync::Arc;
22

3-
use axum_core::response::{IntoResponse, Response};
3+
use axum_core::response::{IntoResponse, IntoResponseFailed, Response};
44
use bytes::{BufMut, Bytes, BytesMut};
55
use http::{header, HeaderValue, StatusCode};
66
use serde_core::Serialize;
@@ -78,7 +78,12 @@ impl IntoResponse for ErasedJson {
7878
bytes,
7979
)
8080
.into_response(),
81-
Err(err) => (StatusCode::INTERNAL_SERVER_ERROR, err.to_string()).into_response(),
81+
Err(err) => (
82+
StatusCode::INTERNAL_SERVER_ERROR,
83+
IntoResponseFailed,
84+
err.to_string(),
85+
)
86+
.into_response(),
8287
}
8388
}
8489
}

‎axum/src/form.rs‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::extract::Request;
22
use crate::extract::{rejection::*, FromRequest, RawForm};
3-
use axum_core::response::{IntoResponse, Response};
3+
use axum_core::response::{IntoResponse, IntoResponseFailed, Response};
44
use axum_core::RequestExt;
55
use http::header::CONTENT_TYPE;
66
use http::StatusCode;
@@ -117,7 +117,12 @@ where
117117
body,
118118
)
119119
.into_response(),
120-
Err(err) => (StatusCode::INTERNAL_SERVER_ERROR, err.to_string()).into_response(),
120+
Err(err) => (
121+
StatusCode::INTERNAL_SERVER_ERROR,
122+
IntoResponseFailed,
123+
err.to_string(),
124+
)
125+
.into_response(),
121126
}
122127
}
123128

‎axum/src/json.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use crate::extract::Request;
22
use crate::extract::{rejection::*, FromRequest};
33
use axum_core::extract::OptionalFromRequest;
4-
use axum_core::response::{IntoResponse, Response};
4+
use axum_core::response::{IntoResponse, IntoResponseFailed, Response};
55
use bytes::{BufMut, Bytes, BytesMut};
66
use http::{
77
header::{self, HeaderMap, HeaderValue},
@@ -216,6 +216,7 @@ where
216216
header::CONTENT_TYPE,
217217
HeaderValue::from_static(mime::TEXT_PLAIN_UTF_8.as_ref()),
218218
)],
219+
IntoResponseFailed,
219220
err.to_string(),
220221
)
221222
.into_response(),

0 commit comments

Comments
 (0)