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

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

Open
ghost opened this issue May 13, 2024 · 4 comments
Open

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

ghost opened this issue May 13, 2024 · 4 comments

Comments

@ghost
Copy link

ghost commented May 13, 2024

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
Copy link
Member

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

@ghost
Copy link
Author

ghost commented May 17, 2024

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
Copy link
Member

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())?)
         }

@ghost
Copy link
Author

ghost commented May 24, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant