libp2p / rust-libp2p

The Rust Implementation of the libp2p networking stack.
https://libp2p.io
MIT License
4.29k stars 889 forks source link

request-response: Document request/response max sizes #5383

Open caementarius opened 1 month ago

caementarius commented 1 month ago

Description

cbor and json codecs have hard-coded req-resp max size that are not documentated. I propose to document them at the Behaviour section

Motivation

Trying to send a too-big request throw I/O errors (like WriteZero) which is not always easy to understand (for beginner like me at least). Having a documentation might help to understand the source of the issue when facing such errors.

Current Implementation

No documentation

Are you planning to do it yourself in a pull request ?

Yes

dariusc93 commented 1 month ago

I do like the idea of this being documented but I do wonder if we should make sizes configurable, either as a const or adding fields to Codec for the size

caementarius commented 1 month ago

I do like the idea of this being documented but I do wonder if we should make sizes configurable, either as a const or adding fields to Codec for the size

I like the idea too, it would require more work but it might be usefull

dariusc93 commented 1 month ago

I like the idea too, it would require more work but it might be usefull

Though outside the scope of this issue, I would imagine it being something along the lines of the following (give or take):

diff --git a/protocols/request-response/src/cbor.rs b/protocols/request-response/src/cbor.rs
index 44d82be2..aaad251c 100644
--- a/protocols/request-response/src/cbor.rs
+++ b/protocols/request-response/src/cbor.rs
@@ -56,21 +56,30 @@ mod codec {
     /// Max response size in bytes
     const RESPONSE_SIZE_MAXIMUM: u64 = 10 * 1024 * 1024;

+    #[derive(Clone)]
     pub struct Codec<Req, Resp> {
+        max_request_size: u64,
+        max_response_size: u64,
         phantom: PhantomData<(Req, Resp)>,
     }

     impl<Req, Resp> Default for Codec<Req, Resp> {
         fn default() -> Self {
             Codec {
+                max_response_size: RESPONSE_SIZE_MAXIMUM,
+                max_request_size: REQUEST_SIZE_MAXIMUM,
                 phantom: PhantomData,
             }
         }
     }

-    impl<Req, Resp> Clone for Codec<Req, Resp> {
-        fn clone(&self) -> Self {
-            Self::default()
+    impl<Req, Resp> Codec<Req, Resp> {
+        pub fn new(max_request: u64, max_response: u64) -> Self {
+            Self {
+                max_request_size: max_request,
+                max_response_size: max_response,
+                phantom: PhantomData
+            }
         }
     }

@@ -90,7 +99,7 @@ mod codec {
         {
             let mut vec = Vec::new();

-            io.take(REQUEST_SIZE_MAXIMUM).read_to_end(&mut vec).await?;
+            io.take(self.max_request_size).read_to_end(&mut vec).await?;

             cbor4ii::serde::from_slice(vec.as_slice()).map_err(decode_into_io_error)
         }
@@ -101,7 +110,7 @@ mod codec {
         {
             let mut vec = Vec::new();

-            io.take(RESPONSE_SIZE_MAXIMUM).read_to_end(&mut vec).await?;
+            io.take(self.max_response_size).read_to_end(&mut vec).await?;

             cbor4ii::serde::from_slice(vec.as_slice()).map_err(decode_into_io_error)
         }
diff --git a/protocols/request-response/src/json.rs b/protocols/request-response/src/json.rs
index 85e78e7d..09c1f45f 100644
--- a/protocols/request-response/src/json.rs
+++ b/protocols/request-response/src/json.rs
@@ -54,21 +54,30 @@ mod codec {
     /// Max response size in bytes
     const RESPONSE_SIZE_MAXIMUM: u64 = 10 * 1024 * 1024;

+    #[derive(Clone)]
     pub struct Codec<Req, Resp> {
+        max_request_size: u64,
+        max_response_size: u64,
         phantom: PhantomData<(Req, Resp)>,
     }

     impl<Req, Resp> Default for Codec<Req, Resp> {
         fn default() -> Self {
             Codec {
+                max_response_size: RESPONSE_SIZE_MAXIMUM,
+                max_request_size: REQUEST_SIZE_MAXIMUM,
                 phantom: PhantomData,
             }
         }
     }

-    impl<Req, Resp> Clone for Codec<Req, Resp> {
-        fn clone(&self) -> Self {
-            Self::default()
+    impl<Req, Resp> Codec<Req, Resp> {
+        pub fn new(max_request: u64, max_response: u64) -> Self {
+            Self {
+                max_request_size: max_request,
+                max_response_size: max_response,
+                phantom: PhantomData
+            }
         }
     }

@@ -88,7 +97,7 @@ mod codec {
         {
             let mut vec = Vec::new();

-            io.take(REQUEST_SIZE_MAXIMUM).read_to_end(&mut vec).await?;
+            io.take(self.max_request_size).read_to_end(&mut vec).await?;

             Ok(serde_json::from_slice(vec.as_slice())?)
         }
@@ -99,7 +108,7 @@ mod codec {
         {
             let mut vec = Vec::new();

-            io.take(RESPONSE_SIZE_MAXIMUM).read_to_end(&mut vec).await?;
+            io.take(self.max_response_size).read_to_end(&mut vec).await?;

             Ok(serde_json::from_slice(vec.as_slice())?)
         }
caementarius commented 1 month ago

Thank you @dariusc93, it seems perfect ! With the documentation it should be good