You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ko...@apache.org on 2015/11/26 11:18:01 UTC
svn commit: r1716593 - in /subversion/trunk/subversion: mod_dav_svn/repos.c
tests/cmdline/lock_tests.py tests/cmdline/mod_dav_svn_tests.py
tests/cmdline/svntest/main.py
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 )."
Re: svn commit: r1716593 - in /subversion/trunk/subversion:
mod_dav_svn/repos.c tests/cmdline/lock_tests.py tests/cmdline/mod_dav_svn_tests.py
tests/cmdline/svntest/main.py
Posted by Evgeny Kotkov <ev...@visualsvn.com>.
Bert Huijben <be...@qqmail.nl> writes:
>> 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
>
> I think you should document this as not sending ?p=<> in the url.
>
> Once you add a ?p=, an additional &r= specifying the same or lower revision
> is 100% safe.
Thanks for taking a look at this change.
I updated the log message for this revision in order to make more precise.
Regards,
Evgeny Kotkov
Re: svn commit: r1716593 - in /subversion/trunk/subversion:
mod_dav_svn/repos.c tests/cmdline/lock_tests.py tests/cmdline/mod_dav_svn_tests.py
tests/cmdline/svntest/main.py
Posted by Evgeny Kotkov <ev...@visualsvn.com>.
Bert Huijben <be...@qqmail.nl> writes:
>> 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
>
> I think you should document this as not sending ?p=<> in the url.
>
> Once you add a ?p=, an additional &r= specifying the same or lower revision
> is 100% safe.
Thanks for taking a look at this change.
I updated the log message for this revision in order to make more precise.
Regards,
Evgeny Kotkov
RE: svn commit: r1716593 - in /subversion/trunk/subversion: mod_dav_svn/repos.c tests/cmdline/lock_tests.py tests/cmdline/mod_dav_svn_tests.py tests/cmdline/svntest/main.py
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: kotkov@apache.org [mailto:kotkov@apache.org]
> Sent: donderdag 26 november 2015 11:18
> To: commits@subversion.apache.org
> Subject: svn commit: r1716593 - in /subversion/trunk/subversion:
> mod_dav_svn/repos.c tests/cmdline/lock_tests.py
> tests/cmdline/mod_dav_svn_tests.py tests/cmdline/svntest/main.py
>
> 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
I think you should document this as not sending ?p=<> in the url.
Once you add a ?p=, an additional &r= specifying the same or lower revision is 100% safe.
Bert
RE: svn commit: r1716593 - in /subversion/trunk/subversion: mod_dav_svn/repos.c tests/cmdline/lock_tests.py tests/cmdline/mod_dav_svn_tests.py tests/cmdline/svntest/main.py
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: kotkov@apache.org [mailto:kotkov@apache.org]
> Sent: donderdag 26 november 2015 11:18
> To: commits@subversion.apache.org
> Subject: svn commit: r1716593 - in /subversion/trunk/subversion:
> mod_dav_svn/repos.c tests/cmdline/lock_tests.py
> tests/cmdline/mod_dav_svn_tests.py tests/cmdline/svntest/main.py
>
> 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
I think you should document this as not sending ?p=<> in the url.
Once you add a ?p=, an additional &r= specifying the same or lower revision is 100% safe.
Bert