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

fix(server): make tower::Service impl generic over HttpBody #1475

Merged
merged 10 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@ The format is based on [Keep a Changelog].

[Keep a Changelog]: http://keepachangelog.com/en/1.0.0/

## [v0.24.7]

niklasad1 marked this conversation as resolved.
Show resolved Hide resolved
This is a bug-fix release that fixes the tower::Service implementation to be generic over the HttpBody to work with all middleware layers.
For instance, this makes `tower_http::compression::CompressionLayer` work, which didn't compile before.
niklasad1 marked this conversation as resolved.
Show resolved Hide resolved

### [Fixed]
- fix(server): make tower::Service impl generic over HttpBody ([#1475](https://github.com/paritytech/jsonrpsee/pull/1475))

## [v0.24.6] - 2024-10-07

This is a bug-fix release that fixes that the `ConnectionGuard` was dropped before the future was resolved which,
Expand Down
21 changes: 12 additions & 9 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ resolver = "2"

[workspace.package]
authors = ["Parity Technologies <admin@parity.io>", "Pierre Krieger <pierre.krieger1708@gmail.com>"]
version = "0.24.6"
version = "0.24.7"
edition = "2021"
rust-version = "1.74.1"
license = "MIT"
Expand All @@ -31,11 +31,14 @@ keywords = ["jsonrpc", "json", "http", "websocket", "WASM"]
readme = "README.md"

[workspace.dependencies]
jsonrpsee-types = { path = "types", version = "0.24.6" }
jsonrpsee-core = { path = "core", version = "0.24.6" }
jsonrpsee-server = { path = "server", version = "0.24.6" }
jsonrpsee-ws-client = { path = "client/ws-client", version = "0.24.6" }
jsonrpsee-http-client = { path = "client/http-client", version = "0.24.6" }
jsonrpsee-wasm-client = { path = "client/wasm-client", version = "0.24.6" }
jsonrpsee-client-transport = { path = "client/transport", version = "0.24.6" }
jsonrpsee-proc-macros = { path = "proc-macros", version = "0.24.6" }
jsonrpsee-types = { path = "types", version = "0.24.7" }
jsonrpsee-core = { path = "core", version = "0.24.7" }
jsonrpsee-server = { path = "server", version = "0.24.7" }
jsonrpsee-ws-client = { path = "client/ws-client", version = "0.24.7" }
jsonrpsee-http-client = { path = "client/http-client", version = "0.24.7" }
jsonrpsee-wasm-client = { path = "client/wasm-client", version = "0.24.7" }
jsonrpsee-client-transport = { path = "client/transport", version = "0.24.7" }
jsonrpsee-proc-macros = { path = "proc-macros", version = "0.24.7" }

tower = "0.4"
tower-http = "0.5"
2 changes: 1 addition & 1 deletion client/http-client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ serde_json = "1"
thiserror = "1"
tokio = { version = "1.23.1", features = ["time"] }
tracing = "0.1.34"
tower = { version = "0.4.13", features = ["util"] }
tower = { workspace = true, features = ["util"] }
url = "2.4.0"

[dev-dependencies]
Expand Down
2 changes: 1 addition & 1 deletion examples/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ tokio = { version = "1.23.1", features = ["full"] }
tokio-stream = { version = "0.1", features = ["sync"] }
serde_json = { version = "1" }
tower-http = { version = "0.5.2", features = ["full"] }
tower = { version = "0.4.13", features = ["full"] }
tower = { workspace = true, features = ["full"] }
hyper = "1.3"
hyper-util = { version = "0.1.3", features = ["client", "client-legacy"]}
console-subscriber = "0.4.0"
2 changes: 2 additions & 0 deletions examples/examples/http_middleware.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ use jsonrpsee::rpc_params;
use std::iter::once;
use std::net::SocketAddr;
use std::time::Duration;
use tower_http::compression::CompressionLayer;
use tower_http::cors::CorsLayer;
use tower_http::sensitive_headers::SetSensitiveRequestHeadersLayer;
use tower_http::trace::{DefaultMakeSpan, DefaultOnResponse, TraceLayer};
Expand Down Expand Up @@ -107,6 +108,7 @@ async fn run_server() -> anyhow::Result<SocketAddr> {
// Mark the `Authorization` request header as sensitive so it doesn't show in logs
.layer(SetSensitiveRequestHeadersLayer::new(once(hyper::header::AUTHORIZATION)))
.layer(cors)
.layer(CompressionLayer::new())
.timeout(Duration::from_secs(2));

let server =
Expand Down
7 changes: 6 additions & 1 deletion examples/examples/jsonrpsee_as_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ use jsonrpsee::ws_client::{HeaderValue, WsClientBuilder};
use jsonrpsee::{MethodResponse, Methods};
use tokio::net::TcpListener;
use tower::Service;
use tower_http::compression::CompressionLayer;
use tower_http::cors::CorsLayer;
use tracing_subscriber::util::SubscriberInitExt;

Expand Down Expand Up @@ -202,14 +203,18 @@ async fn run_server(metrics: Metrics) -> anyhow::Result<ServerHandle> {
let transport_label = if is_websocket { "ws" } else { "http" };
let PerConnection { methods, stop_handle, metrics, svc_builder } = per_conn2.clone();

let http_middleware = tower::ServiceBuilder::new().layer(CompressionLayer::new());
// NOTE, the rpc middleware must be initialized here to be able to created once per connection
// with data from the connection such as the headers in this example
let headers = req.headers().clone();
let rpc_middleware = RpcServiceBuilder::new().rpc_logger(1024).layer_fn(move |service| {
AuthorizationMiddleware { inner: service, headers: headers.clone(), transport_label }
});

let mut svc = svc_builder.set_rpc_middleware(rpc_middleware).build(methods, stop_handle);
let mut svc = svc_builder
.set_http_middleware(http_middleware)
.set_rpc_middleware(rpc_middleware)
.build(methods, stop_handle);

if is_websocket {
// Utilize the session close future to know when the actual WebSocket
Expand Down
2 changes: 1 addition & 1 deletion proc-macros/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,4 @@ serde_json = "1"
serde = "1"
trybuild = "1.0.97"
tokio = { version = "1.23.1", features = ["rt", "macros"] }
tower = "0.4"
tower = { workspace = true }
4 changes: 2 additions & 2 deletions server/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ hyper-util = { version = "0.1", features = ["tokio", "service", "tokio", "server
http = "1"
http-body = "1"
http-body-util = "0.1.0"
tower = { version = "0.4.13", features = ["util"] }
tower = { workspace = true, features = ["util"] }
thiserror = "1"
route-recognizer = "0.3.1"
pin-project = "1.1.3"

[dev-dependencies]
jsonrpsee-test-utils = { path = "../test-utils" }
tracing-subscriber = { version = "0.3.3", features = ["env-filter"] }
tower = { version = "0.4.13", features = ["timeout"] }
tower = { workspace = true, features = ["timeout"] }
socket2 = "0.5.1"
14 changes: 7 additions & 7 deletions server/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -970,28 +970,28 @@ impl<RpcMiddleware, HttpMiddleware> TowerService<RpcMiddleware, HttpMiddleware>
}
}

impl<Body, RpcMiddleware, HttpMiddleware> Service<HttpRequest<Body>> for TowerService<RpcMiddleware, HttpMiddleware>
impl<RequestBody, ResponseBody, RpcMiddleware, HttpMiddleware> Service<HttpRequest<RequestBody>> for TowerService<RpcMiddleware, HttpMiddleware>
Copy link
Member

@niklasad1 niklasad1 Oct 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, this is the fix right i.e, to make it generic over the body?

I would rather just include this fix and avoid updating tower so we don't have make breaking release for this but looks good to me overall and fix it for folks of v0.24.x release as well

Then release v0.25 as well with tower 0.5

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@niklasad1 thanks for your quick review! Just reverted the tower* upgrade, will do that in a separate PR. BTW, should I bump the jsonrpsee patch versions as well?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually we do a separate release PR for such things but since you already done it's fine. Nice work

where
RpcMiddleware: for<'a> tower::Layer<RpcService> + Clone,
<RpcMiddleware as Layer<RpcService>>::Service: Send + Sync + 'static,
for<'a> <RpcMiddleware as Layer<RpcService>>::Service: RpcServiceT<'a>,
HttpMiddleware: Layer<TowerServiceNoHttp<RpcMiddleware>> + Send + 'static,
<HttpMiddleware as Layer<TowerServiceNoHttp<RpcMiddleware>>>::Service:
Send + Service<HttpRequest<Body>, Response = HttpResponse, Error = Box<(dyn StdError + Send + Sync + 'static)>>,
<<HttpMiddleware as Layer<TowerServiceNoHttp<RpcMiddleware>>>::Service as Service<HttpRequest<Body>>>::Future:
Send + Service<HttpRequest<RequestBody>, Response = HttpResponse<ResponseBody>, Error = Box<(dyn StdError + Send + Sync + 'static)>>,
<<HttpMiddleware as Layer<TowerServiceNoHttp<RpcMiddleware>>>::Service as Service<HttpRequest<RequestBody>>>::Future:
Send + 'static,
Body: http_body::Body<Data = Bytes> + Send + 'static,
Body::Error: Into<BoxError>,
RequestBody: http_body::Body<Data = Bytes> + Send + 'static,
RequestBody::Error: Into<BoxError>,
{
type Response = HttpResponse;
type Response = HttpResponse<ResponseBody>;
type Error = BoxError;
type Future = Pin<Box<dyn Future<Output = Result<Self::Response, Self::Error>> + Send>>;

fn poll_ready(&mut self, _cx: &mut std::task::Context<'_>) -> Poll<Result<(), Self::Error>> {
Poll::Ready(Ok(()))
}

fn call(&mut self, request: HttpRequest<Body>) -> Self::Future {
fn call(&mut self, request: HttpRequest<RequestBody>) -> Self::Future {
Box::pin(self.http_middleware.service(self.rpc_middleware.clone()).call(request))
}
}
Expand Down
4 changes: 2 additions & 2 deletions tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ serde_json = "1"
tokio = { version = "1.23.1", features = ["full"] }
tokio-stream = "0.1"
tokio-util = { version = "0.7", features = ["compat"]}
tower = { version = "0.4.13", features = ["full"] }
tower-http = { version = "0.5.2", features = ["full"] }
tower = { workspace = true, features = ["full"] }
tower-http = { workspace = true, features = ["full"] }
tracing = "0.1.34"
tracing-subscriber = { version = "0.3.3", features = ["env-filter"] }
pin-project = "1"
Loading