diff --git a/swift/common/middleware/dlo.py b/swift/common/middleware/dlo.py index 491e8324dd..56673af2bb 100644 --- a/swift/common/middleware/dlo.py +++ b/swift/common/middleware/dlo.py @@ -21,9 +21,10 @@ from swift.common.exceptions import ListingIterError from swift.common.http import is_success from swift.common.swob import Request, Response, \ HTTPRequestedRangeNotSatisfiable, HTTPBadRequest -from swift.common.utils import get_logger, json, SegmentedIterable, \ +from swift.common.utils import get_logger, json, \ RateLimitedIterator, read_conf_dir, quote -from swift.common.wsgi import WSGIContext +from swift.common.request_helpers import SegmentedIterable +from swift.common.wsgi import WSGIContext, make_request from urllib import unquote @@ -35,13 +36,12 @@ class GetContext(WSGIContext): def _get_container_listing(self, req, version, account, container, prefix, marker=''): - con_req = req.copy_get() - con_req.script_name = '' - con_req.environ['swift.source'] = 'DLO' - con_req.range = None - con_req.path_info = '/'.join(['', version, account, container]) + con_req = make_request( + req.environ, path='/'.join(['', version, account, container]), + method='GET', + headers={'x-auth-token': req.headers.get('x-auth-token')}, + agent=('%(orig)s ' + 'DLO MultipartGET'), swift_source='DLO') con_req.query_string = 'format=json&prefix=%s' % quote(prefix) - con_req.user_agent = '%s DLO MultipartGET' % con_req.user_agent if marker: con_req.query_string += '&marker=%s' % quote(marker) diff --git a/swift/common/middleware/slo.py b/swift/common/middleware/slo.py index 6086fcb2c9..9c45aa0b4a 100644 --- a/swift/common/middleware/slo.py +++ b/swift/common/middleware/slo.py @@ -146,11 +146,12 @@ from swift.common.swob import Request, HTTPBadRequest, HTTPServerError, \ HTTPUnauthorized, HTTPRequestedRangeNotSatisfiable, Response from swift.common.utils import json, get_logger, config_true_value, \ get_valid_utf8_str, override_bytes_from_content_type, split_path, \ - register_swift_info, RateLimitedIterator, SegmentedIterable, \ - closing_if_possible, close_if_possible, quote + register_swift_info, RateLimitedIterator, quote +from swift.common.request_helpers import SegmentedIterable, \ + closing_if_possible, close_if_possible from swift.common.constraints import check_utf8, MAX_BUFFERED_SLO_SEGMENTS from swift.common.http import HTTP_NOT_FOUND, HTTP_UNAUTHORIZED, is_success -from swift.common.wsgi import WSGIContext +from swift.common.wsgi import WSGIContext, make_request from swift.common.middleware.bulk import get_response_body, \ ACCEPTABLE_FORMATS, Bulk @@ -215,11 +216,11 @@ class SloGetContext(WSGIContext): Fetch the submanifest, parse it, and return it. Raise exception on failures. """ - sub_req = req.copy_get() - sub_req.range = None - sub_req.environ['PATH_INFO'] = '/'.join(['', version, acc, con, obj]) - sub_req.environ['swift.source'] = 'SLO' - sub_req.user_agent = "%s SLO MultipartGET" % sub_req.user_agent + sub_req = make_request( + req.environ, path='/'.join(['', version, acc, con, obj]), + method='GET', + headers={'x-auth-token': req.headers.get('x-auth-token')}, + agent=('%(orig)s ' + 'SLO MultipartGET'), swift_source='SLO') sub_resp = sub_req.get_response(self.slo.app) if not is_success(sub_resp.status_int): @@ -310,7 +311,18 @@ class SloGetContext(WSGIContext): if req.method == 'HEAD': return True - if req.range and self._response_status[:3] in ("206", "416"): + response_status = int(self._response_status[:3]) + + # These are based on etag, and the SLO's etag is almost certainly not + # the manifest object's etag. Still, it's highly likely that the + # submitted If-None-Match won't match the manifest object's etag, so + # we can avoid re-fetching the manifest if we got a successful + # response. + if ((req.if_match or req.if_none_match) and + not is_success(response_status)): + return True + + if req.range and response_status in (206, 416): content_range = '' for header, value in self._response_headers: if header.lower() == 'content-range': @@ -373,10 +385,10 @@ class SloGetContext(WSGIContext): close_if_possible(resp_iter) del req.environ['swift.non_client_disconnect'] - get_req = req.copy_get() - get_req.range = None - get_req.environ['swift.source'] = 'SLO' - get_req.user_agent = "%s SLO MultipartGET" % get_req.user_agent + get_req = make_request( + req.environ, method='GET', + headers={'x-auth-token': req.headers.get('x-auth-token')}, + agent=('%(orig)s ' + 'SLO MultipartGET'), swift_source='SLO') resp_iter = self._app_call(get_req.environ) # Any Content-Range from a manifest is almost certainly wrong for the @@ -417,7 +429,8 @@ class SloGetContext(WSGIContext): req, content_length, response_headers, segments) def _manifest_head_response(self, req, response_headers): - return HTTPOk(request=req, headers=response_headers, body='') + return HTTPOk(request=req, headers=response_headers, body='', + conditional_response=True) def _manifest_get_response(self, req, content_length, response_headers, segments): diff --git a/swift/common/request_helpers.py b/swift/common/request_helpers.py index 255dbf1790..a191096206 100644 --- a/swift/common/request_helpers.py +++ b/swift/common/request_helpers.py @@ -20,10 +20,16 @@ Why not swift.common.utils, you ask? Because this way we can import things from swob in here without creating circular imports. """ +import sys +import time +from contextlib import contextmanager +from urllib import unquote from swift.common.constraints import FORMAT2CONTENT_TYPE +from swift.common.exceptions import ListingIterError, SegmentError +from swift.common.http import is_success, HTTP_SERVICE_UNAVAILABLE from swift.common.swob import HTTPBadRequest, HTTPNotAcceptable from swift.common.utils import split_path, validate_device_partition -from urllib import unquote +from swift.common.wsgi import make_request def get_param(req, name, default=None): @@ -194,3 +200,146 @@ def remove_items(headers, condition): keys = filter(condition, headers) removed.update((key, headers.pop(key)) for key in keys) return removed + + +def close_if_possible(maybe_closable): + close_method = getattr(maybe_closable, 'close', None) + if callable(close_method): + return close_method() + + +@contextmanager +def closing_if_possible(maybe_closable): + """ + Like contextlib.closing(), but doesn't crash if the object lacks a close() + method. + + PEP 333 (WSGI) says: "If the iterable returned by the application has a + close() method, the server or gateway must call that method upon + completion of the current request[.]" This function makes that easier. + """ + yield maybe_closable + close_if_possible(maybe_closable) + + +class SegmentedIterable(object): + """ + Iterable that returns the object contents for a large object. + + :param req: original request object + :param app: WSGI application from which segments will come + :param listing_iter: iterable yielding the object segments to fetch, + along with the byte subranges to fetch, in the + form of a tuple (object-path, first-byte, last-byte) + or (object-path, None, None) to fetch the whole thing. + :param max_get_time: maximum permitted duration of a GET request (seconds) + :param logger: logger object + :param swift_source: value of swift.source in subrequest environ + (just for logging) + :param ua_suffix: string to append to user-agent. + :param name: name of manifest (used in logging only) + :param response: optional response object for the response being sent + to the client. Only affects logs. + """ + def __init__(self, req, app, listing_iter, max_get_time, + logger, ua_suffix, swift_source, + name='', response=None): + self.req = req + self.app = app + self.listing_iter = listing_iter + self.max_get_time = max_get_time + self.logger = logger + self.ua_suffix = " " + ua_suffix + self.swift_source = swift_source + self.name = name + self.response = response + + def app_iter_range(self, *a, **kw): + """ + swob.Response will only respond with a 206 status in certain cases; one + of those is if the body iterator responds to .app_iter_range(). + + However, this object (or really, its listing iter) is smart enough to + handle the range stuff internally, so we just no-op this out for swob. + """ + return self + + def __iter__(self): + start_time = time.time() + have_yielded_data = False + try: + for seg_path, seg_etag, seg_size, first_byte, last_byte \ + in self.listing_iter: + if time.time() - start_time > self.max_get_time: + raise SegmentError( + 'ERROR: While processing manifest %s, ' + 'max LO GET time of %ds exceeded' % + (self.name, self.max_get_time)) + seg_req = make_request( + self.req.environ, path=seg_path, method='GET', + headers={'x-auth-token': self.req.headers.get( + 'x-auth-token')}, + agent=('%(orig)s ' + self.ua_suffix), + swift_source=self.swift_source) + if first_byte is not None or last_byte is not None: + seg_req.headers['Range'] = "bytes=%s-%s" % ( + # The 0 is to avoid having a range like "bytes=-10", + # which actually means the *last* 10 bytes. + '0' if first_byte is None else first_byte, + '' if last_byte is None else last_byte) + + seg_resp = seg_req.get_response(self.app) + if not is_success(seg_resp.status_int): + close_if_possible(seg_resp.app_iter) + raise SegmentError( + 'ERROR: While processing manifest %s, ' + 'got %d while retrieving %s' % + (self.name, seg_resp.status_int, seg_path)) + + elif ((seg_etag and (seg_resp.etag != seg_etag)) or + (seg_size and (seg_resp.content_length != seg_size) and + not seg_req.range)): + # The content-length check is for security reasons. Seems + # possible that an attacker could upload a >1mb object and + # then replace it with a much smaller object with same + # etag. Then create a big nested SLO that calls that + # object many times which would hammer our obj servers. If + # this is a range request, don't check content-length + # because it won't match. + close_if_possible(seg_resp.app_iter) + raise SegmentError( + 'Object segment no longer valid: ' + '%(path)s etag: %(r_etag)s != %(s_etag)s or ' + '%(r_size)s != %(s_size)s.' % + {'path': seg_req.path, 'r_etag': seg_resp.etag, + 'r_size': seg_resp.content_length, + 's_etag': seg_etag, + 's_size': seg_size}) + + for chunk in seg_resp.app_iter: + yield chunk + have_yielded_data = True + close_if_possible(seg_resp.app_iter) + except ListingIterError as err: + # I have to save this error because yielding the ' ' below clears + # the exception from the current stack frame. + excinfo = sys.exc_info() + self.logger.exception('ERROR: While processing manifest %s, %s', + self.name, err) + # Normally, exceptions before any data has been yielded will + # cause Eventlet to send a 5xx response. In this particular + # case of ListingIterError we don't want that and we'd rather + # just send the normal 2xx response and then hang up early + # since 5xx codes are often used to judge Service Level + # Agreements and this ListingIterError indicates the user has + # created an invalid condition. + if not have_yielded_data: + yield ' ' + raise excinfo + except SegmentError as err: + self.logger.exception(err) + # This doesn't actually change the response status (we're too + # late for that), but this does make it to the logs. + if self.response: + self.response.status = HTTP_SERVICE_UNAVAILABLE + raise diff --git a/swift/common/swob.py b/swift/common/swob.py index 725271239c..ba3b54bb7c 100644 --- a/swift/common/swob.py +++ b/swift/common/swob.py @@ -591,7 +591,7 @@ class Range(object): class Match(object): """ - Wraps a Request's If-None-Match header as a friendly object. + Wraps a Request's If-[None-]Match header as a friendly object. :param headerval: value of the header as a str """ @@ -757,7 +757,7 @@ class Request(object): remote_user = _req_environ_property('REMOTE_USER') user_agent = _req_environ_property('HTTP_USER_AGENT') query_string = _req_environ_property('QUERY_STRING') - if_match = _req_environ_property('HTTP_IF_MATCH') + if_match = _req_fancy_property(Match, 'if-match') body_file = _req_environ_property('wsgi.input') content_length = _header_int_property('content-length') if_modified_since = _datetime_property('if-modified-since') @@ -1097,9 +1097,33 @@ class Response(object): return content_size, content_type def _response_iter(self, app_iter, body): + if self.conditional_response and self.request: + if self.etag and self.request.if_none_match and \ + self.etag in self.request.if_none_match: + self.status = 304 + self.content_length = 0 + return [''] + + if self.etag and self.request.if_match and \ + self.etag not in self.request.if_match: + self.status = 412 + self.content_length = 0 + return [''] + + if self.status_int == 404 and self.request.if_match \ + and '*' in self.request.if_match: + # If none of the entity tags match, or if "*" is given and no + # current entity exists, the server MUST NOT perform the + # requested method, and MUST return a 412 (Precondition + # Failed) response. [RFC 2616 section 14.24] + self.status = 412 + self.content_length = 0 + return [''] + if self.request and self.request.method == 'HEAD': # We explicitly do NOT want to set self.content_length to 0 here return [''] + if self.conditional_response and self.request and \ self.request.range and self.request.range.ranges and \ not self.content_range: diff --git a/swift/common/utils.py b/swift/common/utils.py index 6186422ce4..8860512427 100644 --- a/swift/common/utils.py +++ b/swift/common/utils.py @@ -61,10 +61,8 @@ utf8_decoder = codecs.getdecoder('utf-8') utf8_encoder = codecs.getencoder('utf-8') from swift import gettext_ as _ -from swift.common.exceptions import LockTimeout, MessageTimeout, \ - ListingIterError, SegmentError -from swift.common.http import is_success, is_redirection, HTTP_NOT_FOUND, \ - HTTP_SERVICE_UNAVAILABLE +from swift.common.exceptions import LockTimeout, MessageTimeout +from swift.common.http import is_success, is_redirection, HTTP_NOT_FOUND # logging doesn't import patched as cleanly as one would like from logging.handlers import SysLogHandler @@ -2559,146 +2557,3 @@ def quote(value, safe='/'): Patched version of urllib.quote that encodes utf-8 strings before quoting """ return _quote(get_valid_utf8_str(value), safe) - - -def close_if_possible(maybe_closable): - close_method = getattr(maybe_closable, 'close', None) - if callable(close_method): - return close_method() - - -@contextmanager -def closing_if_possible(maybe_closable): - """ - Like contextlib.closing(), but doesn't crash if the object lacks a close() - method. - - PEP 333 (WSGI) says: "If the iterable returned by the application has a - close() method, the server or gateway must call that method upon - completion of the current request[.]" This function makes that easier. - """ - yield maybe_closable - close_if_possible(maybe_closable) - - -class SegmentedIterable(object): - """ - Iterable that returns the object contents for a large object. - - :param req: original request object - :param app: WSGI application from which segments will come - :param listing_iter: iterable yielding the object segments to fetch, - along with the byte subranges to fetch, in the - form of a tuple (object-path, first-byte, last-byte) - or (object-path, None, None) to fetch the whole thing. - :param max_get_time: maximum permitted duration of a GET request (seconds) - :param logger: logger object - :param swift_source: value of swift.source in subrequest environ - (just for logging) - :param ua_suffix: string to append to user-agent. - :param name: name of manifest (used in logging only) - :param response: optional response object for the response being sent - to the client. Only affects logs. - """ - def __init__(self, req, app, listing_iter, max_get_time, - logger, ua_suffix, swift_source, - name='', response=None): - self.req = req - self.app = app - self.listing_iter = listing_iter - self.max_get_time = max_get_time - self.logger = logger - self.ua_suffix = " " + ua_suffix - self.swift_source = swift_source - self.name = name - self.response = response - - def app_iter_range(self, *a, **kw): - """ - swob.Response will only respond with a 206 status in certain cases; one - of those is if the body iterator responds to .app_iter_range(). - - However, this object (or really, its listing iter) is smart enough to - handle the range stuff internally, so we just no-op this out for swob. - """ - return self - - def __iter__(self): - start_time = time.time() - have_yielded_data = False - try: - for seg_path, seg_etag, seg_size, first_byte, last_byte \ - in self.listing_iter: - if time.time() - start_time > self.max_get_time: - raise SegmentError( - 'ERROR: While processing manifest %s, ' - 'max LO GET time of %ds exceeded' % - (self.name, self.max_get_time)) - seg_req = self.req.copy_get() - seg_req.range = None - seg_req.environ['PATH_INFO'] = seg_path - seg_req.environ['swift.source'] = self.swift_source - seg_req.user_agent = "%s %s" % (seg_req.user_agent, - self.ua_suffix) - if first_byte is not None or last_byte is not None: - seg_req.headers['Range'] = "bytes=%s-%s" % ( - # The 0 is to avoid having a range like "bytes=-10", - # which actually means the *last* 10 bytes. - '0' if first_byte is None else first_byte, - '' if last_byte is None else last_byte) - - seg_resp = seg_req.get_response(self.app) - if not is_success(seg_resp.status_int): - close_if_possible(seg_resp.app_iter) - raise SegmentError( - 'ERROR: While processing manifest %s, ' - 'got %d while retrieving %s' % - (self.name, seg_resp.status_int, seg_path)) - - elif ((seg_etag and (seg_resp.etag != seg_etag)) or - (seg_size and (seg_resp.content_length != seg_size) and - not seg_req.range)): - # The content-length check is for security reasons. Seems - # possible that an attacker could upload a >1mb object and - # then replace it with a much smaller object with same - # etag. Then create a big nested SLO that calls that - # object many times which would hammer our obj servers. If - # this is a range request, don't check content-length - # because it won't match. - close_if_possible(seg_resp.app_iter) - raise SegmentError( - 'Object segment no longer valid: ' - '%(path)s etag: %(r_etag)s != %(s_etag)s or ' - '%(r_size)s != %(s_size)s.' % - {'path': seg_req.path, 'r_etag': seg_resp.etag, - 'r_size': seg_resp.content_length, - 's_etag': seg_etag, - 's_size': seg_size}) - - for chunk in seg_resp.app_iter: - yield chunk - have_yielded_data = True - close_if_possible(seg_resp.app_iter) - except ListingIterError as err: - # I have to save this error because yielding the ' ' below clears - # the exception from the current stack frame. - excinfo = sys.exc_info() - self.logger.exception('ERROR: While processing manifest %s, %s', - self.name, err) - # Normally, exceptions before any data has been yielded will - # cause Eventlet to send a 5xx response. In this particular - # case of ListingIterError we don't want that and we'd rather - # just send the normal 2xx response and then hang up early - # since 5xx codes are often used to judge Service Level - # Agreements and this ListingIterError indicates the user has - # created an invalid condition. - if not have_yielded_data: - yield ' ' - raise excinfo - except SegmentError as err: - self.logger.exception(err) - # This doesn't actually change the response status (we're too - # late for that), but this does make it to the logs. - if self.response: - self.response.status = HTTP_SERVICE_UNAVAILABLE - raise diff --git a/swift/common/wsgi.py b/swift/common/wsgi.py index 1b9d8e49a6..7feb25d0b2 100644 --- a/swift/common/wsgi.py +++ b/swift/common/wsgi.py @@ -543,54 +543,10 @@ class WSGIContext(object): return None -def make_pre_authed_request(env, method=None, path=None, body=None, - headers=None, agent='Swift', swift_source=None): +def make_env(env, method=None, path=None, agent='Swift', query_string=None, + swift_source=None): """ - Makes a new swob.Request based on the current env but with the - parameters specified. Note that this request will be preauthorized. - - :param env: The WSGI environment to base the new request on. - :param method: HTTP method of new request; default is from - the original env. - :param path: HTTP path of new request; default is from the - original env. path should be compatible with what you - would send to Request.blank. path should be quoted and it - can include a query string. for example: - '/a%20space?unicode_str%E8%AA%9E=y%20es' - :param body: HTTP body of new request; empty by default. - :param headers: Extra HTTP headers of new request; None by - default. - :param agent: The HTTP user agent to use; default 'Swift'. You - can put %(orig)s in the agent to have it replaced - with the original env's HTTP_USER_AGENT, such as - '%(orig)s StaticWeb'. You also set agent to None to - use the original env's HTTP_USER_AGENT or '' to - have no HTTP_USER_AGENT. - :param swift_source: Used to mark the request as originating out of - middleware. Will be logged in proxy logs. - :returns: Fresh swob.Request object. - """ - query_string = None - path = path or '' - if path and '?' in path: - path, query_string = path.split('?', 1) - newenv = make_pre_authed_env(env, method, path=unquote(path), agent=agent, - query_string=query_string, - swift_source=swift_source) - if not headers: - headers = {} - if body: - return Request.blank(path, environ=newenv, body=body, headers=headers) - else: - return Request.blank(path, environ=newenv, headers=headers) - - -def make_pre_authed_env(env, method=None, path=None, agent='Swift', - query_string=None, swift_source=None): - """ - Returns a new fresh WSGI environment with escalated privileges to - do backend checks, listings, etc. that the remote user wouldn't - be able to accomplish directly. + Returns a new fresh WSGI environment. :param env: The WSGI environment to base the new environment on. :param method: The new REQUEST_METHOD or None to use the @@ -635,10 +591,70 @@ def make_pre_authed_env(env, method=None, path=None, agent='Swift', del newenv['HTTP_USER_AGENT'] if swift_source: newenv['swift.source'] = swift_source - newenv['swift.authorize'] = lambda req: None - newenv['swift.authorize_override'] = True - newenv['REMOTE_USER'] = '.wsgi.pre_authed' newenv['wsgi.input'] = StringIO('') if 'SCRIPT_NAME' not in newenv: newenv['SCRIPT_NAME'] = '' return newenv + + +def make_request(env, method=None, path=None, body=None, headers=None, + agent='Swift', swift_source=None, make_env=make_env): + """ + Makes a new swob.Request based on the current env but with the + parameters specified. + + :param env: The WSGI environment to base the new request on. + :param method: HTTP method of new request; default is from + the original env. + :param path: HTTP path of new request; default is from the + original env. path should be compatible with what you + would send to Request.blank. path should be quoted and it + can include a query string. for example: + '/a%20space?unicode_str%E8%AA%9E=y%20es' + :param body: HTTP body of new request; empty by default. + :param headers: Extra HTTP headers of new request; None by + default. + :param agent: The HTTP user agent to use; default 'Swift'. You + can put %(orig)s in the agent to have it replaced + with the original env's HTTP_USER_AGENT, such as + '%(orig)s StaticWeb'. You also set agent to None to + use the original env's HTTP_USER_AGENT or '' to + have no HTTP_USER_AGENT. + :param swift_source: Used to mark the request as originating out of + middleware. Will be logged in proxy logs. + :param make_env: make_request calls this make_env to help build the + swob.Request. + :returns: Fresh swob.Request object. + """ + query_string = None + path = path or '' + if path and '?' in path: + path, query_string = path.split('?', 1) + newenv = make_env(env, method, path=unquote(path), agent=agent, + query_string=query_string, swift_source=swift_source) + if not headers: + headers = {} + if body: + return Request.blank(path, environ=newenv, body=body, headers=headers) + else: + return Request.blank(path, environ=newenv, headers=headers) + + +def make_pre_authed_env(env, method=None, path=None, agent='Swift', + query_string=None, swift_source=None): + """Same as :py:func:`make_env` but with preauthorization.""" + newenv = make_env( + env, method=method, path=path, agent=agent, query_string=query_string, + swift_source=swift_source) + newenv['swift.authorize'] = lambda req: None + newenv['swift.authorize_override'] = True + newenv['REMOTE_USER'] = '.wsgi.pre_authed' + return newenv + + +def make_pre_authed_request(env, method=None, path=None, body=None, + headers=None, agent='Swift', swift_source=None): + """Same as :py:func:`make_request` but with preauthorization.""" + return make_request( + env, method=method, path=path, body=body, headers=headers, agent=agent, + swift_source=swift_source, make_env=make_pre_authed_env) diff --git a/swift/obj/server.py b/swift/obj/server.py index 9a0e2e06aa..4ce53ddf0f 100644 --- a/swift/obj/server.py +++ b/swift/obj/server.py @@ -471,14 +471,6 @@ class ObjectController(object): with disk_file.open(): metadata = disk_file.get_metadata() obj_size = int(metadata['Content-Length']) - if request.headers.get('if-match') not in (None, '*') and \ - metadata['ETag'] not in request.if_match: - return HTTPPreconditionFailed(request=request) - if request.headers.get('if-none-match') is not None: - if metadata['ETag'] in request.if_none_match: - resp = HTTPNotModified(request=request) - resp.etag = metadata['ETag'] - return resp file_x_ts = metadata['X-Timestamp'] file_x_ts_flt = float(file_x_ts) try: @@ -518,13 +510,8 @@ class ObjectController(object): pass response.headers['X-Timestamp'] = file_x_ts resp = request.get_response(response) - except DiskFileNotExist: - if request.headers.get('if-match') == '*': - resp = HTTPPreconditionFailed(request=request) - else: - resp = HTTPNotFound(request=request) - except DiskFileQuarantined: - resp = HTTPNotFound(request=request) + except (DiskFileNotExist, DiskFileQuarantined): + resp = HTTPNotFound(request=request, conditional_response=True) return resp @public @@ -541,7 +528,7 @@ class ObjectController(object): try: metadata = disk_file.read_metadata() except (DiskFileNotExist, DiskFileQuarantined): - return HTTPNotFound(request=request) + return HTTPNotFound(request=request, conditional_response=True) response = Response(request=request, conditional_response=True) response.headers['Content-Type'] = metadata.get( 'Content-Type', 'application/octet-stream') diff --git a/test/unit/common/middleware/test_dlo.py b/test/unit/common/middleware/test_dlo.py index 6763775163..52989fe278 100644 --- a/test/unit/common/middleware/test_dlo.py +++ b/test/unit/common/middleware/test_dlo.py @@ -479,6 +479,75 @@ class TestDloGetManifest(DloTestCase): self.assertEqual(headers.get("Content-Range"), None) self.assertEqual(body, "aaaaabbbbbcccccdddddeeeee") + def test_if_match_matches(self): + manifest_etag = '"%s"' % hashlib.md5( + "seg01-etag" + "seg02-etag" + "seg03-etag" + + "seg04-etag" + "seg05-etag").hexdigest() + req = swob.Request.blank('/v1/AUTH_test/mancon/manifest', + environ={'REQUEST_METHOD': 'GET'}, + headers={'If-Match': manifest_etag}) + + status, headers, body = self.call_dlo(req) + headers = swob.HeaderKeyDict(headers) + + self.assertEqual(status, '200 OK') + self.assertEqual(headers['Content-Length'], '25') + self.assertEqual(body, 'aaaaabbbbbcccccdddddeeeee') + + def test_if_match_does_not_match(self): + req = swob.Request.blank('/v1/AUTH_test/mancon/manifest', + environ={'REQUEST_METHOD': 'GET'}, + headers={'If-Match': 'not it'}) + + status, headers, body = self.call_dlo(req) + headers = swob.HeaderKeyDict(headers) + + self.assertEqual(status, '412 Precondition Failed') + self.assertEqual(headers['Content-Length'], '0') + self.assertEqual(body, '') + + def test_if_none_match_matches(self): + manifest_etag = '"%s"' % hashlib.md5( + "seg01-etag" + "seg02-etag" + "seg03-etag" + + "seg04-etag" + "seg05-etag").hexdigest() + req = swob.Request.blank('/v1/AUTH_test/mancon/manifest', + environ={'REQUEST_METHOD': 'GET'}, + headers={'If-None-Match': manifest_etag}) + + status, headers, body = self.call_dlo(req) + headers = swob.HeaderKeyDict(headers) + + self.assertEqual(status, '304 Not Modified') + self.assertEqual(headers['Content-Length'], '0') + self.assertEqual(body, '') + + def test_if_none_match_does_not_match(self): + req = swob.Request.blank('/v1/AUTH_test/mancon/manifest', + environ={'REQUEST_METHOD': 'GET'}, + headers={'If-None-Match': 'not it'}) + + status, headers, body = self.call_dlo(req) + headers = swob.HeaderKeyDict(headers) + + self.assertEqual(status, '200 OK') + self.assertEqual(headers['Content-Length'], '25') + self.assertEqual(body, 'aaaaabbbbbcccccdddddeeeee') + + def test_get_with_if_modified_since(self): + # It's important not to pass the If-[Un]Modified-Since header to the + # proxy for segment GET requests, as it may result in 304 Not Modified + # responses, and those don't contain segment data. + req = swob.Request.blank( + '/v1/AUTH_test/mancon/manifest', + environ={'REQUEST_METHOD': 'GET'}, + headers={'If-Modified-Since': 'Wed, 12 Feb 2014 22:24:52 GMT', + 'If-Unmodified-Since': 'Thu, 13 Feb 2014 23:25:53 GMT'}) + status, headers, body, exc = self.call_dlo(req, expect_exception=True) + + for _, _, hdrs in self.app.calls_with_headers[1:]: + self.assertFalse('If-Modified-Since' in hdrs) + self.assertFalse('If-Unmodified-Since' in hdrs) + def test_error_fetching_first_segment(self): self.app.register( 'GET', '/v1/AUTH_test/c/seg_01', @@ -601,8 +670,10 @@ class TestDloGetManifest(DloTestCase): environ={'REQUEST_METHOD': 'GET'}) with contextlib.nested( - mock.patch('swift.common.utils.time.time', mock_time), - mock.patch('swift.common.utils.is_success', mock_is_success), + mock.patch('swift.common.request_helpers.time.time', + mock_time), + mock.patch('swift.common.request_helpers.is_success', + mock_is_success), mock.patch.object(dlo, 'is_success', mock_is_success)): status, headers, body, exc = self.call_dlo( req, expect_exception=True) diff --git a/test/unit/common/middleware/test_slo.py b/test/unit/common/middleware/test_slo.py index 6a08adbe56..942ef764b6 100644 --- a/test/unit/common/middleware/test_slo.py +++ b/test/unit/common/middleware/test_slo.py @@ -726,6 +726,15 @@ class TestSloHeadManifest(SloTestCase): md5("seg01-hashseg02-hash").hexdigest()) self.assertEqual(body, '') # it's a HEAD request, after all + def test_etag_matching(self): + etag = md5("seg01-hashseg02-hash").hexdigest() + req = Request.blank( + '/v1/AUTH_test/headtest/man', + environ={'REQUEST_METHOD': 'HEAD'}, + headers={'If-None-Match': etag}) + status, headers, body = self.call_slo(req) + self.assertEqual(status, '304 Not Modified') + class TestSloGetManifest(SloTestCase): def setUp(self): @@ -763,21 +772,25 @@ class TestSloGetManifest(SloTestCase): 'GET', '/v1/AUTH_test/gettest/manifest-bc', swob.HTTPOk, {'Content-Type': 'application/json;swift_bytes=25', 'X-Static-Large-Object': 'true', - 'X-Object-Meta-Plant': 'Ficus'}, + 'X-Object-Meta-Plant': 'Ficus', + 'Etag': md5(_bc_manifest_json).hexdigest()}, _bc_manifest_json) + _abcd_manifest_json = json.dumps( + [{'name': '/gettest/a_5', 'hash': 'a', + 'content_type': 'text/plain', 'bytes': '5'}, + {'name': '/gettest/manifest-bc', 'sub_slo': True, + 'content_type': 'application/json;swift_bytes=25', + 'hash': md5("bc").hexdigest(), + 'bytes': len(_bc_manifest_json)}, + {'name': '/gettest/d_20', 'hash': 'd', + 'content_type': 'text/plain', 'bytes': '20'}]) self.app.register( 'GET', '/v1/AUTH_test/gettest/manifest-abcd', swob.HTTPOk, {'Content-Type': 'application/json', - 'X-Static-Large-Object': 'true'}, - json.dumps([{'name': '/gettest/a_5', 'hash': 'a', - 'content_type': 'text/plain', 'bytes': '5'}, - {'name': '/gettest/manifest-bc', 'sub_slo': True, - 'content_type': 'application/json;swift_bytes=25', - 'hash': 'manifest-bc', - 'bytes': len(_bc_manifest_json)}, - {'name': '/gettest/d_20', 'hash': 'd', - 'content_type': 'text/plain', 'bytes': '20'}])) + 'X-Static-Large-Object': 'true', + 'Etag': md5(_abcd_manifest_json).hexdigest()}, + _abcd_manifest_json) self.app.register( 'GET', '/v1/AUTH_test/gettest/manifest-badjson', @@ -842,6 +855,77 @@ class TestSloGetManifest(SloTestCase): self.assertFalse( "SLO MultipartGET" in first_ua) + def test_if_none_match_matches(self): + manifest_etag = md5("a" + md5("bc").hexdigest() + "d").hexdigest() + + req = Request.blank( + '/v1/AUTH_test/gettest/manifest-abcd', + environ={'REQUEST_METHOD': 'GET'}, + headers={'If-None-Match': manifest_etag}) + status, headers, body = self.call_slo(req) + headers = swob.HeaderKeyDict(headers) + + self.assertEqual(status, '304 Not Modified') + self.assertEqual(headers['Content-Length'], '0') + self.assertEqual(body, '') + + def test_if_none_match_does_not_match(self): + manifest_etag = md5("a" + md5("bc").hexdigest() + "d").hexdigest() + + req = Request.blank( + '/v1/AUTH_test/gettest/manifest-abcd', + environ={'REQUEST_METHOD': 'GET'}, + headers={'If-None-Match': "not-%s" % manifest_etag}) + status, headers, body = self.call_slo(req) + headers = swob.HeaderKeyDict(headers) + + self.assertEqual(status, '200 OK') + self.assertEqual( + body, 'aaaaabbbbbbbbbbcccccccccccccccdddddddddddddddddddd') + + def test_if_match_matches(self): + manifest_etag = md5("a" + md5("bc").hexdigest() + "d").hexdigest() + + req = Request.blank( + '/v1/AUTH_test/gettest/manifest-abcd', + environ={'REQUEST_METHOD': 'GET'}, + headers={'If-Match': manifest_etag}) + status, headers, body = self.call_slo(req) + headers = swob.HeaderKeyDict(headers) + + self.assertEqual(status, '200 OK') + self.assertEqual( + body, 'aaaaabbbbbbbbbbcccccccccccccccdddddddddddddddddddd') + + def test_if_match_does_not_match(self): + manifest_etag = md5("a" + md5("bc").hexdigest() + "d").hexdigest() + + req = Request.blank( + '/v1/AUTH_test/gettest/manifest-abcd', + environ={'REQUEST_METHOD': 'GET'}, + headers={'If-Match': "not-%s" % manifest_etag}) + status, headers, body = self.call_slo(req) + headers = swob.HeaderKeyDict(headers) + + self.assertEqual(status, '412 Precondition Failed') + self.assertEqual(headers['Content-Length'], '0') + self.assertEqual(body, '') + + def test_if_match_matches_and_range(self): + manifest_etag = md5("a" + md5("bc").hexdigest() + "d").hexdigest() + + req = Request.blank( + '/v1/AUTH_test/gettest/manifest-abcd', + environ={'REQUEST_METHOD': 'GET'}, + headers={'If-Match': manifest_etag, + 'Range': 'bytes=3-6'}) + status, headers, body = self.call_slo(req) + headers = swob.HeaderKeyDict(headers) + + self.assertEqual(status, '206 Partial Content') + self.assertEqual(headers['Content-Length'], '4') + self.assertEqual(body, 'aabb') + def test_get_manifest_with_submanifest(self): req = Request.blank( '/v1/AUTH_test/gettest/manifest-abcd', @@ -849,7 +933,7 @@ class TestSloGetManifest(SloTestCase): status, headers, body = self.call_slo(req) headers = swob.HeaderKeyDict(headers) - manifest_etag = md5("a" + "manifest-bc" + "d").hexdigest() + manifest_etag = md5("a" + md5("bc").hexdigest() + "d").hexdigest() self.assertEqual(status, '200 OK') self.assertEqual(headers['Content-Length'], '50') self.assertEqual(headers['Etag'], '"%s"' % manifest_etag) @@ -1115,7 +1199,7 @@ class TestSloGetManifest(SloTestCase): status, headers, body = self.call_slo(req) headers = swob.HeaderKeyDict(headers) - manifest_etag = md5("a" + "manifest-bc" + "d").hexdigest() + manifest_etag = md5("a" + md5("bc").hexdigest() + "d").hexdigest() self.assertEqual(status, '200 OK') self.assertEqual(headers['Content-Length'], '50') self.assertEqual(headers['Etag'], '"%s"' % manifest_etag) @@ -1183,6 +1267,21 @@ class TestSloGetManifest(SloTestCase): # make sure we didn't keep asking for segments self.assertEqual(self.app.call_count, 20) + def test_get_with_if_modified_since(self): + # It's important not to pass the If-[Un]Modified-Since header to the + # proxy for segment or submanifest GET requests, as it may result in + # 304 Not Modified responses, and those don't contain any useful data. + req = swob.Request.blank( + '/v1/AUTH_test/gettest/manifest-abcd', + environ={'REQUEST_METHOD': 'GET'}, + headers={'If-Modified-Since': 'Wed, 12 Feb 2014 22:24:52 GMT', + 'If-Unmodified-Since': 'Thu, 13 Feb 2014 23:25:53 GMT'}) + status, headers, body, exc = self.call_slo(req, expect_exception=True) + + for _, _, hdrs in self.app.calls_with_headers[1:]: + self.assertFalse('If-Modified-Since' in hdrs) + self.assertFalse('If-Unmodified-Since' in hdrs) + def test_error_fetching_segment(self): self.app.register('GET', '/v1/AUTH_test/gettest/c_15', swob.HTTPUnauthorized, {}, None) @@ -1320,8 +1419,10 @@ class TestSloGetManifest(SloTestCase): environ={'REQUEST_METHOD': 'GET'}) with nested(patch.object(slo, 'is_success', mock_is_success), - patch('swift.common.utils.time.time', mock_time), - patch('swift.common.utils.is_success', mock_is_success)): + patch('swift.common.request_helpers.time.time', + mock_time), + patch('swift.common.request_helpers.is_success', + mock_is_success)): status, headers, body, exc = self.call_slo( req, expect_exception=True) diff --git a/test/unit/common/test_swob.py b/test/unit/common/test_swob.py index c0de63d045..a2e5387047 100644 --- a/test/unit/common/test_swob.py +++ b/test/unit/common/test_swob.py @@ -347,7 +347,6 @@ class TestRequest(unittest.TestCase): self.assertEquals(req.query_string, 'a=b&c=d') self.assertEquals(req.environ['QUERY_STRING'], 'a=b&c=d') req = blank('/', if_match='*') - self.assertEquals(req.if_match, '*') self.assertEquals(req.environ['HTTP_IF_MATCH'], '*') self.assertEquals(req.headers['If-Match'], '*') @@ -366,7 +365,6 @@ class TestRequest(unittest.TestCase): self.assertEquals(req.user_agent, 'curl/7.22.0 (x86_64-pc-linux-gnu)') self.assertEquals(req.query_string, 'a=b&c=d') self.assertEquals(req.environ['QUERY_STRING'], 'a=b&c=d') - self.assertEquals(req.if_match, '*') def test_invalid_req_environ_property_args(self): # getter only property @@ -1325,5 +1323,127 @@ class TestUTC(unittest.TestCase): self.assertEquals(swift.common.swob.UTC.tzname(None), 'UTC') +class TestConditionalIfNoneMatch(unittest.TestCase): + def fake_app(self, environ, start_response): + start_response('200 OK', [('Etag', 'the-etag')]) + return ['hi'] + + def fake_start_response(*a, **kw): + pass + + def test_simple_match(self): + # etag matches --> 304 + req = swift.common.swob.Request.blank( + '/', headers={'If-None-Match': 'the-etag'}) + resp = req.get_response(self.fake_app) + resp.conditional_response = True + body = ''.join(resp(req.environ, self.fake_start_response)) + self.assertEquals(resp.status_int, 304) + self.assertEquals(body, '') + + def test_quoted_simple_match(self): + # double quotes don't matter + req = swift.common.swob.Request.blank( + '/', headers={'If-None-Match': '"the-etag"'}) + resp = req.get_response(self.fake_app) + resp.conditional_response = True + body = ''.join(resp(req.environ, self.fake_start_response)) + self.assertEquals(resp.status_int, 304) + self.assertEquals(body, '') + + def test_list_match(self): + # it works with lists of etags to match + req = swift.common.swob.Request.blank( + '/', headers={'If-None-Match': '"bert", "the-etag", "ernie"'}) + resp = req.get_response(self.fake_app) + resp.conditional_response = True + body = ''.join(resp(req.environ, self.fake_start_response)) + self.assertEquals(resp.status_int, 304) + self.assertEquals(body, '') + + def test_list_no_match(self): + # no matches --> whatever the original status was + req = swift.common.swob.Request.blank( + '/', headers={'If-None-Match': '"bert", "ernie"'}) + resp = req.get_response(self.fake_app) + resp.conditional_response = True + body = ''.join(resp(req.environ, self.fake_start_response)) + self.assertEquals(resp.status_int, 200) + self.assertEquals(body, 'hi') + + def test_match_star(self): + # "*" means match anything; see RFC 2616 section 14.24 + req = swift.common.swob.Request.blank( + '/', headers={'If-None-Match': '*'}) + resp = req.get_response(self.fake_app) + resp.conditional_response = True + body = ''.join(resp(req.environ, self.fake_start_response)) + self.assertEquals(resp.status_int, 304) + self.assertEquals(body, '') + + +class TestConditionalIfMatch(unittest.TestCase): + def fake_app(self, environ, start_response): + start_response('200 OK', [('Etag', 'the-etag')]) + return ['hi'] + + def fake_start_response(*a, **kw): + pass + + def test_simple_match(self): + # if etag matches, proceed as normal + req = swift.common.swob.Request.blank( + '/', headers={'If-Match': 'the-etag'}) + resp = req.get_response(self.fake_app) + resp.conditional_response = True + body = ''.join(resp(req.environ, self.fake_start_response)) + self.assertEquals(resp.status_int, 200) + self.assertEquals(body, 'hi') + + def test_quoted_simple_match(self): + # double quotes or not, doesn't matter + req = swift.common.swob.Request.blank( + '/', headers={'If-Match': '"the-etag"'}) + resp = req.get_response(self.fake_app) + resp.conditional_response = True + body = ''.join(resp(req.environ, self.fake_start_response)) + self.assertEquals(resp.status_int, 200) + self.assertEquals(body, 'hi') + + def test_no_match(self): + # no match --> 412 + req = swift.common.swob.Request.blank( + '/', headers={'If-Match': 'not-the-etag'}) + resp = req.get_response(self.fake_app) + resp.conditional_response = True + body = ''.join(resp(req.environ, self.fake_start_response)) + self.assertEquals(resp.status_int, 412) + self.assertEquals(body, '') + + def test_match_star(self): + # "*" means match anything; see RFC 2616 section 14.24 + req = swift.common.swob.Request.blank( + '/', headers={'If-Match': '*'}) + resp = req.get_response(self.fake_app) + resp.conditional_response = True + body = ''.join(resp(req.environ, self.fake_start_response)) + self.assertEquals(resp.status_int, 200) + self.assertEquals(body, 'hi') + + def test_match_star_on_404(self): + + def fake_app_404(environ, start_response): + start_response('404 Not Found', []) + return ['hi'] + + req = swift.common.swob.Request.blank( + '/', headers={'If-Match': '*'}) + resp = req.get_response(fake_app_404) + resp.conditional_response = True + body = ''.join(resp(req.environ, self.fake_start_response)) + self.assertEquals(resp.status_int, 412) + self.assertEquals(body, '') + + if __name__ == '__main__': unittest.main() diff --git a/test/unit/obj/test_server.py b/test/unit/obj/test_server.py index 8934fac0d8..0e0c3c3b16 100755 --- a/test/unit/obj/test_server.py +++ b/test/unit/obj/test_server.py @@ -960,6 +960,65 @@ class TestObjectController(unittest.TestCase): resp = req.get_response(self.object_controller) self.assertEquals(resp.status_int, 412) + def test_HEAD_if_match(self): + req = Request.blank('/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'PUT'}, + headers={ + 'X-Timestamp': normalize_timestamp(time()), + 'Content-Type': 'application/octet-stream', + 'Content-Length': '4'}) + req.body = 'test' + resp = req.get_response(self.object_controller) + self.assertEquals(resp.status_int, 201) + etag = resp.etag + + req = Request.blank('/sda1/p/a/c/o', + environ={'REQUEST_METHOD': 'HEAD'}) + resp = req.get_response(self.object_controller) + self.assertEquals(resp.status_int, 200) + self.assertEquals(resp.etag, etag) + + req = Request.blank('/sda1/p/a/c/o', + environ={'REQUEST_METHOD': 'HEAD'}, + headers={'If-Match': '*'}) + resp = req.get_response(self.object_controller) + self.assertEquals(resp.status_int, 200) + self.assertEquals(resp.etag, etag) + + req = Request.blank('/sda1/p/a/c/o2', + environ={'REQUEST_METHOD': 'HEAD'}, + headers={'If-Match': '*'}) + resp = req.get_response(self.object_controller) + self.assertEquals(resp.status_int, 412) + + req = Request.blank('/sda1/p/a/c/o', + environ={'REQUEST_METHOD': 'HEAD'}, + headers={'If-Match': '"%s"' % etag}) + resp = req.get_response(self.object_controller) + self.assertEquals(resp.status_int, 200) + self.assertEquals(resp.etag, etag) + + req = Request.blank( + '/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'HEAD'}, + headers={'If-Match': '"11111111111111111111111111111111"'}) + resp = req.get_response(self.object_controller) + self.assertEquals(resp.status_int, 412) + + req = Request.blank( + '/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'HEAD'}, + headers={ + 'If-Match': '"11111111111111111111111111111111", "%s"' % etag}) + resp = req.get_response(self.object_controller) + self.assertEquals(resp.status_int, 200) + + req = Request.blank( + '/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'HEAD'}, + headers={ + 'If-Match': + '"11111111111111111111111111111111", ' + '"22222222222222222222222222222222"'}) + resp = req.get_response(self.object_controller) + self.assertEquals(resp.status_int, 412) + def test_GET_if_none_match(self): req = Request.blank('/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'PUT'}, headers={ @@ -1010,6 +1069,60 @@ class TestObjectController(unittest.TestCase): self.assertEquals(resp.status_int, 304) self.assertEquals(resp.etag, etag) + def test_HEAD_if_none_match(self): + req = Request.blank('/sda1/p/a/c/o', + environ={'REQUEST_METHOD': 'PUT'}, + headers={ + 'X-Timestamp': normalize_timestamp(time()), + 'Content-Type': 'application/octet-stream', + 'Content-Length': '4'}) + req.body = 'test' + resp = req.get_response(self.object_controller) + self.assertEquals(resp.status_int, 201) + etag = resp.etag + + req = Request.blank('/sda1/p/a/c/o', + environ={'REQUEST_METHOD': 'HEAD'}) + resp = req.get_response(self.object_controller) + self.assertEquals(resp.status_int, 200) + self.assertEquals(resp.etag, etag) + + req = Request.blank('/sda1/p/a/c/o', + environ={'REQUEST_METHOD': 'HEAD'}, + headers={'If-None-Match': '*'}) + resp = req.get_response(self.object_controller) + self.assertEquals(resp.status_int, 304) + self.assertEquals(resp.etag, etag) + + req = Request.blank('/sda1/p/a/c/o2', + environ={'REQUEST_METHOD': 'HEAD'}, + headers={'If-None-Match': '*'}) + resp = req.get_response(self.object_controller) + self.assertEquals(resp.status_int, 404) + + req = Request.blank('/sda1/p/a/c/o', + environ={'REQUEST_METHOD': 'HEAD'}, + headers={'If-None-Match': '"%s"' % etag}) + resp = req.get_response(self.object_controller) + self.assertEquals(resp.status_int, 304) + self.assertEquals(resp.etag, etag) + + req = Request.blank( + '/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'HEAD'}, + headers={'If-None-Match': '"11111111111111111111111111111111"'}) + resp = req.get_response(self.object_controller) + self.assertEquals(resp.status_int, 200) + self.assertEquals(resp.etag, etag) + + req = Request.blank( + '/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'HEAD'}, + headers={'If-None-Match': + '"11111111111111111111111111111111", ' + '"%s"' % etag}) + resp = req.get_response(self.object_controller) + self.assertEquals(resp.status_int, 304) + self.assertEquals(resp.etag, etag) + def test_GET_if_modified_since(self): timestamp = normalize_timestamp(time()) req = Request.blank('/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'PUT'},