Skip to content

Commit

Permalink
remove json and url encoded form support from -http (#2148)
Browse files Browse the repository at this point in the history
  • Loading branch information
robjtede committed Apr 12, 2021
1 parent 44c55dd commit 981c544
Show file tree
Hide file tree
Showing 15 changed files with 137 additions and 258 deletions.
5 changes: 5 additions & 0 deletions CHANGES.md
Expand Up @@ -4,7 +4,12 @@
### Added
* `HttpResponse` and `HttpResponseBuilder` structs. [#2065]

### Changed
* Most error types are now marked `#[non_exhaustive]`. [#2148]

[#2065]: https://github.com/actix/actix-web/pull/2065
[#2148]: https://github.com/actix/actix-web/pull/2148


## 4.0.0-beta.5 - 2021-04-02
### Added
Expand Down
5 changes: 5 additions & 0 deletions actix-http/CHANGES.md
Expand Up @@ -6,8 +6,13 @@
* Top-level `cookies` mod (re-export). [#2065]
* `HttpMessage` trait loses the `cookies` and `cookie` methods. [#2065]
* `impl ResponseError for CookieParseError`. [#2065]
* Deprecated methods on `ResponseBuilder`: `if_true`, `if_some`. [#2148]
* `ResponseBuilder::json`. [#2148]
* `ResponseBuilder::{set_header, header}`. [#2148]
* `impl From<serde_json::Value> for Body`. [#2148]

[#2065]: https://github.com/actix/actix-web/pull/2065
[#2148]: https://github.com/actix/actix-web/pull/2148


## 3.0.0-beta.5 - 2021-04-02
Expand Down
3 changes: 1 addition & 2 deletions actix-http/Cargo.toml
Expand Up @@ -68,8 +68,6 @@ pin-project-lite = "0.2"
rand = "0.8"
regex = "1.3"
serde = "1.0"
serde_json = "1.0"
serde_urlencoded = "0.7"
sha-1 = "0.9"
smallvec = "1.6"
time = { version = "0.2.23", default-features = false, features = ["std"] }
Expand All @@ -89,6 +87,7 @@ criterion = "0.3"
env_logger = "0.8"
rcgen = "0.8"
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"
tls-openssl = { version = "0.10", package = "openssl" }
tls-rustls = { version = "0.19", package = "rustls" }

Expand Down
6 changes: 0 additions & 6 deletions actix-http/src/body/body.rs
Expand Up @@ -132,12 +132,6 @@ impl From<BytesMut> for Body {
}
}

impl From<serde_json::Value> for Body {
fn from(v: serde_json::Value) -> Body {
Body::Bytes(v.to_string().into())
}
}

impl<S> From<SizedStream<S>> for Body
where
S: Stream<Item = Result<Bytes, Error>> + Unpin + 'static,
Expand Down
8 changes: 5 additions & 3 deletions actix-http/src/body/mod.rs
Expand Up @@ -174,13 +174,15 @@ mod tests {

#[actix_rt::test]
async fn test_serde_json() {
use serde_json::json;
use serde_json::{json, Value};
assert_eq!(
Body::from(serde_json::Value::String("test".into())).size(),
Body::from(serde_json::to_vec(&Value::String("test".to_owned())).unwrap())
.size(),
BodySize::Sized(6)
);
assert_eq!(
Body::from(json!({"test-key":"test-value"})).size(),
Body::from(serde_json::to_vec(&json!({"test-key":"test-value"})).unwrap())
.size(),
BodySize::Sized(25)
);
}
Expand Down
31 changes: 12 additions & 19 deletions actix-http/src/error.rs
@@ -1,18 +1,18 @@
//! Error and Result module

use std::cell::RefCell;
use std::io::Write;
use std::str::Utf8Error;
use std::string::FromUtf8Error;
use std::{fmt, io, result};
use std::{
cell::RefCell,
fmt,
io::{self, Write as _},
str::Utf8Error,
string::FromUtf8Error,
};

use bytes::BytesMut;
use derive_more::{Display, From};
use derive_more::{Display, Error, From};
use http::uri::InvalidUri;
use http::{header, Error as HttpError, StatusCode};
use serde::de::value::Error as DeError;
use serde_json::error::Error as JsonError;
use serde_urlencoded::ser::Error as FormError;

use crate::{body::Body, helpers::Writer, Response, ResponseBuilder};

Expand All @@ -22,7 +22,7 @@ use crate::{body::Body, helpers::Writer, Response, ResponseBuilder};
/// This typedef is generally used to avoid writing out
/// `actix_http::error::Error` directly and is otherwise a direct mapping to
/// `Result`.
pub type Result<T, E = Error> = result::Result<T, E>;
pub type Result<T, E = Error> = std::result::Result<T, E>;

/// General purpose actix web error.
///
Expand Down Expand Up @@ -147,14 +147,8 @@ struct UnitError;
/// Returns [`StatusCode::INTERNAL_SERVER_ERROR`] for [`UnitError`].
impl ResponseError for UnitError {}

/// Returns [`StatusCode::INTERNAL_SERVER_ERROR`] for [`JsonError`].
impl ResponseError for JsonError {}

/// Returns [`StatusCode::INTERNAL_SERVER_ERROR`] for [`FormError`].
impl ResponseError for FormError {}

#[cfg(feature = "openssl")]
/// Returns [`StatusCode::INTERNAL_SERVER_ERROR`] for [`actix_tls::accept::openssl::SslError`].
#[cfg(feature = "openssl")]
impl ResponseError for actix_tls::accept::openssl::SslError {}

/// Returns [`StatusCode::BAD_REQUEST`] for [`DeError`].
Expand Down Expand Up @@ -421,18 +415,17 @@ pub enum DispatchError {
}

/// A set of error that can occur during parsing content type
#[derive(PartialEq, Debug, Display)]
#[derive(Debug, PartialEq, Display, Error)]
pub enum ContentTypeError {
/// Can not parse content type
#[display(fmt = "Can not parse content type")]
ParseError,

/// Unknown content encoding
#[display(fmt = "Unknown content encoding")]
UnknownEncoding,
}

impl std::error::Error for ContentTypeError {}

/// Return `BadRequest` for `ContentTypeError`
impl ResponseError for ContentTypeError {
fn status_code(&self) -> StatusCode {
Expand Down
145 changes: 5 additions & 140 deletions actix-http/src/response.rs
Expand Up @@ -2,7 +2,6 @@

use std::{
cell::{Ref, RefMut},
convert::TryInto,
fmt,
future::Future,
pin::Pin,
Expand All @@ -12,17 +11,13 @@ use std::{

use bytes::{Bytes, BytesMut};
use futures_core::Stream;
use serde::Serialize;

use crate::{
body::{Body, BodyStream, MessageBody, ResponseBody},
error::Error,
extensions::Extensions,
header::{IntoHeaderPair, IntoHeaderValue},
http::{
header::{self, HeaderName},
Error as HttpError, HeaderMap, StatusCode,
},
http::{header, Error as HttpError, HeaderMap, StatusCode},
message::{BoxedResponseHead, ConnectionType, ResponseHead},
};

Expand Down Expand Up @@ -335,54 +330,6 @@ impl ResponseBuilder {
self
}

/// Replaced with [`Self::insert_header()`].
#[deprecated(
since = "4.0.0",
note = "Replaced with `insert_header((key, value))`. Will be removed in v5."
)]
pub fn set_header<K, V>(&mut self, key: K, value: V) -> &mut Self
where
K: TryInto<HeaderName>,
K::Error: Into<HttpError>,
V: IntoHeaderValue,
{
if self.err.is_some() {
return self;
}

match (key.try_into(), value.try_into_value()) {
(Ok(name), Ok(value)) => return self.insert_header((name, value)),
(Err(err), _) => self.err = Some(err.into()),
(_, Err(err)) => self.err = Some(err.into()),
}

self
}

/// Replaced with [`Self::append_header()`].
#[deprecated(
since = "4.0.0",
note = "Replaced with `append_header((key, value))`. Will be removed in v5."
)]
pub fn header<K, V>(&mut self, key: K, value: V) -> &mut Self
where
K: TryInto<HeaderName>,
K::Error: Into<HttpError>,
V: IntoHeaderValue,
{
if self.err.is_some() {
return self;
}

match (key.try_into(), value.try_into_value()) {
(Ok(name), Ok(value)) => return self.append_header((name, value)),
(Err(err), _) => self.err = Some(err.into()),
(_, Err(err)) => self.err = Some(err.into()),
}

self
}

/// Set the custom reason for the response.
#[inline]
pub fn reason(&mut self, reason: &'static str) -> &mut Self {
Expand Down Expand Up @@ -456,32 +403,6 @@ impl ResponseBuilder {
self
}

/// This method calls provided closure with builder reference if value is `true`.
#[doc(hidden)]
#[deprecated = "Use an if statement."]
pub fn if_true<F>(&mut self, value: bool, f: F) -> &mut Self
where
F: FnOnce(&mut ResponseBuilder),
{
if value {
f(self);
}
self
}

/// This method calls provided closure with builder reference if value is `Some`.
#[doc(hidden)]
#[deprecated = "Use an if-let construction."]
pub fn if_some<T, F>(&mut self, value: Option<T>, f: F) -> &mut Self
where
F: FnOnce(T, &mut ResponseBuilder),
{
if let Some(val) = value {
f(val, self);
}
self
}

/// Responses extensions
#[inline]
pub fn extensions(&self) -> Ref<'_, Extensions> {
Expand All @@ -496,10 +417,10 @@ impl ResponseBuilder {
head.extensions.borrow_mut()
}

#[inline]
/// Set a body and generate `Response`.
///
/// `ResponseBuilder` can not be used after this call.
#[inline]
pub fn body<B: Into<Body>>(&mut self, body: B) -> Response {
self.message_body(body.into())
}
Expand All @@ -521,10 +442,10 @@ impl ResponseBuilder {
}
}

#[inline]
/// Set a streaming body and generate `Response`.
///
/// `ResponseBuilder` can not be used after this call.
#[inline]
pub fn streaming<S, E>(&mut self, stream: S) -> Response
where
S: Stream<Item = Result<Bytes, E>> + Unpin + 'static,
Expand All @@ -533,32 +454,10 @@ impl ResponseBuilder {
self.body(Body::from_message(BodyStream::new(stream)))
}

/// Set a json body and generate `Response`
///
/// `ResponseBuilder` can not be used after this call.
pub fn json(&mut self, value: impl Serialize) -> Response {
match serde_json::to_string(&value) {
Ok(body) => {
let contains = if let Some(parts) = parts(&mut self.head, &self.err) {
parts.headers.contains_key(header::CONTENT_TYPE)
} else {
true
};

if !contains {
self.insert_header((header::CONTENT_TYPE, mime::APPLICATION_JSON));
}

self.body(Body::from(body))
}
Err(e) => Error::from(e).into(),
}
}

#[inline]
/// Set an empty body and generate `Response`
///
/// `ResponseBuilder` can not be used after this call.
#[inline]
pub fn finish(&mut self) -> Response {
self.body(Body::Empty)
}
Expand Down Expand Up @@ -706,11 +605,9 @@ impl From<BytesMut> for Response {

#[cfg(test)]
mod tests {
use serde_json::json;

use super::*;
use crate::body::Body;
use crate::http::header::{HeaderValue, CONTENT_TYPE, COOKIE};
use crate::http::header::{HeaderName, HeaderValue, CONTENT_TYPE, COOKIE};

#[test]
fn test_debug() {
Expand Down Expand Up @@ -754,38 +651,6 @@ mod tests {
assert_eq!(resp.headers().get(CONTENT_TYPE).unwrap(), "text/plain")
}

#[test]
fn test_json() {
let resp = Response::Ok().json(vec!["v1", "v2", "v3"]);
let ct = resp.headers().get(CONTENT_TYPE).unwrap();
assert_eq!(ct, HeaderValue::from_static("application/json"));
assert_eq!(resp.body().get_ref(), b"[\"v1\",\"v2\",\"v3\"]");

let resp = Response::Ok().json(&["v1", "v2", "v3"]);
let ct = resp.headers().get(CONTENT_TYPE).unwrap();
assert_eq!(ct, HeaderValue::from_static("application/json"));
assert_eq!(resp.body().get_ref(), b"[\"v1\",\"v2\",\"v3\"]");
}

#[test]
fn test_json_ct() {
let resp = Response::build(StatusCode::OK)
.insert_header((CONTENT_TYPE, "text/json"))
.json(&vec!["v1", "v2", "v3"]);
let ct = resp.headers().get(CONTENT_TYPE).unwrap();
assert_eq!(ct, HeaderValue::from_static("text/json"));
assert_eq!(resp.body().get_ref(), b"[\"v1\",\"v2\",\"v3\"]");
}

#[test]
fn test_serde_json_in_body() {
use serde_json::json;

let resp =
Response::build(StatusCode::OK).body(json!({"test-key":"test-value"}));
assert_eq!(resp.body().get_ref(), br#"{"test-key":"test-value"}"#);
}

#[test]
fn test_into_response() {
let resp: Response = "test".into();
Expand Down
4 changes: 4 additions & 0 deletions awc/CHANGES.md
@@ -1,6 +1,10 @@
# Changes

## Unreleased - 2021-xx-xx
### Removed
* Deprecated methods on `ClientRequest`: `if_true`, `if_some`. [#2148]

[#2148]: https://github.com/actix/actix-web/pull/2148


## 3.0.0-beta.4 - 2021-04-02
Expand Down

0 comments on commit 981c544

Please sign in to comment.