Merge #354: Drop Windows support for JSONRPC over UDS

7ce039c Make the jsonrpc module optional through a default feature (Antoine Poinsot)
767c38e jsonrpc: remove support for Windows (Antoine Poinsot)

Pull request description:

  Based on #353, which provides a way to access `revaultd`'s commands through a library API, this drops the support of our greatly hacky JSONRPC server on UDS socket for Windows >10.

  This also makes the JSONRPC server an optional feature (activated by default).

  Fixes #339
  Fixes #79
  Fixes #27

ACKs for top commit:
  edouardparis:
    ACK 7ce039c

Tree-SHA512: de1d7da33311da5b10e5e6cc23558cbdfaeccafddc74838c894076f25ba254559204782ad91ce66b85c10838e550e2071d57df88cbbbba5b10a85a1942da5c78

Author: darosior

.github/workflows/main.yml

@@ -38,5 +38,9 @@ jobs:
           toolchain: ${{ matrix.toolchain }}
           override: true
           profile: minimal
-      - name: Test on Rust ${{ matrix.toolchain }}
+      - name: Test on Rust ${{ matrix.toolchain }} (only Windows)
+        if: matrix.os == 'windows-latest'
+        run: cargo test --verbose --no-default-features
+      - name: Test on Rust ${{ matrix.toolchain }} (non Windows)
+        if: matrix.os != 'windows-latest'
         run: cargo test --verbose --color always -- --nocapture

Cargo.lock

