From 9a08f04a7698ae237e352c57ebb58456e727fc93 Mon Sep 17 00:00:00 2001 From: Gabriela Moldovan Date: Fri, 11 Aug 2023 19:26:32 +0100 Subject: [PATCH 01/12] tor-dirclient: Rename download() to send_request(). `download()` is actually a general-purpose function for sending HTTP requests on a stream. We will soon repurpose it for `POST`-ing descriptors, so let's rename it to `send_request`. --- crates/tor-dirclient/semver.md | 1 + crates/tor-dirclient/src/lib.rs | 12 ++++++------ crates/tor-dirclient/src/util.rs | 1 + crates/tor-dirmgr/src/bridgedesc.rs | 4 ++-- crates/tor-hsclient/src/connect.rs | 2 +- 5 files changed, 11 insertions(+), 9 deletions(-) create mode 100644 crates/tor-dirclient/semver.md diff --git a/crates/tor-dirclient/semver.md b/crates/tor-dirclient/semver.md new file mode 100644 index 000000000..42d5de6b1 --- /dev/null +++ b/crates/tor-dirclient/semver.md @@ -0,0 +1 @@ +BREAKING: `download()` is renamed to `send_request()` diff --git a/crates/tor-dirclient/src/lib.rs b/crates/tor-dirclient/src/lib.rs index f094c63e4..714ce9df8 100644 --- a/crates/tor-dirclient/src/lib.rs +++ b/crates/tor-dirclient/src/lib.rs @@ -79,7 +79,7 @@ pub type RequestResult = std::result::Result; /// constructed using `dirinfo`. /// /// For more fine-grained control over the circuit and stream used, -/// construct them yourself, and then call [`download`] instead. +/// construct them yourself, and then call [`send_request`] instead. /// /// # TODO /// @@ -122,7 +122,7 @@ where // TODO: Perhaps we want separate timeouts for each phase of this. // For now, we just use higher-level timeouts in `dirmgr`. - let r = download(runtime, req, &mut stream, Some(source.clone())).await; + let r = send_request(runtime, req, &mut stream, Some(source.clone())).await; if should_retire_circ(&r) { retire_circ(&circ_mgr, &source, "Partial response"); @@ -140,7 +140,7 @@ fn should_retire_circ(result: &Result) -> bool { } } -/// Fetch a Tor directory object from a provided stream. +/// Fetch or upload a Tor directory object using the provided stream. /// /// To do this, we send a simple HTTP/1.0 request for the described /// object in `req` over `stream`, and then wait for a response. In @@ -158,7 +158,7 @@ fn should_retire_circ(result: &Result) -> bool { /// The only error variant returned is [`Error::RequestFailed`]. // TODO: should the error return type change to `RequestFailedError`? // If so, that would simplify some code in_dirmgr::bridgedesc. -pub async fn download( +pub async fn send_request( runtime: &SP, req: &R, stream: &mut S, @@ -660,7 +660,7 @@ mod test { ) = futures::join!( async { // Run the download function. - let r = download(&rt, &req, &mut s1, None).await; + let r = send_request(&rt, &req, &mut s1, None).await; s1.close().await.map_err(|error| { Error::RequestFailed(RequestFailedError { source: None, @@ -703,7 +703,7 @@ mod test { } #[test] - fn test_download() -> RequestResult<()> { + fn test_send_request() -> RequestResult<()> { let req: request::MicrodescRequest = vec![[9; 32]].into_iter().collect(); let (response, request) = run_download_test( diff --git a/crates/tor-dirclient/src/util.rs b/crates/tor-dirclient/src/util.rs index 5c9b0685f..ccb713ce6 100644 --- a/crates/tor-dirclient/src/util.rs +++ b/crates/tor-dirclient/src/util.rs @@ -16,6 +16,7 @@ pub(crate) fn encode_request(req: &http::Request<()>) -> String { ) .unwrap(); } + // TODO HSS encode the body s.push_str("\r\n"); s } diff --git a/crates/tor-dirmgr/src/bridgedesc.rs b/crates/tor-dirmgr/src/bridgedesc.rs index dd9a13045..982de829a 100644 --- a/crates/tor-dirmgr/src/bridgedesc.rs +++ b/crates/tor-dirmgr/src/bridgedesc.rs @@ -212,11 +212,11 @@ impl mockable::MockableAPI for () { .await .map_err(Error::StreamFailed)?; let request = tor_dirclient::request::RoutersOwnDescRequest::new(); - let response = tor_dirclient::download(runtime, &request, &mut stream, None) + let response = tor_dirclient::send_request(runtime, &request, &mut stream, None) .await .map_err(|dce| match dce { tor_dirclient::Error::RequestFailed(re) => Error::RequestFailed(re), - _ => internal!("tor_dirclient::download gave non-RequestFailed {:?}", dce).into(), + _ => internal!("tor_dirclient::send_request gave non-RequestFailed {:?}", dce).into(), })?; let output = response.into_output_string()?; Ok(Some(output)) diff --git a/crates/tor-hsclient/src/connect.rs b/crates/tor-hsclient/src/connect.rs index 7a261e981..69ffb0f16 100644 --- a/crates/tor-hsclient/src/connect.rs +++ b/crates/tor-hsclient/src/connect.rs @@ -580,7 +580,7 @@ impl<'c, R: Runtime, M: MocksForConnect> Context<'c, R, M> { .await .map_err(DescriptorErrorDetail::Stream)?; - let response = tor_dirclient::download(self.runtime, &request, &mut stream, None) + let response = tor_dirclient::send_request(self.runtime, &request, &mut stream, None) .await .map_err(|dir_error| match dir_error { tor_dirclient::Error::RequestFailed(rfe) => DescriptorErrorDetail::from(rfe.error), From 0fde1d09f50794163e2a5cb3a61f59a555d98c84 Mon Sep 17 00:00:00 2001 From: Gabriela Moldovan Date: Mon, 14 Aug 2023 01:20:20 +0100 Subject: [PATCH 02/12] tor-dirclient: Rename download() to send_request() (fmt). --- crates/tor-dirmgr/src/bridgedesc.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/tor-dirmgr/src/bridgedesc.rs b/crates/tor-dirmgr/src/bridgedesc.rs index 982de829a..aa55dc8f8 100644 --- a/crates/tor-dirmgr/src/bridgedesc.rs +++ b/crates/tor-dirmgr/src/bridgedesc.rs @@ -216,7 +216,11 @@ impl mockable::MockableAPI for () { .await .map_err(|dce| match dce { tor_dirclient::Error::RequestFailed(re) => Error::RequestFailed(re), - _ => internal!("tor_dirclient::send_request gave non-RequestFailed {:?}", dce).into(), + _ => internal!( + "tor_dirclient::send_request gave non-RequestFailed {:?}", + dce + ) + .into(), })?; let output = response.into_output_string()?; Ok(Some(output)) From 8ce948bef970d3708365172d66394a5a68e1acac Mon Sep 17 00:00:00 2001 From: Gabriela Moldovan Date: Mon, 14 Aug 2023 00:43:51 +0100 Subject: [PATCH 03/12] tor-dirclient: Make the body type of a `Requestable` type configurable. Previously, the `Requestable` trait assumed the body of the request would always be empty (`http::Request<()>`). This change replaces the hardcoded `()` body type with the `Requestable::Body` associated type (which will allow implementors to create requests with non-empty bodies). This will enable us to reuse the `Requestable` trait for building `POST` requests for uploading descriptors. --- crates/tor-dirclient/semver.md | 2 ++ crates/tor-dirclient/src/request.rs | 43 ++++++++++++++++++++++++++++- crates/tor-dirclient/src/util.rs | 6 ++-- crates/tor-dirmgr/src/docid.rs | 2 +- 4 files changed, 49 insertions(+), 4 deletions(-) diff --git a/crates/tor-dirclient/semver.md b/crates/tor-dirclient/semver.md index 42d5de6b1..89a97031f 100644 --- a/crates/tor-dirclient/semver.md +++ b/crates/tor-dirclient/semver.md @@ -1 +1,3 @@ BREAKING: `download()` is renamed to `send_request()` +BREAKING: `Requestable` now has an associated `Body` type +ADDED: `StringBody` trait diff --git a/crates/tor-dirclient/src/request.rs b/crates/tor-dirclient/src/request.rs index d6be07525..75a886b48 100644 --- a/crates/tor-dirclient/src/request.rs +++ b/crates/tor-dirclient/src/request.rs @@ -24,11 +24,40 @@ use itertools::Itertools; use crate::err::RequestError; +/// The body of an [`http::Request`] made by a `Requestable` implementation. +pub trait StringBody { + /// The string representation of the request body. + /// + /// Bodies consisting of binary data are not supported. + fn str(&self) -> &str; +} + +impl StringBody for () { + fn str(&self) -> &str { + "" + } +} + +impl StringBody for String { + fn str(&self) -> &str { + self.as_ref() + } +} + +impl StringBody for &str { + fn str(&self) -> &str { + self + } +} + /// A request for an object that can be served over the Tor directory system. pub trait Requestable { + /// The body type of the [`http::Request`]. + type Body: StringBody; + /// Build an [`http::Request`] from this Requestable, if /// it is well-formed. - fn make_request(&self) -> Result>; + fn make_request(&self) -> Result>; /// Return true if partial downloads are potentially useful. This /// is true for request types where we're going to be downloading @@ -190,6 +219,8 @@ impl Default for ConsensusRequest { } impl Requestable for ConsensusRequest { + type Body = (); + fn make_request(&self) -> Result> { // Build the URL. let mut uri = "/tor/status-vote/current/consensus".to_string(); @@ -273,6 +304,8 @@ impl AuthCertRequest { } impl Requestable for AuthCertRequest { + type Body = (); + fn make_request(&self) -> Result> { if self.ids.is_empty() { return Err(RequestError::EmptyRequest); @@ -343,6 +376,8 @@ impl MicrodescRequest { } impl Requestable for MicrodescRequest { + type Body = (); + fn make_request(&self) -> Result> { let d_encode_b64 = |d: &[u8; 32]| Base64Unpadded::encode_string(&d[..]); let ids = digest_list_stringify(&self.digests, d_encode_b64, "-") @@ -418,6 +453,8 @@ impl RouterDescRequest { #[cfg(feature = "routerdesc")] impl Requestable for RouterDescRequest { + type Body = (); + fn make_request(&self) -> Result> { let mut uri = "/tor/server/".to_string(); @@ -484,6 +521,8 @@ impl RoutersOwnDescRequest { #[cfg(feature = "routerdesc")] impl Requestable for RoutersOwnDescRequest { + type Body = (); + fn make_request(&self) -> Result> { let uri = "/tor/server/authority.z"; let req = http::Request::builder().method("GET").uri(uri); @@ -530,6 +569,8 @@ impl HsDescDownloadRequest { #[cfg(feature = "hs-client")] impl Requestable for HsDescDownloadRequest { + type Body = (); + fn make_request(&self) -> Result> { let hsid = Base64Unpadded::encode_string(self.hsid.as_ref()); // We hardcode version 3 here; if we ever have a v4 onion service diff --git a/crates/tor-dirclient/src/util.rs b/crates/tor-dirclient/src/util.rs index ccb713ce6..bd69dc60c 100644 --- a/crates/tor-dirclient/src/util.rs +++ b/crates/tor-dirclient/src/util.rs @@ -2,8 +2,10 @@ use std::fmt::Write; +use crate::request::StringBody; + /// Encode an HTTP request in a quick and dirty HTTP 1.0 format. -pub(crate) fn encode_request(req: &http::Request<()>) -> String { +pub(crate) fn encode_request(req: &http::Request) -> String { let mut s = format!("{} {} HTTP/1.0\r\n", req.method(), req.uri()); for (key, val) in req.headers().iter() { @@ -16,8 +18,8 @@ pub(crate) fn encode_request(req: &http::Request<()>) -> String { ) .unwrap(); } - // TODO HSS encode the body s.push_str("\r\n"); + s.push_str(req.body().str()); s } diff --git a/crates/tor-dirmgr/src/docid.rs b/crates/tor-dirmgr/src/docid.rs index 9486a6a64..78f5622d3 100644 --- a/crates/tor-dirmgr/src/docid.rs +++ b/crates/tor-dirmgr/src/docid.rs @@ -84,7 +84,7 @@ pub(crate) enum ClientRequest { impl ClientRequest { /// Turn a ClientRequest into a Requestable. - pub(crate) fn as_requestable(&self) -> &(dyn request::Requestable + Send + Sync) { + pub(crate) fn as_requestable(&self) -> &(dyn request::Requestable + Send + Sync) { use ClientRequest::*; match self { Consensus(a) => a, From 636a18bd7d8fe92f0a1e006db283fa3edb43fb06 Mon Sep 17 00:00:00 2001 From: Gabriela Moldovan Date: Mon, 14 Aug 2023 00:50:30 +0100 Subject: [PATCH 04/12] tor-dirclient: Add `HsDescUploadRequest`. The hsdir publisher will send the `HsDescUploadRequest`s to the appropriate directory using `send_request()`. --- crates/tor-dirclient/Cargo.toml | 3 ++ crates/tor-dirclient/semver.md | 1 + crates/tor-dirclient/src/request.rs | 46 +++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+) diff --git a/crates/tor-dirclient/Cargo.toml b/crates/tor-dirclient/Cargo.toml index 5d9e6e233..99f6b5137 100644 --- a/crates/tor-dirclient/Cargo.toml +++ b/crates/tor-dirclient/Cargo.toml @@ -16,6 +16,8 @@ default = ["xz", "zstd"] # Enable support for hidden service descriptor downloads. hs-client = ["tor-hscrypto"] +# Enable support for uploading hidden service descriptor downloads. +hs-service = ["tor-hscrypto"] xz = ["async-compression/xz"] zstd = ["async-compression/zstd"] @@ -24,6 +26,7 @@ routerdesc = [] full = [ "hs-client", + "hs-service", "xz", "zstd", "routerdesc", diff --git a/crates/tor-dirclient/semver.md b/crates/tor-dirclient/semver.md index 89a97031f..f37b00ddc 100644 --- a/crates/tor-dirclient/semver.md +++ b/crates/tor-dirclient/semver.md @@ -1,3 +1,4 @@ BREAKING: `download()` is renamed to `send_request()` BREAKING: `Requestable` now has an associated `Body` type ADDED: `StringBody` trait +ADDED: `tor-dirclient::request::HsDescUploadRequest`. diff --git a/crates/tor-dirclient/src/request.rs b/crates/tor-dirclient/src/request.rs index 75a886b48..faefc9b34 100644 --- a/crates/tor-dirclient/src/request.rs +++ b/crates/tor-dirclient/src/request.rs @@ -589,6 +589,52 @@ impl Requestable for HsDescDownloadRequest { self.max_len } } + +/// A request to upload a hidden service descriptor +/// +/// rend-spec-v3 2.2.6 +#[derive(Debug, Clone)] +#[cfg(feature = "hs-service")] +pub struct HsDescUploadRequest(String); + +#[cfg(feature = "hs-service")] +impl HsDescUploadRequest { + /// Construct a request for uploading a single onion service descriptor. + pub fn new(hsdesc: String) -> Self { + HsDescUploadRequest(hsdesc) + } +} + +#[cfg(feature = "hs-service")] +impl Requestable for HsDescUploadRequest { + type Body = String; + + fn make_request(&self) -> Result> { + /// The upload URI. + const URI: &str = "/tor/hs/3/publish"; + + let req = http::Request::builder().method("POST").uri(URI); + let req = add_common_headers(req); + // TODO HSS: we shouldn't have to clone here! + Ok(req.body(self.0.clone())?) + } + + // TODO HSS: the name of this function doesn't make sense in this case. + // Perhaps it should be renamed to `partial_response_ok()`. + fn partial_docs_ok(&self) -> bool { + false + } + + fn max_response_len(&self) -> usize { + // We expect the response body to be empty + // + // TODO HSS: perhaps we shouldn't? In the case of an error response, do we expect the body + // to contain e.g. an explanation for the error? If so, we should document this behaviour + // in rend-spec. + 0 + } +} + /// List the encodings we accept fn encodings() -> String { #[allow(unused_mut)] From e71703ad90642196cdbe2210bee382e19a1ad258 Mon Sep 17 00:00:00 2001 From: Gabriela Moldovan Date: Mon, 14 Aug 2023 11:02:25 +0100 Subject: [PATCH 05/12] tor-dirclient: Move request building to a test helper function. --- crates/tor-dirclient/src/util.rs | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/crates/tor-dirclient/src/util.rs b/crates/tor-dirclient/src/util.rs index bd69dc60c..69c9d3927 100644 --- a/crates/tor-dirclient/src/util.rs +++ b/crates/tor-dirclient/src/util.rs @@ -38,20 +38,22 @@ mod test { //! use super::*; + fn build_request(body: B, headers: &[(&str, &str)]) -> http::Request { + let mut builder = http::Request::builder().method("GET").uri("/index.html"); + + for (name, value) in headers { + builder = builder.header(*name, *value); + } + + builder.body(body).unwrap() + } + #[test] fn format() { - let req = http::Request::builder() - .method("GET") - .uri("/index.html") - .body(()) - .unwrap(); + let req = build_request((), &[]); assert_eq!(encode_request(&req), "GET /index.html HTTP/1.0\r\n\r\n"); - let req = http::Request::builder() - .method("GET") - .uri("/index.html") - .header("X-Marsupial", "Opossum") - .body(()) - .unwrap(); + + let req = build_request((), &[("X-Marsupial", "Opossum")]); assert_eq!( encode_request(&req), "GET /index.html HTTP/1.0\r\nx-marsupial: Opossum\r\n\r\n" From 966150f9b2b2d9a0ac80725ce0711d1f850dde71 Mon Sep 17 00:00:00 2001 From: Gabriela Moldovan Date: Mon, 14 Aug 2023 11:07:45 +0100 Subject: [PATCH 06/12] tor-dirclient: Extend format test to check the body is formatted too. --- crates/tor-dirclient/src/util.rs | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/crates/tor-dirclient/src/util.rs b/crates/tor-dirclient/src/util.rs index 69c9d3927..f56e97c31 100644 --- a/crates/tor-dirclient/src/util.rs +++ b/crates/tor-dirclient/src/util.rs @@ -38,6 +38,15 @@ mod test { //! use super::*; + #[derive(Copy, Clone, Debug)] + struct TestBody; + + impl StringBody for TestBody { + fn str(&self) -> &str { + "hello" + } + } + fn build_request(body: B, headers: &[(&str, &str)]) -> http::Request { let mut builder = http::Request::builder().method("GET").uri("/index.html"); @@ -50,13 +59,18 @@ mod test { #[test] fn format() { - let req = build_request((), &[]); - assert_eq!(encode_request(&req), "GET /index.html HTTP/1.0\r\n\r\n"); + fn chk_format(body: B, expected_body: &str) { + let req = build_request(body.clone(), &[]); + assert_eq!(encode_request(&req), format!("GET /index.html HTTP/1.0\r\n\r\n{expected_body}")); - let req = build_request((), &[("X-Marsupial", "Opossum")]); - assert_eq!( - encode_request(&req), - "GET /index.html HTTP/1.0\r\nx-marsupial: Opossum\r\n\r\n" - ); + let req = build_request(body, &[("X-Marsupial", "Opossum")]); + assert_eq!( + encode_request(&req), + format!("GET /index.html HTTP/1.0\r\nx-marsupial: Opossum\r\n\r\n{expected_body}") + ); + } + + chk_format((), ""); + chk_format(TestBody, "hello"); } } From 6625b2c55f65a805a49f3cc90d0cee99191cfeab Mon Sep 17 00:00:00 2001 From: Gabriela Moldovan Date: Mon, 14 Aug 2023 11:19:51 +0100 Subject: [PATCH 07/12] tor-dirclient: Extend format test to check the body is formatted too (fmt) --- crates/tor-dirclient/src/util.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/tor-dirclient/src/util.rs b/crates/tor-dirclient/src/util.rs index f56e97c31..e25d30dea 100644 --- a/crates/tor-dirclient/src/util.rs +++ b/crates/tor-dirclient/src/util.rs @@ -61,7 +61,10 @@ mod test { fn format() { fn chk_format(body: B, expected_body: &str) { let req = build_request(body.clone(), &[]); - assert_eq!(encode_request(&req), format!("GET /index.html HTTP/1.0\r\n\r\n{expected_body}")); + assert_eq!( + encode_request(&req), + format!("GET /index.html HTTP/1.0\r\n\r\n{expected_body}") + ); let req = build_request(body, &[("X-Marsupial", "Opossum")]); assert_eq!( From aa3e60b994d7f8bd4e8258b12c7aff7eb101b5d0 Mon Sep 17 00:00:00 2001 From: Gabriela Moldovan Date: Mon, 14 Aug 2023 15:58:33 +0100 Subject: [PATCH 08/12] tor-dirclient: Deprecate download() instead of removing it. --- crates/tor-dirclient/semver.md | 3 ++- crates/tor-dirclient/src/lib.rs | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/crates/tor-dirclient/semver.md b/crates/tor-dirclient/semver.md index f37b00ddc..3f8224dbf 100644 --- a/crates/tor-dirclient/semver.md +++ b/crates/tor-dirclient/semver.md @@ -1,4 +1,5 @@ -BREAKING: `download()` is renamed to `send_request()` +DEPRECATED: `download()` +ADDED: `send_request()` BREAKING: `Requestable` now has an associated `Body` type ADDED: `StringBody` trait ADDED: `tor-dirclient::request::HsDescUploadRequest`. diff --git a/crates/tor-dirclient/src/lib.rs b/crates/tor-dirclient/src/lib.rs index 714ce9df8..e270b63ee 100644 --- a/crates/tor-dirclient/src/lib.rs +++ b/crates/tor-dirclient/src/lib.rs @@ -140,6 +140,22 @@ fn should_retire_circ(result: &Result) -> bool { } } +/// Fetch a Tor directory object from a provided stream. +#[deprecated(since = "0.8.1", note = "Use send_request instead.")] +pub async fn download( + runtime: &SP, + req: &R, + stream: &mut S, + source: Option, +) -> Result +where + R: request::Requestable + ?Sized, + S: AsyncRead + AsyncWrite + Send + Unpin, + SP: SleepProvider, +{ + send_request(runtime, req, stream, source).await +} + /// Fetch or upload a Tor directory object using the provided stream. /// /// To do this, we send a simple HTTP/1.0 request for the described From 2a7ba4ceb0737530a69ef5ea04ac60383c3eeab2 Mon Sep 17 00:00:00 2001 From: Gabriela Moldovan Date: Mon, 14 Aug 2023 16:00:12 +0100 Subject: [PATCH 09/12] tor-dirclient: Rename StringBody::str() to StringBody::as_str(). --- crates/tor-dirclient/src/request.rs | 8 ++++---- crates/tor-dirclient/src/util.rs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/tor-dirclient/src/request.rs b/crates/tor-dirclient/src/request.rs index faefc9b34..615600d11 100644 --- a/crates/tor-dirclient/src/request.rs +++ b/crates/tor-dirclient/src/request.rs @@ -29,23 +29,23 @@ pub trait StringBody { /// The string representation of the request body. /// /// Bodies consisting of binary data are not supported. - fn str(&self) -> &str; + fn as_str(&self) -> &str; } impl StringBody for () { - fn str(&self) -> &str { + fn as_str(&self) -> &str { "" } } impl StringBody for String { - fn str(&self) -> &str { + fn as_str(&self) -> &str { self.as_ref() } } impl StringBody for &str { - fn str(&self) -> &str { + fn as_str(&self) -> &str { self } } diff --git a/crates/tor-dirclient/src/util.rs b/crates/tor-dirclient/src/util.rs index e25d30dea..24b224bf3 100644 --- a/crates/tor-dirclient/src/util.rs +++ b/crates/tor-dirclient/src/util.rs @@ -19,7 +19,7 @@ pub(crate) fn encode_request(req: &http::Request) -> String { .unwrap(); } s.push_str("\r\n"); - s.push_str(req.body().str()); + s.push_str(req.body().as_str()); s } @@ -42,7 +42,7 @@ mod test { struct TestBody; impl StringBody for TestBody { - fn str(&self) -> &str { + fn as_str(&self) -> &str { "hello" } } From cd6c4674dc560d9c1dc355cac627edac32138f2c Mon Sep 17 00:00:00 2001 From: Gabriela Moldovan Date: Wed, 16 Aug 2023 15:09:52 +0100 Subject: [PATCH 10/12] tor-dirclient: Make Requestable return requests with String bodies. It's simpler to always use a `String` to represent directory request bodies. We no longer need the `StringBody` trait. --- crates/tor-dirclient/semver.md | 2 +- crates/tor-dirclient/src/request.rs | 45 +++++++++-------------------- crates/tor-dirclient/src/util.rs | 29 ++++++------------- crates/tor-dirmgr/src/docid.rs | 2 +- 4 files changed, 25 insertions(+), 53 deletions(-) diff --git a/crates/tor-dirclient/semver.md b/crates/tor-dirclient/semver.md index 3f8224dbf..8b90a06a8 100644 --- a/crates/tor-dirclient/semver.md +++ b/crates/tor-dirclient/semver.md @@ -1,5 +1,5 @@ DEPRECATED: `download()` ADDED: `send_request()` -BREAKING: `Requestable` now has an associated `Body` type ADDED: `StringBody` trait ADDED: `tor-dirclient::request::HsDescUploadRequest`. +BREAKING: `Requestable::make_request` now returns `http::Request` diff --git a/crates/tor-dirclient/src/request.rs b/crates/tor-dirclient/src/request.rs index 615600d11..b82d36ea1 100644 --- a/crates/tor-dirclient/src/request.rs +++ b/crates/tor-dirclient/src/request.rs @@ -52,12 +52,9 @@ impl StringBody for &str { /// A request for an object that can be served over the Tor directory system. pub trait Requestable { - /// The body type of the [`http::Request`]. - type Body: StringBody; - /// Build an [`http::Request`] from this Requestable, if /// it is well-formed. - fn make_request(&self) -> Result>; + fn make_request(&self) -> Result>; /// Return true if partial downloads are potentially useful. This /// is true for request types where we're going to be downloading @@ -219,9 +216,7 @@ impl Default for ConsensusRequest { } impl Requestable for ConsensusRequest { - type Body = (); - - fn make_request(&self) -> Result> { + fn make_request(&self) -> Result> { // Build the URL. let mut uri = "/tor/status-vote/current/consensus".to_string(); match self.flavor { @@ -256,7 +251,7 @@ impl Requestable for ConsensusRequest { req = req.header("X-Or-Diff-From-Consensus", &ids); } - Ok(req.body(())?) + Ok(req.body(String::new())?) } fn partial_docs_ok(&self) -> bool { @@ -304,9 +299,7 @@ impl AuthCertRequest { } impl Requestable for AuthCertRequest { - type Body = (); - - fn make_request(&self) -> Result> { + fn make_request(&self) -> Result> { if self.ids.is_empty() { return Err(RequestError::EmptyRequest); } @@ -329,7 +322,7 @@ impl Requestable for AuthCertRequest { let req = http::Request::builder().method("GET").uri(uri); let req = add_common_headers(req); - Ok(req.body(())?) + Ok(req.body(String::new())?) } fn partial_docs_ok(&self) -> bool { @@ -376,9 +369,7 @@ impl MicrodescRequest { } impl Requestable for MicrodescRequest { - type Body = (); - - fn make_request(&self) -> Result> { + fn make_request(&self) -> Result> { let d_encode_b64 = |d: &[u8; 32]| Base64Unpadded::encode_string(&d[..]); let ids = digest_list_stringify(&self.digests, d_encode_b64, "-") .ok_or(RequestError::EmptyRequest)?; @@ -387,7 +378,7 @@ impl Requestable for MicrodescRequest { let req = add_common_headers(req); - Ok(req.body(())?) + Ok(req.body(String::new())?) } fn partial_docs_ok(&self) -> bool { @@ -453,9 +444,7 @@ impl RouterDescRequest { #[cfg(feature = "routerdesc")] impl Requestable for RouterDescRequest { - type Body = (); - - fn make_request(&self) -> Result> { + fn make_request(&self) -> Result> { let mut uri = "/tor/server/".to_string(); match self.requested_descriptors { @@ -475,7 +464,7 @@ impl Requestable for RouterDescRequest { let req = http::Request::builder().method("GET").uri(uri); let req = add_common_headers(req); - Ok(req.body(())?) + Ok(req.body(String::new())?) } fn partial_docs_ok(&self) -> bool { @@ -521,14 +510,12 @@ impl RoutersOwnDescRequest { #[cfg(feature = "routerdesc")] impl Requestable for RoutersOwnDescRequest { - type Body = (); - - fn make_request(&self) -> Result> { + fn make_request(&self) -> Result> { let uri = "/tor/server/authority.z"; let req = http::Request::builder().method("GET").uri(uri); let req = add_common_headers(req); - Ok(req.body(())?) + Ok(req.body(String::new())?) } fn partial_docs_ok(&self) -> bool { @@ -569,16 +556,14 @@ impl HsDescDownloadRequest { #[cfg(feature = "hs-client")] impl Requestable for HsDescDownloadRequest { - type Body = (); - - fn make_request(&self) -> Result> { + fn make_request(&self) -> Result> { let hsid = Base64Unpadded::encode_string(self.hsid.as_ref()); // We hardcode version 3 here; if we ever have a v4 onion service // descriptor, it will need a different kind of Request. let uri = format!("/tor/hs/3/{}", hsid); let req = http::Request::builder().method("GET").uri(uri); let req = add_common_headers(req); - Ok(req.body(())?) + Ok(req.body(String::new())?) } fn partial_docs_ok(&self) -> bool { @@ -607,9 +592,7 @@ impl HsDescUploadRequest { #[cfg(feature = "hs-service")] impl Requestable for HsDescUploadRequest { - type Body = String; - - fn make_request(&self) -> Result> { + fn make_request(&self) -> Result> { /// The upload URI. const URI: &str = "/tor/hs/3/publish"; diff --git a/crates/tor-dirclient/src/util.rs b/crates/tor-dirclient/src/util.rs index 24b224bf3..9e407b2f4 100644 --- a/crates/tor-dirclient/src/util.rs +++ b/crates/tor-dirclient/src/util.rs @@ -2,10 +2,8 @@ use std::fmt::Write; -use crate::request::StringBody; - /// Encode an HTTP request in a quick and dirty HTTP 1.0 format. -pub(crate) fn encode_request(req: &http::Request) -> String { +pub(crate) fn encode_request(req: &http::Request) -> String { let mut s = format!("{} {} HTTP/1.0\r\n", req.method(), req.uri()); for (key, val) in req.headers().iter() { @@ -19,7 +17,7 @@ pub(crate) fn encode_request(req: &http::Request) -> String { .unwrap(); } s.push_str("\r\n"); - s.push_str(req.body().as_str()); + s.push_str(req.body()); s } @@ -38,16 +36,7 @@ mod test { //! use super::*; - #[derive(Copy, Clone, Debug)] - struct TestBody; - - impl StringBody for TestBody { - fn as_str(&self) -> &str { - "hello" - } - } - - fn build_request(body: B, headers: &[(&str, &str)]) -> http::Request { + fn build_request(body: String, headers: &[(&str, &str)]) -> http::Request { let mut builder = http::Request::builder().method("GET").uri("/index.html"); for (name, value) in headers { @@ -59,21 +48,21 @@ mod test { #[test] fn format() { - fn chk_format(body: B, expected_body: &str) { + fn chk_format(body: String) { let req = build_request(body.clone(), &[]); assert_eq!( encode_request(&req), - format!("GET /index.html HTTP/1.0\r\n\r\n{expected_body}") + format!("GET /index.html HTTP/1.0\r\n\r\n{body}") ); - let req = build_request(body, &[("X-Marsupial", "Opossum")]); + let req = build_request(body.clone(), &[("X-Marsupial", "Opossum")]); assert_eq!( encode_request(&req), - format!("GET /index.html HTTP/1.0\r\nx-marsupial: Opossum\r\n\r\n{expected_body}") + format!("GET /index.html HTTP/1.0\r\nx-marsupial: Opossum\r\n\r\n{body}") ); } - chk_format((), ""); - chk_format(TestBody, "hello"); + chk_format(Default::default()); + chk_format("hello".into()); } } diff --git a/crates/tor-dirmgr/src/docid.rs b/crates/tor-dirmgr/src/docid.rs index 78f5622d3..9486a6a64 100644 --- a/crates/tor-dirmgr/src/docid.rs +++ b/crates/tor-dirmgr/src/docid.rs @@ -84,7 +84,7 @@ pub(crate) enum ClientRequest { impl ClientRequest { /// Turn a ClientRequest into a Requestable. - pub(crate) fn as_requestable(&self) -> &(dyn request::Requestable + Send + Sync) { + pub(crate) fn as_requestable(&self) -> &(dyn request::Requestable + Send + Sync) { use ClientRequest::*; match self { Consensus(a) => a, From c3ea366539e39012d80b22c35753fe961b1e99f0 Mon Sep 17 00:00:00 2001 From: Gabriela Moldovan Date: Wed, 16 Aug 2023 15:11:58 +0100 Subject: [PATCH 11/12] tor-dirclient: Remove unused `StringBody` trait. --- crates/tor-dirclient/semver.md | 1 - crates/tor-dirclient/src/request.rs | 26 -------------------------- 2 files changed, 27 deletions(-) diff --git a/crates/tor-dirclient/semver.md b/crates/tor-dirclient/semver.md index 8b90a06a8..b64baf6c3 100644 --- a/crates/tor-dirclient/semver.md +++ b/crates/tor-dirclient/semver.md @@ -1,5 +1,4 @@ DEPRECATED: `download()` ADDED: `send_request()` -ADDED: `StringBody` trait ADDED: `tor-dirclient::request::HsDescUploadRequest`. BREAKING: `Requestable::make_request` now returns `http::Request` diff --git a/crates/tor-dirclient/src/request.rs b/crates/tor-dirclient/src/request.rs index b82d36ea1..d7375283e 100644 --- a/crates/tor-dirclient/src/request.rs +++ b/crates/tor-dirclient/src/request.rs @@ -24,32 +24,6 @@ use itertools::Itertools; use crate::err::RequestError; -/// The body of an [`http::Request`] made by a `Requestable` implementation. -pub trait StringBody { - /// The string representation of the request body. - /// - /// Bodies consisting of binary data are not supported. - fn as_str(&self) -> &str; -} - -impl StringBody for () { - fn as_str(&self) -> &str { - "" - } -} - -impl StringBody for String { - fn as_str(&self) -> &str { - self.as_ref() - } -} - -impl StringBody for &str { - fn as_str(&self) -> &str { - self - } -} - /// A request for an object that can be served over the Tor directory system. pub trait Requestable { /// Build an [`http::Request`] from this Requestable, if From 310b4bf35efa8a21f32d46785e8e2064f4d7c2de Mon Sep 17 00:00:00 2001 From: Gabriela Moldovan Date: Wed, 16 Aug 2023 15:46:14 +0100 Subject: [PATCH 12/12] tor-dirclient: Fix clippy lints. --- crates/tor-dirclient/src/util.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/tor-dirclient/src/util.rs b/crates/tor-dirclient/src/util.rs index 9e407b2f4..b0a8aef54 100644 --- a/crates/tor-dirclient/src/util.rs +++ b/crates/tor-dirclient/src/util.rs @@ -48,21 +48,21 @@ mod test { #[test] fn format() { - fn chk_format(body: String) { - let req = build_request(body.clone(), &[]); + fn chk_format(body: &str) { + let req = build_request(body.to_string(), &[]); assert_eq!( encode_request(&req), format!("GET /index.html HTTP/1.0\r\n\r\n{body}") ); - let req = build_request(body.clone(), &[("X-Marsupial", "Opossum")]); + let req = build_request(body.to_string(), &[("X-Marsupial", "Opossum")]); assert_eq!( encode_request(&req), format!("GET /index.html HTTP/1.0\r\nx-marsupial: Opossum\r\n\r\n{body}") ); } - chk_format(Default::default()); - chk_format("hello".into()); + chk_format(""); + chk_format("hello"); } }