Treat non-redirect 304 responses as API status errors

Author: corsicanec82

lib/docker_engine_ruby/internal/transport/base_client.rb

@@ -11,6 +11,7 @@ class BaseClient
 
         # from whatwg fetch spec
         MAX_REDIRECTS = 20
+        REDIRECT_STATUSES = [301, 302, 303, 307, 308].freeze
 
         # rubocop:disable Style/MutableConstant
         PLATFORM_HEADERS =
@@ -67,6 +68,15 @@ def should_retry?(status, headers:)
             end
           end
 
+          # @api private
+          #
+          # @param status [Integer]
+          #
+          # @return [Boolean]
+          def redirect_status?(status)
+            REDIRECT_STATUSES.include?(status)
+          end
+
           # @api private
           #
           # @param request [Hash{Symbol=>Object}] .
@@ -393,7 +403,8 @@ def send_request(request, redirect_count:, retry_count:, send_retry_header:)
           case status
           in ..299
             [status, response, stream]
-          in 300..399 if redirect_count >= self.class::MAX_REDIRECTS
+          in Integer => status if self.class.redirect_status?(status) &&
+            redirect_count >= self.class::MAX_REDIRECTS
             self.class.reap_connection!(status, stream: stream)
 
             message = "Failed to complete the request within #{self.class::MAX_REDIRECTS} redirects."
@@ -402,7 +413,7 @@ def send_request(request, redirect_count:, retry_count:, send_retry_header:)
               response: response,
               message: message
             )
-          in 300..399
+          in Integer => status if self.class.redirect_status?(status)
             self.class.reap_connection!(status, stream: stream)
 
             request = self.class.follow_redirect(request, status: status, response_headers: headers)
@@ -414,7 +425,7 @@ def send_request(request, redirect_count:, retry_count:, send_retry_header:)
             )
           in DockerEngineRuby::Errors::APIConnectionError if retry_count >= max_retries
             raise status
-          in (400..) if retry_count >= max_retries || !self.class.should_retry?(status, headers: headers)
+          in (300..) if retry_count >= max_retries || !self.class.should_retry?(status, headers: headers)
             decoded = Kernel.then do
               DockerEngineRuby::Internal::Util.decode_content(headers, stream: stream, suppress_error: true)
             ensure

test/docker_engine_ruby/client_test.rb

@@ -342,6 +342,18 @@ def test_images_pull_with_json_lines_response_returns_nil
     assert_nil(response)
   end
 
+  def test_non_redirect_304_is_treated_as_status_error
+    stub_request(:post, "http://localhost/containers/id/start").to_return(status: 304, body: "")
+
+    docker = DockerEngineRuby::Client.new(base_url: "http://localhost")
+
+    error = assert_raises(DockerEngineRuby::Errors::APIStatusError) do
+      docker.containers.start("id")
+    end
+
+    assert_equal(304, error.status)
+  end
+
   def test_default_headers
     stub_request(:get, "http://localhost/containers/json").to_return_json(status: 200, body: {})