@@ -12,10 +12,16 @@ exclude = [".github/", ".cirrus.yml", "tests/",  "test_data/", "contrib/", "pypr
 [[bin]]
 name = "revaultd"
 path = "src/bin/daemon.rs"
+required-features = ["jsonrpc_server"]
 
 [[bin]]
 name = "revault-cli"
 path = "src/bin/cli.rs"
+required-features = ["jsonrpc_server"]
+
+[features]
+default = ["jsonrpc_server"]
+jsonrpc_server = ["jsonrpc-core", "jsonrpc-derive", "mio"]
 
 [dependencies]
 revault_tx = { git = "https://github.com/revault/revault_tx", features = ["use-serde"] }
@@ -54,9 +60,6 @@ rusqlite = { version = "0.26.3", features = ["bundled", "unlock_notify"] }
 libc = "0.2.80"
 
 # For the JSONRPC server
-jsonrpc-core = "15.1"
-jsonrpc-derive = "15.1"
-[target.'cfg(not(windows))'.dependencies]
-mio = { version = "0.7", features = ["default", "os-poll", "os-util", "uds"] }
-[target.'cfg(windows)'.dependencies]
-uds_windows = "1.0"
+jsonrpc-core = { version = "15.1", optional = true }
+jsonrpc-derive = { version = "15.1", optional = true }
+mio = { version = "0.7", features = ["default", "os-poll", "os-util", "uds"], optional = true }

Cargo.toml

@@ -9,10 +9,7 @@ use std::{
 
 use serde_json::Value as Json;
 
-#[cfg(not(windows))]
 use std::os::unix::net::UnixStream;
-#[cfg(windows)]
-use uds_windows::UnixStream;
 
 // Exits with error
 fn show_usage() {

src/bin/cli.rs

@@ -12,14 +12,8 @@ use std::{
     thread,
 };
 
-#[cfg(not(windows))]
 pub use mio::net::UnixListener;
-#[cfg(not(windows))]
 use mio::{net::UnixStream, Events, Interest, Poll, Token};
-#[cfg(windows)]
-pub use uds_windows::UnixListener;
-#[cfg(windows)]
-use uds_windows::UnixStream;
 
 use jsonrpc_core::{futures::Future, Call, MethodCall, Response};
 
@@ -62,10 +56,6 @@ fn read_bytes_from_stream(stream: &mut dyn io::Read) -> Result<Option<Vec<u8>>,
                 // done writing.
                 if total_read == buf.len() {
                     buf.resize(total_read * 2, 0);
-                } else {
-                    // But on windows, we do as we'll never receive a WouldBlock..
-                    #[cfg(windows)]
-                    return Ok(Some(trimmed(buf, total_read)));
                 }
             }
             Err(err) => {
@@ -131,7 +121,6 @@ fn write_byte_stream(stream: &mut UnixStream, resp: Vec<u8>) -> Result<Option<Ve
 
 // Used to check if, when receiving an event for a token, we have an ongoing connection and stream
 // for it.
-#[cfg(not(windows))]
 type ConnectionMap = HashMap<Token, (UnixStream, Arc<RwLock<VecDeque<Vec<u8>>>>)>;
 
 fn handle_single_request(
@@ -235,8 +224,7 @@ fn read_handle_request(
     Ok(())
 }
 
-// For all but Windows, we use Mio.
-#[cfg(not(windows))]
+// Our main polling loop
 fn mio_loop(
     mut listener: UnixListener,
     jsonrpc_io: jsonrpc_core::MetaIoHandler<JsonRpcMetaData>,
@@ -375,52 +363,6 @@ fn mio_loop(
     }
 }
 
-// For windows, we don't: Mio UDS support for Windows is not yet implemented.
-#[cfg(windows)]
-fn windows_loop(
-    listener: UnixListener,
-    jsonrpc_io: jsonrpc_core::MetaIoHandler<JsonRpcMetaData>,
-    metadata: JsonRpcMetaData,
-) -> Result<(), io::Error> {
-    for mut stream in listener.incoming() {
-        let mut stream = stream?;
-
-        // Ok, so we got something to read (we don't respond to garbage)
-        while let Some(bytes) = read_bytes_from_stream(&mut stream)? {
-            // Is it actually readable?
-            match String::from_utf8(bytes) {
-                Ok(string) => {
-                    // If it is and wants a response, write it directly
-                    if let Some(resp) = jsonrpc_io.handle_request_sync(&string, metadata.clone()) {
-                        let mut resp = Some(resp.into_bytes());
-                        loop {
-                            resp = write_byte_stream(&mut stream, resp.unwrap())?;
-                            if resp.is_none() {
-                                break;
-                            }
-                        }
-                    }
-                }
-                Err(e) => {
-                    log::error!(
-                        "JSONRPC server: error interpreting request: '{}'",
-                        e.to_string()
-                    );
-                }
-            }
-        }
-
-        // We can't loop until is_shutdown() as we block until we got a message.
-        // So, to handle shutdown the cleanest way is to check if the above handler
-        // just set shutdown.
-        if metadata.is_shutdown() {
-            break;
-        }
-    }
-
-    Ok(())
-}
-
 // Tries to bind to the socket, if we are told it's already in use try to connect
 // to check there is actually someone listening and it's not a leftover from a
 // crash.
@@ -448,11 +390,8 @@ fn bind(socket_path: PathBuf) -> Result<UnixListener, io::Error> {
 /// Bind to the UDS at `socket_path`
 pub fn rpcserver_setup(socket_path: PathBuf) -> Result<UnixListener, io::Error> {
     // Create the socket with RW permissions only for the user
-    // FIXME: find a workaround for Windows...
-    #[cfg(unix)]
     let old_umask = unsafe { libc::umask(0o177) };
     let listener = bind(socket_path);
-    #[cfg(unix)]
     unsafe {
         libc::umask(old_umask);
     }
@@ -470,10 +409,7 @@ pub fn rpcserver_loop(
     let metadata = JsonRpcMetaData::new(daemon_control);
 
     log::info!("JSONRPC server started.");
-    #[cfg(not(windows))]
     return mio_loop(listener, jsonrpc_io, metadata);
-    #[cfg(windows)]
-    return windows_loop(listener, jsonrpc_io, metadata);
 }
 
 #[cfg(test)]
@@ -488,10 +424,7 @@ mod tests {
         time::Duration,
     };
 
-    #[cfg(not(windows))]
     use std::os::unix::net::UnixStream;
-    #[cfg(windows)]
-    use uds_windows::UnixStream;
 
     // Redundant with functional tests but useful for testing the Windows loop
     // until the functional tests suite can run on it.
@@ -539,8 +472,6 @@ mod tests {
             )
         );
 
-        // TODO: support this for Windows..
-        #[cfg(not(windows))]
         {
             // Write valid JSONRPC message with a half-written one afterward
             let msg = String::from(

src/jsonrpc/server.rs

@@ -6,6 +6,7 @@ pub mod commands;
 mod communication;
 pub mod config;
 mod database;
+#[cfg(all(not(windows), feature = "jsonrpc_server"))]
 mod jsonrpc;
 mod revaultd;
 mod sigfetcher;
@@ -150,8 +151,8 @@ impl DaemonControl {
         self.sigfetcher_conn.shutdown();
     }
 
-    // TODO: make it optional at compile time
     /// Start and bind the server to the configured UNIX socket
+    #[cfg(all(not(windows), feature = "jsonrpc_server"))]
     pub fn rpc_server_setup(&self) -> Result<jsonrpc::server::UnixListener, io::Error> {
         let socket_file = self.revaultd.read().unwrap().rpc_socket_file();
         jsonrpc::server::rpcserver_setup(socket_file)
@@ -264,8 +265,8 @@ impl DaemonHandle {
             .expect("Joining sigfetcher thread");
     }
 
-    // TODO: make it optional at compilation time
     /// Start the JSONRPC server and listen for commands until we are stopped
+    #[cfg(all(not(windows), feature = "jsonrpc_server"))]
     pub fn rpc_server(&self) -> Result<(), io::Error> {
         log::info!("Starting JSONRPC server");