Merge pull request #288 from freedomlayer/real/fix/bug-fixes1

Pre release bug fixes
freedomlayer · Apr 18, 2020 · 4978157 · 4978157
2 parents adc5cdb + 38310d3
commit 4978157
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 8 deletions.
diff --git a/components/app_server/src/server.rs b/components/app_server/src/server.rs
@@ -25,6 +25,8 @@ use proto::index_client::messages::{
     AppServerToIndexClient, IndexClientRequest, IndexClientToAppServer,
 };
 
+const APP_SENDER_BUFFER: usize = 0x20;
+
 pub type ConnPairServer<B> = ConnPair<AppServerToApp<B>, AppToAppServer<B>>;
 
 /*
@@ -213,7 +215,7 @@ where
             .spawn(send_all_fut)
             .map_err(|_| AppServerError::SpawnError)?;
 
-        let sender = sink_to_sender(sender, &self.spawner);
+        let sender = sink_to_sender(sender, APP_SENDER_BUFFER, &self.spawner);
         let app = App::new(app_permissions, sender);
 
         self.apps.insert(self.app_counter, app);

diff --git a/components/common/src/conn.rs b/components/common/src/conn.rs
@@ -72,12 +72,13 @@ pub type ConnPairString = ConnPair<String, String>;
 /// This is useful because mpsc::Sender is cloneable.
 pub fn sink_to_sender<T>(
     mut sink: impl Sink<T> + Unpin + Send + 'static,
+    buffer: usize,
     spawner: &impl Spawn,
 ) -> mpsc::Sender<T>
 where
     T: Send + 'static,
 {
-    let (sender, receiver) = mpsc::channel(0);
+    let (sender, receiver) = mpsc::channel(buffer);
     spawner
         .spawn(async move {
             let _ = sink.send_all(&mut receiver.map(Ok)).await;

diff --git a/components/index_server/src/server_loop.rs b/components/index_server/src/server_loop.rs
@@ -29,6 +29,9 @@ use crate::verifier::Verifier;
 pub type ServerConn = ConnPair<IndexServerToServer, IndexServerToServer>;
 pub type ClientConn = ConnPair<IndexServerToClient, IndexClientToServer>;
 
+const SERVER_SENDER_BUFFER: usize = 0x20;
+const CLIENT_SENDER_BUFFER: usize = 0x20;
+
 impl LinearRate for Rate {
     /// Type used to count credits
     type K = u128;
@@ -79,11 +82,10 @@ impl<T> Connected<T> {
     pub fn try_send(&mut self, t: T) -> Result<(), ServerLoopError> {
         if let Some(mut sender) = self.opt_sender.take() {
             if let Err(e) = sender.try_send(t) {
-                if e.is_full() {
-                    warn!("try_send() failed: {:?}", e);
-                    self.opt_sender = Some(sender);
+                warn!("try_send() failed: {:?}", e);
+                if !e.is_full() {
+                    return Err(ServerLoopError::RemoteSendError);
                 }
-                return Err(ServerLoopError::RemoteSendError);
             }
             self.opt_sender = Some(sender);
         }
@@ -567,7 +569,7 @@ where
                 };
 
                 let (sender, receiver) = server_conn.split();
-                let sender = sink_to_sender(sender, &spawner);
+                let sender = sink_to_sender(sender, SERVER_SENDER_BUFFER, &spawner);
 
                 remote_server.state = RemoteServerState::Connected(Connected::new(sender));
 
@@ -621,7 +623,7 @@ where
 
                 // Duplicate the client sender:
                 let (sender, receiver) = client_conn.split();
-                let sender = sink_to_sender(sender, &spawner);
+                let sender = sink_to_sender(sender, CLIENT_SENDER_BUFFER, &spawner);
                 // TODO: Possibly use channel redirection here:
                 let c_sender = sender.clone();
 

diff --git a/components/stcompact/src/store/file_store.rs b/components/stcompact/src/store/file_store.rs
@@ -87,6 +87,7 @@ pub enum FileStoreError {
     NodeDoesNotExist,
     LoadIdentityError,
     LoadDbError,
+    InvalidNodeName,
 }
 
 impl StoreError for FileStoreError {
@@ -96,6 +97,7 @@ impl StoreError for FileStoreError {
             | FileStoreError::NodeIsLoaded
             | FileStoreError::NodeNotLoaded
             | FileStoreError::NodeDoesNotExist
+            | FileStoreError::InvalidNodeName
             | FileStoreError::RemoveNodeError => false,
             FileStoreError::SpawnError(_)
             | FileStoreError::LockError
@@ -319,6 +321,20 @@ where
     Ok(())
 }
 
+// TODO:
+// - Make checks more serious here, there might be a way around this
+// function's checks.
+// - Possibly encode node name in hex/base64 instead? In that case, how to deal with empty name?
+/// Basic verification to make sure node_name can be used as a filename.
+fn is_node_name_valid(node_name: &NodeName) -> bool {
+    !(node_name.as_str().is_empty()
+        || node_name.as_str().contains('\\')
+        || node_name.as_str().contains('/')
+        || node_name.as_str().contains(':')
+        || node_name.as_str().contains('$')
+        || node_name.as_str().contains('.'))
+}
+
 async fn create_local_node<FS>(
     node_name: NodeName,
     node_private_key: PrivateKey,
@@ -328,6 +344,10 @@ async fn create_local_node<FS>(
 where
     FS: Spawn,
 {
+    if !is_node_name_valid(&node_name) {
+        return Err(FileStoreError::InvalidNodeName);
+    }
+
     let node_path = store_path.join(LOCAL).join(&node_name.as_str());
 
     // Create node's dir. Should fail if the directory already exists:
@@ -388,6 +408,10 @@ async fn create_remote_node<FS>(
 where
     FS: Spawn,
 {
+    if !is_node_name_valid(&node_name) {
+        return Err(FileStoreError::InvalidNodeName);
+    }
+
     let node_path = store_path.join(REMOTE).join(&node_name.as_str());
 
     // Create node's dir. Should fail if the directory already exists: