From acb742ac1fa2c233aedb1af97a0dd3ab1b5a201d Mon Sep 17 00:00:00 2001
From: Tim Burke <tim.burke@gmail.com>
Date: Wed, 21 Oct 2020 08:25:48 -0700
Subject: [PATCH] py3: Fix status comparison in ECGetResponseCollection

The initial dummy bad bucket was previously stored using None as a
key. In some circumstances this None key might be compared with
integer response status, which is unsupported in py3.

Move away from having any sort of default value and instead return a
default dummy bucket if we don't have any responses or only have 404
responses.

Co-Authored-By: Alistair Coles <alistairncoles@gmail.com>
Change-Id: I6a2ec3739ac92affb0f3dac248f26a1fc4ac9493
Closes-Bug: #1900770
(cherry picked from commit a5fa3cfcafbc6a441139b6e9bccc06bf38042d64)
---
 swift/proxy/controllers/obj.py          |  6 +++++-
 test/unit/proxy/controllers/test_obj.py | 27 +++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/swift/proxy/controllers/obj.py b/swift/proxy/controllers/obj.py
index cd924b0c61..f0dbc0830a 100644
--- a/swift/proxy/controllers/obj.py
+++ b/swift/proxy/controllers/obj.py
@@ -2118,7 +2118,8 @@ def __init__(self, policy):
         """
         self.policy = policy
         self.buckets = {}
-        self.bad_buckets = {None: ECGetResponseBucket(self.policy, None)}
+        self.default_bad_bucket = ECGetResponseBucket(self.policy, None)
+        self.bad_buckets = {}
         self.node_iter_count = 0
 
     def _get_bucket(self, timestamp):
@@ -2247,6 +2248,9 @@ def least_bad_bucket(self):
         """
         Return the bad_bucket with the smallest shortfall
         """
+        if all(status == 404 for status in self.bad_buckets):
+            # NB: also covers an empty self.bad_buckets
+            return self.default_bad_bucket
         # we want "enough" 416s to prevent "extra" requests - but we keep
         # digging on 404s
         short, status = min((bucket.shortfall, status)
diff --git a/test/unit/proxy/controllers/test_obj.py b/test/unit/proxy/controllers/test_obj.py
index 3f73dd8d5a..cdc6fe22a0 100644
--- a/test/unit/proxy/controllers/test_obj.py
+++ b/test/unit/proxy/controllers/test_obj.py
@@ -3949,6 +3949,33 @@ def get_response(req):
         self.assertEqual(resp.status_int, 416)
         self.assertEqual(len(log), 2 * self.replicas())
 
+    @patch_policies(
+        [ECStoragePolicy(0, name='ec', is_default=True,
+                         ec_type=DEFAULT_TEST_EC_TYPE, ec_ndata=4,
+                         ec_nparity=4, ec_segment_size=4096)],
+        fake_ring_args=[{'replicas': 8}]
+    )
+    def test_GET_ndata_equals_nparity_with_missing_and_errors(self):
+        # when ec_ndata == ec_nparity it is possible for the shortfall of a bad
+        # bucket (412's) to equal ec_ndata; verify that the 412 bucket is still
+        # chosen ahead of the initial 'dummy' bad bucket
+        POLICIES.default.object_ring.max_more_nodes = 8
+        responses = [
+            StubResponse(412, frag_index=0),
+            StubResponse(412, frag_index=1),
+        ]
+
+        def get_response(req):
+            return responses.pop(0) if responses else StubResponse(404)
+
+        req = swob.Request.blank('/v1/a/c/o', headers={
+            'Range': 'bytes=%s-' % 100000000000000})
+        with capture_http_requests(get_response) as log:
+            resp = req.get_response(self.app)
+
+        self.assertEqual(resp.status_int, 412)
+        self.assertEqual(len(log), 2 * 8)
+
     def test_GET_with_success_and_507_will_503(self):
         responses = [  # only 9 good nodes
             StubResponse(200),