diff --git a/swift/common/internal_client.py b/swift/common/internal_client.py index 9db36cfd15..f861412cae 100644 --- a/swift/common/internal_client.py +++ b/swift/common/internal_client.py @@ -214,7 +214,10 @@ class InternalClient(object): :raises Exception: Exception is raised when code fails in an unexpected way. """ - + if isinstance(marker, unicode): + marker = marker.encode('utf8') + if isinstance(end_marker, unicode): + end_marker = end_marker.encode('utf8') while True: resp = self.make_request( 'GET', '%s?format=json&marker=%s&end_marker=%s' % @@ -227,7 +230,7 @@ class InternalClient(object): break for item in data: yield item - marker = data[-1]['name'] + marker = data[-1]['name'].encode('utf8') def make_path(self, account, container=None, obj=None): """ diff --git a/swift/obj/expirer.py b/swift/obj/expirer.py index cc4a1dd4eb..85bc29b5d7 100644 --- a/swift/obj/expirer.py +++ b/swift/obj/expirer.py @@ -15,7 +15,6 @@ from random import random from time import time -from urllib import quote from os.path import join from eventlet import sleep, Timeout @@ -26,11 +25,6 @@ from swift.common.utils import get_logger, dump_recon_cache from swift.common.http import HTTP_NOT_FOUND, HTTP_CONFLICT, \ HTTP_PRECONDITION_FAILED -try: - import simplejson as json -except ImportError: - import json - class ObjectExpirer(Daemon): """ @@ -104,7 +98,7 @@ class ObjectExpirer(Daemon): break for o in self.swift.iter_objects(self.expiring_objects_account, container): - obj = o['name'] + obj = o['name'].encode('utf8') timestamp, actual_obj = obj.split('-', 1) timestamp = int(timestamp) if timestamp > int(time()): @@ -168,6 +162,6 @@ class ObjectExpirer(Daemon): :param timestamp: The timestamp the X-Delete-At value must match to perform the actual delete. """ - self.swift.make_request('DELETE', '/v1/%s' % (quote(actual_obj),), + self.swift.make_request('DELETE', '/v1/%s' % actual_obj.lstrip('/'), {'X-If-Delete-At': str(timestamp)}, (2, HTTP_NOT_FOUND, HTTP_PRECONDITION_FAILED)) diff --git a/test/unit/common/test_internal_client.py b/test/unit/common/test_internal_client.py index 89bb6642ad..e3aa6f96d0 100644 --- a/test/unit/common/test_internal_client.py +++ b/test/unit/common/test_internal_client.py @@ -484,12 +484,12 @@ class TestInternalClient(unittest.TestCase): paths = [ '/?format=json&marker=start&end_marker=end', - '/?format=json&marker=one&end_marker=end', + '/?format=json&marker=one%C3%A9&end_marker=end', '/?format=json&marker=two&end_marker=end', ] responses = [ - Response(200, json.dumps([{'name': 'one'}, ])), + Response(200, json.dumps([{'name': 'one\xc3\xa9'}, ])), Response(200, json.dumps([{'name': 'two'}, ])), Response(204, ''), ] @@ -497,9 +497,9 @@ class TestInternalClient(unittest.TestCase): items = [] client = InternalClient(self, paths, responses) for item in client._iter_items('/', marker='start', end_marker='end'): - items.append(item['name']) + items.append(item['name'].encode('utf8')) - self.assertEquals('one two'.split(), items) + self.assertEquals('one\xc3\xa9 two'.split(), items) def test_set_metadata(self): class InternalClient(internal_client.InternalClient): diff --git a/test/unit/obj/test_expirer.py b/test/unit/obj/test_expirer.py index d364a29596..bcd3974e91 100644 --- a/test/unit/obj/test_expirer.py +++ b/test/unit/obj/test_expirer.py @@ -285,6 +285,47 @@ class TestObjectExpirer(TestCase): '2 possible objects',), {}), (('Pass completed in 0s; 1 objects expired',), {})]) + def test_delete_actual_object_does_not_get_unicode(self): + class InternalClient(object): + def __init__(self, containers, objects): + self.containers = containers + self.objects = objects + + def get_account_info(*a, **kw): + return 1, 2 + + def iter_containers(self, *a, **kw): + return self.containers + + def delete_container(*a, **kw): + pass + + def delete_object(*a, **kw): + pass + + def iter_objects(self, *a, **kw): + return self.objects + + got_unicode = [False] + + def delete_actual_object_test_for_unicode(actual_obj, timestamp): + if isinstance(actual_obj, unicode): + got_unicode[0] = True + + x = expirer.ObjectExpirer({}) + x.logger = FakeLogger() + x.delete_actual_object = delete_actual_object_test_for_unicode + self.assertEquals(x.report_objects, 0) + x.swift = InternalClient([{'name': str(int(time() - 86400))}], + [{'name': u'%d-actual-obj' % int(time() - 86400)}]) + x.run_once() + self.assertEquals(x.report_objects, 1) + self.assertEquals(x.logger.log_dict['info'], + [(('Pass beginning; 1 possible containers; ' + '2 possible objects',), {}), + (('Pass completed in 0s; 1 objects expired',), {})]) + self.assertFalse(got_unicode[0]) + def test_failed_delete_continues_on(self): class InternalClient(object): def __init__(self, containers, objects): @@ -416,6 +457,24 @@ class TestObjectExpirer(TestCase): x.delete_actual_object('/path/to/object', ts) self.assertEquals(got_env[0]['HTTP_X_IF_DELETE_AT'], ts) + def test_delete_actual_object_nourlquoting(self): + # delete_actual_object should not do its own url quoting because + # internal client's make_request handles that. + got_env = [None] + + def fake_app(env, start_response): + got_env[0] = env + start_response('204 No Content', [('Content-Length', '0')]) + return [] + + internal_client.loadapp = lambda x: fake_app + + x = expirer.ObjectExpirer({}) + ts = '1234' + x.delete_actual_object('/path/to/object name', ts) + self.assertEquals(got_env[0]['HTTP_X_IF_DELETE_AT'], ts) + self.assertEquals(got_env[0]['PATH_INFO'], '/v1/path/to/object name') + def test_delete_actual_object_handles_404(self): def fake_app(env, start_response):