Merge pull request #166 from jrdnbradford/rsession-socket

Make unix socket default

Author: yuvipanda

README.md

@@ -39,14 +39,14 @@ conda install -c conda-forge jupyter-rsession-proxy
 
 ### Multiuser Considerations
 
-This extension launches an rstudio server process from the jupyter notebook server. This is fine in JupyterHub deployments where user servers are containerized since other users cannot connect to the rstudio server port. In non-containerized JupyterHub deployments, for example on multiuser systems running LocalSpawner or BatchSpawner, this not secure. Any user may connect to rstudio server and run arbitrary code.
+This extension launches an RStudio server process from the Jupyter notebook server. This is fine in JupyterHub deployments where user servers are containerized since other users cannot connect to the RStudio server port. In non-containerized JupyterHub deployments, for example on multiuser systems running LocalSpawner or BatchSpawner, this not secure. Any user may connect to Rstudio server and run arbitrary code.
 
 ## Configuration with Environment Variables
-The following behavior can be configured with environment variables
+The following behavior can be configured with environment variables:
 
 | Environment Variable                      | Effect                                                                | Default Value       | Notes                                                                     |
 |-------------------------------------------|-----------------------------------------------------------------------|---------------------|---------------------------------------------------------------------------|
-| `JUPYTER_RSESSION_PROXY_USE_SOCKET`       | Use unix sockets instead of TCP sockets                               | `no`                | Anything other than case insensitive `no` or `false` will use unix socket |
+| `JUPYTER_RSESSION_PROXY_USE_SOCKET`       | Whether to use unix socket                                            | `true`                | By default a unix socket is used. If set to case-insensitive `no` or `false` it will switch to using a TCP socket |
 | `JUPYTER_RSESSION_PROXY_WWW_FRAME_ORIGIN` | The value of the `www-frame-origin` flag to rserver                   | `same`              |                                                                           |
 | `RSERVER_TIMEOUT`                         | Idle timeout flag to rserver in minutes                               | `15`                | Must be numeric and positive                                              |
 | `RSESSION_TIMEOUT`                        | Idle timeout flag to rsession in minutes                              | `15`                | Must be numeric and positive                                              |

jupyter_rsession_proxy/__init__.py

@@ -1,6 +1,5 @@
 import getpass
 import os
-import pathlib
 import shutil
 import subprocess
 import tempfile
@@ -42,7 +41,7 @@ def rewrite_netloc(response, request):
             u = urlparse(v)
             redirect_path = u.path
             if u.path.startswith("../"):
-                # R Help server sometimes responds with relative locations which 
+                # R Help server sometimes responds with relative locations which
                 # need to be handled if changing the host part of the header.
                 redirect_path = urljoin(request.path, u.path)
             if u.netloc != request.host:
@@ -132,11 +131,8 @@ def _get_cmd(port, unix_socket):
                 print("Invalid value for JUPYTER_RSESSION_PROXY_THREAD_POOL_SIZE. Must be an integer.")
                 pass
 
-        if unix_socket != "":
-            if supported_args['www-socket']:
-                cmd.append('--www-socket={unix_socket}')
-            else:
-                raise NotImplementedError(f'rstudio-server does not support requested socket connection')
+        if unix_socket and supported_args['www-socket']:
+            cmd.append('--www-socket={unix_socket}')
         else:
             cmd.append('--www-port={port}')
 
@@ -159,14 +155,15 @@ def _get_timeout(default=15):
         }
     }
 
-    use_socket = os.getenv('JUPYTER_RSESSION_PROXY_USE_SOCKET')
-    if use_socket is not None:
-        # If this env var is anything other than case insensitive 'no' or 'false',
-        # use unix sockets instead of tcp sockets. This allows us to default to
-        # using unix sockets by default in the future once this feature is better
-        # tested, and allow people to turn it off if needed.
-        if use_socket.casefold() not in ('no', 'false'):
-            server_process['unix_socket'] = True
+    # The environment variable serves to explicitly disable Unix sockets, which are the default
+    use_unix_socket = True
+    disable_socket = os.getenv('JUPYTER_RSESSION_PROXY_USE_SOCKET')
+    if disable_socket is not None:
+        if disable_socket.casefold() in ('no', 'false'):
+            use_unix_socket = False
+
+    if use_unix_socket:
+        server_process['unix_socket'] = True
 
     return server_process