Make If-None-Match:* work properly with 0-byte PUTs

When PUTting an object with `If-None-Match: *`, we rely 100-continue
support: the proxy checks the responses from all object-servers, and if
any of them respond 412, it closes down the connections. When there's
actual data for the object, this ensures that even nodes that *don't*
respond 412 will hit a ChunkReadTimeout and abort the PUT.

However, if the client does a PUT with a Content-Length of 0, that would
get sent all the way to the object server, which had all the information
it needed to respond 201. After replication, the PUT propagates to the
other nodes and the old object is lost, despite the client receiving a
412 indicating the operation failed.

Now, when PUTting a zero-byte object, switch to a chunked transfer so
the object-server still gets a ChunkReadTimeout.

Change-Id: Ie88e41aca2d59246c3134d743c1531c8e996f9e4

swift/proxy/controllers/obj.py

@@ -633,9 +633,12 @@ class BaseObjectController(Controller):
         pile = GreenPile(len(nodes))
 
         for nheaders in outgoing_headers:
-            # RFC2616:8.2.3 disallows 100-continue without a body
-            if (req.content_length > 0) or req.is_chunked:
-                nheaders['Expect'] = '100-continue'
+            # RFC2616:8.2.3 disallows 100-continue without a body,
+            # so switch to chunked request
+            if nheaders.get('Content-Length') == '0':
+                nheaders['Transfer-Encoding'] = 'chunked'
+                del nheaders['Content-Length']
+            nheaders['Expect'] = '100-continue'
             pile.spawn(self._connect_put_node, node_iter, partition,
                        req, nheaders, self.app.logger.thread_locals)
 
@@ -875,12 +878,13 @@ class ReplicatedObjectController(BaseObjectController):
                 logger=self.app.logger,
                 need_multiphase=False)
         else:
+            te = ',' + headers.get('Transfer-Encoding', '')
             putter = Putter.connect(
                 node, part, req.swift_entity_path, headers,
                 conn_timeout=self.app.conn_timeout,
                 node_timeout=self.app.node_timeout,
                 logger=self.app.logger,
-                chunked=req.is_chunked)
+                chunked=te.endswith(',chunked'))
         return putter
 
     def _transfer_data(self, req, data_source, putters, nodes):

test/probe/test_object_conditional_requests.py

@@ -0,0 +1,79 @@
+# Copyright (c) 2017 OpenStack Foundation
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import uuid
+
+from swift.common.manager import Manager
+from swiftclient import client
+
+from test.probe.brain import BrainSplitter
+from test.probe.common import ReplProbeTest
+
+
+def chunker(body):
+    '''Helper to ensure swiftclient sends a chunked request.'''
+    yield body
+
+
+class TestPutIfNoneMatchRepl(ReplProbeTest):
+    def setUp(self):
+        super(TestPutIfNoneMatchRepl, self).setUp()
+        self.container_name = 'container-%s' % uuid.uuid4()
+        self.object_name = 'object-%s' % uuid.uuid4()
+        self.brain = BrainSplitter(self.url, self.token, self.container_name,
+                                   self.object_name, 'object',
+                                   policy=self.policy)
+
+    def _do_test(self, overwrite_contents):
+        self.brain.put_container()
+        self.brain.stop_primary_half()
+        # put object to only 1 of 3 primaries
+        self.brain.put_object(contents='VERIFY')
+        self.brain.start_primary_half()
+
+        # Restart services and attempt to overwrite
+        with self.assertRaises(client.ClientException) as exc_mgr:
+            self.brain.put_object(headers={'If-None-Match': '*'},
+                                  contents=overwrite_contents)
+        self.assertEqual(exc_mgr.exception.http_status, 412)
+
+        # make sure we're GETting from the servers that missed the original PUT
+        self.brain.stop_handoff_half()
+
+        # verify the PUT did not complete
+        with self.assertRaises(client.ClientException) as exc_mgr:
+            client.get_object(
+                self.url, self.token, self.container_name, self.object_name)
+        self.assertEqual(exc_mgr.exception.http_status, 404)
+
+        # for completeness, run replicators...
+        Manager(['object-replicator']).once()
+
+        # ...and verify the object was not overwritten
+        _headers, body = client.get_object(
+            self.url, self.token, self.container_name, self.object_name)
+        self.assertEqual(body, 'VERIFY')
+
+    def test_content_length_nonzero(self):
+        self._do_test('OVERWRITE')
+
+    def test_content_length_zero(self):
+        self._do_test('')
+
+    def test_chunked(self):
+        self._do_test(chunker('OVERWRITE'))
+
+    def test_chunked_empty(self):
+        self._do_test(chunker(''))

test/unit/proxy/controllers/test_obj.py

@@ -1083,13 +1083,13 @@ class TestReplicatedObjController(BaseObjectControllerMixin,
         for connection_id, info in put_requests.items():
             body = ''.join(info['chunks'])
             headers = info['headers']
-            if chunked:
+            if chunked or not test_body:
                 body = unchunk_body(body)
                 self.assertEqual('100-continue', headers['Expect'])
                 self.assertEqual('chunked', headers['Transfer-Encoding'])
             else:
                 self.assertNotIn('Transfer-Encoding', headers)
-            if body:
+            if body or not test_body:
                 self.assertEqual('100-continue', headers['Expect'])
             else:
                 self.assertNotIn('Expect', headers)

test/unit/proxy/test_server.py

@@ -2584,9 +2584,12 @@ class TestReplicatedObjectController(
         def test_connect(ipaddr, port, device, partition, method, path,
                          headers=None, query_string=None):
             if path == '/a/c/o.jpg':
-                if 'expect' in headers or 'Expect' in headers:
-                    test_errors.append('Expect was in headers for object '
-                                       'server!')
+                if headers.get('Transfer-Encoding') != 'chunked':
+                    test_errors.append('"Transfer-Encoding: chunked" should '
+                                       'be in headers for object server!')
+                if 'Expect' not in headers:
+                    test_errors.append('Expect should be in headers for '
+                                       'object server!')
 
         with save_globals():
             controller = ReplicatedObjectController(