Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove json and url encoded form support from -http #2148

Merged
merged 3 commits into from Apr 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 {
robjtede marked this conversation as resolved.
Show resolved Hide resolved
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