Author: kotkov Date: Thu Nov 26 10:18:01 2015 New Revision: 1716593 URL: http://svn.apache.org/viewvc?rev=1716593&view=rev Log: Fixup the recent solution for issue SVN-4514 by discouraging caching for requests that specify ?r=WORKINGREV in the url, e.g.
https://svn.apache.org/repos/asf/subversion/trunk/README?r=1716593 Results for these requests aren't guaranteed to be stable, since mod_dav_svn can either immediately return the resource at the revision or trace back the history to the canonical ?p=PEGREV location. The result depends on current state of the repository, and cannot be cached, so make sure we set the "Cache-Control: max-age=0" header when responding to such requests. Add a new test that specifies current behavior of how and when we set the Cache-Control header, and as well covers the aforementioned ?r= case. * subversion/mod_dav_svn/repos.c (prep_regular): Do not mark the resource with a URL query string as 'idempotent', unless it specifies a peg revision. * subversion/tests/cmdline/svntest/main.py (create_http_connection): New utility function, factored out from ... * subversion/tests/cmdline/lock_tests.py (http_connection, create_dav_lock_timeout, dav_lock_refresh): ...here. * subversion/tests/cmdline/mod_dav_svn_tests.py: New file. Added: subversion/trunk/subversion/tests/cmdline/mod_dav_svn_tests.py (with props) Modified: subversion/trunk/subversion/mod_dav_svn/repos.c subversion/trunk/subversion/tests/cmdline/lock_tests.py subversion/trunk/subversion/tests/cmdline/svntest/main.py Modified: subversion/trunk/subversion/mod_dav_svn/repos.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/repos.c?rev=1716593&r1=1716592&r2=1716593&view=diff ============================================================================== --- subversion/trunk/subversion/mod_dav_svn/repos.c (original) +++ subversion/trunk/subversion/mod_dav_svn/repos.c Thu Nov 26 10:18:01 2015 @@ -821,9 +821,18 @@ prep_regular(dav_resource_combined *comb } else { - /* Mark resource as 'idempotent' since we have specific revision - in URI. */ - comb->priv.idempotent = TRUE; + /* Did we have a query for this REGULAR resource? */ + if (comb->priv.r->parsed_uri.query) + { + /* If yes, it's 'idempotent' only if peg revision is specified. */ + comb->priv.idempotent = comb->priv.pegged; + } + else + { + /* Otherwise, we have the specific revision in URI, so the resource + is 'idempotent'. */ + comb->priv.idempotent = TRUE; + } } /* get the root of the tree */ Modified: subversion/trunk/subversion/tests/cmdline/lock_tests.py URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/lock_tests.py?rev=1716593&r1=1716592&r2=1716593&view=diff ============================================================================== --- subversion/trunk/subversion/tests/cmdline/lock_tests.py (original) +++ subversion/trunk/subversion/tests/cmdline/lock_tests.py Thu Nov 26 10:18:01 2015 @@ -2074,25 +2074,6 @@ def dav_lock_timeout(sbox): expected_status.tweak('iota', writelocked='K') svntest.actions.run_and_verify_status(wc_dir, expected_status) -def http_connection(repo_url): - - import httplib - from urlparse import urlparse - - loc = urlparse(repo_url) - if loc.scheme == 'http': - h = httplib.HTTPConnection(loc.hostname, loc.port) - else: - try: - import ssl # new in python 2.6 - c = ssl.create_default_context() - c.check_hostname = False - c.verify_mode = ssl.CERT_NONE - h = httplib.HTTPSConnection(loc.hostname, loc.port, context=c) - except: - h = httplib.HTTPSConnection(loc.hostname, loc.port) - return h - @SkipUnless(svntest.main.is_ra_type_dav) def create_dav_lock_timeout(sbox): "create generic DAV lock with timeout" @@ -2103,7 +2084,7 @@ def create_dav_lock_timeout(sbox): sbox.build() wc_dir = sbox.wc_dir - h = http_connection(sbox.repo_url) + h = svntest.main.create_http_connection(sbox.repo_url) lock_body = '<?xml version="1.0" encoding="utf-8" ?>' \ '<D:lockinfo xmlns:D="DAV:">' \ @@ -2119,9 +2100,6 @@ def create_dav_lock_timeout(sbox): 'Timeout': 'Second-86400' } - # Enabling the following line makes this test easier to debug - h.set_debuglevel(9) - h.request('LOCK', sbox.repo_url + '/iota', lock_body, lock_headers) r = h.getresponse() @@ -2248,7 +2226,7 @@ def dav_lock_refresh(sbox): sbox.repo_url + '/iota') # Try to refresh lock using 'If' header - h = http_connection(sbox.repo_url) + h = svntest.main.create_http_connection(sbox.repo_url) lock_token = svntest.actions.run_and_parse_info(sbox.repo_url + '/iota')[0]['Lock Token'] @@ -2258,9 +2236,6 @@ def dav_lock_refresh(sbox): 'Timeout': 'Second-7200' } - # Enabling the following line makes this test easier to debug - h.set_debuglevel(9) - h.request('LOCK', sbox.repo_url + '/iota', '', lock_headers) # XFAIL Refreshing of DAV lock fails with error '412 Precondition Failed' Added: subversion/trunk/subversion/tests/cmdline/mod_dav_svn_tests.py URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/mod_dav_svn_tests.py?rev=1716593&view=auto ============================================================================== --- subversion/trunk/subversion/tests/cmdline/mod_dav_svn_tests.py (added) +++ subversion/trunk/subversion/tests/cmdline/mod_dav_svn_tests.py Thu Nov 26 10:18:01 2015 @@ -0,0 +1,160 @@ +#!/usr/bin/env python +# +# mod_dav_svn_tests.py: testing mod_dav_svn +# +# Subversion is a tool for revision control. +# See http://subversion.apache.org for more information. +# +# ==================================================================== +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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. +###################################################################### + +# General modules +import logging, httplib, base64 + +logger = logging.getLogger() + +# Our testing module +import svntest + +# (abbreviation) +Skip = svntest.testcase.Skip_deco +SkipUnless = svntest.testcase.SkipUnless_deco +XFail = svntest.testcase.XFail_deco +Issues = svntest.testcase.Issues_deco +Issue = svntest.testcase.Issue_deco +Wimp = svntest.testcase.Wimp_deco + +###################################################################### +# Tests + +@SkipUnless(svntest.main.is_ra_type_dav) +def cache_control_header(sbox): + "verify 'Cache-Control' headers on responses" + + sbox.build(create_wc=False, read_only=True) + + headers = { + 'Authorization': 'Basic ' + base64.b64encode('jconstant:rayjandom'), + } + + h = svntest.main.create_http_connection(sbox.repo_url) + + # GET /repos/iota + # Response depends on the youngest revision in the repository, and + # can't be cached; expect to see Cache-Control: max-age=0. + h.request('GET', sbox.repo_url + '/iota', None, headers) + r = h.getresponse() + if r.status != httplib.OK: + raise svntest.Failure('Request failed: %d %s' % (r.status, r.reason)) + svntest.verify.compare_and_display_lines(None, 'Cache-Control', + 'max-age=0', + r.getheader('Cache-Control')) + r.read() + + # GET /repos/A/ + # Response depends on the youngest revision in the repository, and + # can't be cached; expect to see Cache-Control: max-age=0. + h.request('GET', sbox.repo_url + '/A/', None, headers) + r = h.getresponse() + if r.status != httplib.OK: + raise svntest.Failure('Request failed: %d %s' % (r.status, r.reason)) + svntest.verify.compare_and_display_lines(None, 'Cache-Control', + 'max-age=0', + r.getheader('Cache-Control')) + r.read() + + # GET /repos/A/?p=1 + # Response for a pegged directory is a subject for authz filtering, and + # can't be cached; expect to see Cache-Control: max-age=0. + h.request('GET', sbox.repo_url + '/A/?p=1', None, headers) + r = h.getresponse() + if r.status != httplib.OK: + raise svntest.Failure('Request failed: %d %s' % (r.status, r.reason)) + svntest.verify.compare_and_display_lines(None, 'Cache-Control', + 'max-age=0', + r.getheader('Cache-Control')) + r.read() + + # GET /repos/iota?r=1 + # Response for a file URL with ?r=WORKINGREV is mutable, because the + # line of history for this file can be replaced in the future (hence, + # the same request will start producing another response). Expect to + # see Cache-Control: max-age=0. + h.request('GET', sbox.repo_url + '/iota?r=1', None, headers) + r = h.getresponse() + if r.status != httplib.OK: + raise svntest.Failure('Request failed: %d %s' % (r.status, r.reason)) + svntest.verify.compare_and_display_lines(None, 'Cache-Control', + 'max-age=0', + r.getheader('Cache-Control')) + r.read() + + # GET /repos/iota?p=1 + # Response for a pegged file is immutable; expect to see Cache-Control + # with non-zero max-age. + h.request('GET', sbox.repo_url + '/iota?p=1', None, headers) + r = h.getresponse() + if r.status != httplib.OK: + raise svntest.Failure('Request failed: %d %s' % (r.status, r.reason)) + svntest.verify.compare_and_display_lines(None, 'Cache-Control', + 'max-age=604800', + r.getheader('Cache-Control')) + r.read() + + # GET /repos/iota?p=1&r=1 + # Response for a file URL with both ?p=PEG_REV and ?r=WORKINGREV is + # immutable; expect to see Cache-Control with non-zero max-age. + h.request('GET', sbox.repo_url + '/iota?p=1&r=1', None, headers) + r = h.getresponse() + if r.status != httplib.OK: + raise svntest.Failure('Request failed: %d %s' % (r.status, r.reason)) + svntest.verify.compare_and_display_lines(None, 'Cache-Control', + 'max-age=604800', + r.getheader('Cache-Control')) + r.read() + + + # GET /repos/!svn/rvr/1/iota + # Response is immutable; expect to see Cache-Control with non-zero max-age. + h.request('GET', sbox.repo_url + '/!svn/rvr/1/iota', None, headers) + r = h.getresponse() + if r.status != httplib.OK: + raise svntest.Failure('Request failed: %d %s' % (r.status, r.reason)) + svntest.verify.compare_and_display_lines(None, 'Cache-Control', + 'max-age=604800', + r.getheader('Cache-Control')) + r.read() + + +######################################################################## +# Run the tests + + +# list all tests here, starting with None: +test_list = [ None, + cache_control_header, + ] +serial_only = True + +if __name__ == '__main__': + svntest.main.run_tests(test_list) + # NOTREACHED + + +### End of file. Propchange: subversion/trunk/subversion/tests/cmdline/mod_dav_svn_tests.py ------------------------------------------------------------------------------ svn:eol-style = native Modified: subversion/trunk/subversion/tests/cmdline/svntest/main.py URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/svntest/main.py?rev=1716593&r1=1716592&r2=1716593&view=diff ============================================================================== --- subversion/trunk/subversion/tests/cmdline/svntest/main.py (original) +++ subversion/trunk/subversion/tests/cmdline/svntest/main.py Thu Nov 26 10:18:01 2015 @@ -1200,6 +1200,30 @@ def create_python_hook_script(hook_path, file_write(hook_path, "#!%s\n%s" % (sys.executable, hook_script_code)) os.chmod(hook_path, 0755) +def create_http_connection(url, debuglevel=9): + """Create an http(s) connection to the host specified by URL. + Set the debugging level (the amount of debugging output printed when + working with this connection) to DEBUGLEVEL. By default, all debugging + output is printed. """ + + import httplib + from urlparse import urlparse + + loc = urlparse(url) + if loc.scheme == 'http': + h = httplib.HTTPConnection(loc.hostname, loc.port) + else: + try: + import ssl # new in python 2.6 + c = ssl.create_default_context() + c.check_hostname = False + c.verify_mode = ssl.CERT_NONE + h = httplib.HTTPSConnection(loc.hostname, loc.port, context=c) + except: + h = httplib.HTTPSConnection(loc.hostname, loc.port) + h.set_debuglevel(debuglevel) + return h + def write_restrictive_svnserve_conf(repo_dir, anon_access="none"): "Create a restrictive authz file ( no anynomous access )."
