Merge pull request #1264 from tahoe-lafs/3936.client-can-disable-foolscap

Make the client respect the force_foolscap flag

Fixes ticket:3936

integration/util.py

@@ -367,6 +367,12 @@ def _create_node(reactor, request, temp_dir, introducer_furl, flog_gatherer, nam
                 'force_foolscap',
                 str(force_foolscap),
             )
+            set_config(
+                config,
+                'client',
+                'force_foolscap',
+                str(force_foolscap),
+            )
             write_config(FilePath(config_path), config)
         created_d.addCallback(created)
 

src/allmydata/client.py

@@ -60,7 +60,7 @@ from allmydata.interfaces import (
 )
 from allmydata.nodemaker import NodeMaker
 from allmydata.blacklist import Blacklist
-
+from allmydata.node import _Config
 
 KiB=1024
 MiB=1024*KiB
@@ -94,6 +94,7 @@ _client_config = configutil.ValidConfiguration(
             "shares.total",
             "shares._max_immutable_segment_size_for_testing",
             "storage.plugins",
+            "force_foolscap",
         ),
         "storage": (
             "debug_discard",
@@ -465,7 +466,7 @@ def create_introducer_clients(config, main_tub, _introducer_factory=None):
     return introducer_clients
 
 
-def create_storage_farm_broker(config, default_connection_handlers, foolscap_connection_handlers, tub_options, introducer_clients):
+def create_storage_farm_broker(config: _Config, default_connection_handlers, foolscap_connection_handlers, tub_options, introducer_clients):
     """
     Create a StorageFarmBroker object, for use by Uploader/Downloader
     (and everybody else who wants to use storage servers)

src/allmydata/storage_client.py

@@ -34,7 +34,7 @@ from __future__ import annotations
 
 from six import ensure_text
 
-from typing import Union
+from typing import Union, Any
 from os import urandom
 import re
 import time
@@ -89,6 +89,7 @@ from allmydata.storage.http_client import (
     ClientException as HTTPClientException, StorageClientMutables,
     ReadVector, TestWriteVectors, WriteVector, TestVector, ClientException
 )
+from .node import _Config
 
 ANONYMOUS_STORAGE_NURLS = "anonymous-storage-NURLs"
 
@@ -199,7 +200,7 @@ class StorageFarmBroker(service.MultiService):
             self,
             permute_peers,
             tub_maker,
-            node_config,
+            node_config: _Config,
             storage_client_config=None,
     ):
         service.MultiService.__init__(self)
@@ -218,9 +219,9 @@ class StorageFarmBroker(service.MultiService):
         # own Reconnector, and will give us a RemoteReference when we ask
         # them for it.
         self.servers = BytesKeyDict()
-        self._static_server_ids = set() # ignore announcements for these
+        self._static_server_ids : set[bytes] = set() # ignore announcements for these
         self.introducer_client = None
-        self._threshold_listeners = [] # tuples of (threshold, Deferred)
+        self._threshold_listeners : list[tuple[float,defer.Deferred[Any]]]= [] # tuples of (threshold, Deferred)
         self._connected_high_water_mark = 0
 
     @log_call(action_type=u"storage-client:broker:set-static-servers")
@@ -274,6 +275,16 @@ class StorageFarmBroker(service.MultiService):
             in self.storage_client_config.storage_plugins.items()
         })
 
+    @staticmethod
+    def _should_we_use_http(node_config: _Config, announcement: dict) -> bool:
+        """
+        Given an announcement dictionary and config, return whether we should
+        connect to storage server over HTTP.
+        """
+        return not node_config.get_config(
+            "client", "force_foolscap", default=True, boolean=True,
+        ) and len(announcement.get(ANONYMOUS_STORAGE_NURLS, [])) > 0
+
     @log_call(
         action_type=u"storage-client:broker:make-storage-server",
         include_args=["server_id"],
@@ -299,7 +310,7 @@ class StorageFarmBroker(service.MultiService):
             "pub-{}".format(str(server_id, "ascii")),  # server_id is v0-<key> not pub-v0-key .. for reasons?
         )
 
-        if len(server["ann"].get(ANONYMOUS_STORAGE_NURLS, [])) > 0:
+        if self._should_we_use_http(self.node_config, server["ann"]):
             s = HTTPNativeStorageServer(
                 server_id,
                 server["ann"],

src/allmydata/test/common_system.py

@@ -871,6 +871,7 @@ class SystemTestMixin(pollmixin.PollMixin, testutil.StallMixin):
 
         setnode("nickname", u"client %d \N{BLACK SMILING FACE}" % (which,))
         setconf(config, which, "storage", "force_foolscap", str(force_foolscap))
+        setconf(config, which, "client", "force_foolscap", str(force_foolscap))
 
         tub_location_hint, tub_port_endpoint = self.port_assigner.assign(reactor)
         setnode("tub.port", tub_port_endpoint)

src/allmydata/test/test_storage_client.py

@@ -94,10 +94,13 @@ from allmydata.storage_client import (
     StorageFarmBroker,
     _FoolscapStorage,
     _NullStorage,
+    ANONYMOUS_STORAGE_NURLS
 )
 from ..storage.server import (
     StorageServer,
 )
+from ..client import config_from_string
+
 from allmydata.interfaces import (
     IConnectionStatus,
     IStorageServer,
@@ -739,3 +742,42 @@ storage:
 
         yield done
         self.assertTrue(done.called)
+
+    def test_should_we_use_http_default(self):
+        """Default is to not use HTTP; this will change eventually"""
+        basedir = self.mktemp()
+        node_config = config_from_string(basedir, "", "")
+        announcement = {ANONYMOUS_STORAGE_NURLS: ["pb://..."]}
+        self.assertFalse(
+            StorageFarmBroker._should_we_use_http(node_config, announcement)
+        )
+        self.assertFalse(
+            StorageFarmBroker._should_we_use_http(node_config, {})
+        )
+
+    def test_should_we_use_http(self):
+        """
+        If HTTP is allowed, it will only be used if the announcement includes
+        some NURLs.
+        """
+        basedir = self.mktemp()
+
+        no_nurls = {}
+        empty_nurls = {ANONYMOUS_STORAGE_NURLS: []}
+        has_nurls = {ANONYMOUS_STORAGE_NURLS: ["pb://.."]}
+
+        for force_foolscap, announcement, expected_http_usage in [
+                ("false", no_nurls, False),
+                ("false", empty_nurls, False),
+                ("false", has_nurls, True),
+                ("true", empty_nurls, False),
+                ("true", no_nurls, False),
+                ("true", has_nurls, False),
+        ]:
+            node_config = config_from_string(
+                basedir, "", f"[client]\nforce_foolscap = {force_foolscap}"
+            )
+            self.assertEqual(
+                StorageFarmBroker._should_we_use_http(node_config, announcement),
+                expected_http_usage
+            )