From a4255c46b1e32b4a663f47b8b286264d3c1c8bf5 Mon Sep 17 00:00:00 2001 From: Simeon Warner Date: Thu, 4 Jul 2013 17:44:51 -0400 Subject: [PATCH] Relax default notion of URL authority, add --strictauth to cient for Sitemaps style --- bin/resync | 13 ++++++++-- resync/client.py | 31 ++++++++++------------- resync/test/test_url_authority.py | 37 +++++++++++++++++++++++---- resync/url_authority.py | 42 ++++++++++++++++++++++++++----- 4 files changed, 92 insertions(+), 31 deletions(-) diff --git a/bin/resync b/bin/resync index 6eef0df..0c18b7c 100755 --- a/bin/resync +++ b/bin/resync @@ -133,8 +133,15 @@ def main(): opt.add_option('--multifile', '-m', action='store_true', help="disable reading and output of sitemapindex for multifile sitemap") opt.add_option('--noauth', action='store_true', - help="disable checking of URL paths to ensure that the sitemaps refer " - "only to resources on the same server/sub-path etc. Use with care.") + help="disable all checking of URLs to ensure that the ResourceSync " + "documents refer only to resources on the same server or sub-domains. " + "Use with care.") + opt.add_option('--strictauth', action='store_true', + help="use more strict checking of URLs to ensure that the ResourceSync " + "documents refer only to resources on the same server or sub-domains, " + "and on the same server to sub-paths. This is the authority model " + "of Sitemaps but there are legitimate uses where these rules would " + "not be followed.") opt.add_option('--warc', action='store_true', help="write dumps in WARC format (instead of ZIP+Sitemap default)") opt.add_option('--dryrun', '-n', action='store_true', @@ -199,6 +206,8 @@ def main(): c.allow_multifile=not args.multifile if (args.noauth): c.noauth=args.noauth + if (args.strictauth): + c.strictauth=args.strictauth if (args.max_sitemap_entries): c.max_sitemap_entries=args.max_sitemap_entries if (args.ignore_failures): diff --git a/resync/client.py b/resync/client.py index ff9567b..c57a456 100644 --- a/resync/client.py +++ b/resync/client.py @@ -53,6 +53,7 @@ class Client(object): self.sitemap_name = None self.allow_multifile = True self.noauth = False + self.strictauth = False self.max_sitemap_entries = None self.ignore_failures = False self.pretty_xml = True @@ -164,13 +165,10 @@ class Client(object): self.logger.debug("Completed "+action) return ### 4. Check that sitemap has authority over URIs listed - uauth = UrlAuthority(self.sitemap) - for resource in src_resource_list: - if (not uauth.has_authority_over(resource.uri)): - if (self.noauth): - #self.logger.info("Sitemap (%s) mentions resource at a location it does not have authority over (%s)" % (self.sitemap,resource.uri)) - pass - else: + if (not self.noauth): + uauth = UrlAuthority(self.sitemap, strict=self.strictauth) + for resource in src_resource_list: + if (not uauth.has_authority_over(resource.uri)): raise ClientFatalError("Aborting as sitemap (%s) mentions resource at a location it does not have authority over (%s), override with --noauth" % (self.sitemap,resource.uri)) ### 5. Grab files to do sync delete_msg = (", and delete %d resources" % len(deleted)) if (allow_deletion) else '' @@ -255,17 +253,14 @@ class Client(object): ### 4. Check that the change list has authority over URIs listed # FIXME - What does authority mean for change list? Here use both the # change list URI and, if we used it, the sitemap URI - uauth_cs = UrlAuthority(change_list) - if (not change_list_uri): - uauth_sm = UrlAuthority(self.sitemap) - for resource in src_change_list: - if (not uauth_cs.has_authority_over(resource.uri) and - (change_list_uri or not uauth_sm.has_authority_over(resource.uri))): - if (self.noauth): - #self.logger.info("Change list (%s) mentions resource at a location it does not have authority over (%s)" % (change_list,resource.uri)) - pass - else: - raise ClientFatalError("Aborting as change list (%s) mentions resource at a location it does not have authority over (%s), override with --noauth" % (change_list,resource.uri)) + if (not self.noauth): + uauth_cs = UrlAuthority(change_list, self.strict) + if (not change_list_uri): + uauth_sm = UrlAuthority(self.sitemap) + for resource in src_change_list: + if (not uauth_cs.has_authority_over(resource.uri) and + (change_list_uri or not uauth_sm.has_authority_over(resource.uri))): + raise ClientFatalError("Aborting as change list (%s) mentions resource at a location it does not have authority over (%s), override with --noauth" % (change_list,resource.uri)) ### 5. Prune entries before starting timestamp and dupe changes for a resource num_skipped = src_change_list.prune_before(from_timestamp) if (num_skipped>0): diff --git a/resync/test/test_url_authority.py b/resync/test/test_url_authority.py index 66914aa..4b5573d 100644 --- a/resync/test/test_url_authority.py +++ b/resync/test/test_url_authority.py @@ -4,8 +4,9 @@ from resync.url_authority import UrlAuthority class TestUrlAuthority(unittest.TestCase): - def test1(self): - uauth = UrlAuthority( 'http://example.org/sitemap.xml' ) + def test01_strict_authority(self): + # Default is server check only + uauth = UrlAuthority( 'http://example.org/sitemap.xml', True ) self.assertTrue( uauth.has_authority_over( 'http://example.org/sitemap.xml' ) ) self.assertTrue( uauth.has_authority_over( 'http://example.org/sitemap.xml?anything' ) ) self.assertTrue( uauth.has_authority_over( 'http://example.org/sitemap.xml#frag' ) ) @@ -16,14 +17,40 @@ class TestUrlAuthority(unittest.TestCase): self.assertTrue( uauth.has_authority_over( 'http://sub.example.org/subdomain' ) ) self.assertTrue( uauth.has_authority_over( 'http://sub.sub.example.org/subsubdomain' ) ) - def test2_no_authority(self): - uauth = UrlAuthority( 'http://example.org/dir/sitemap.xml' ) + def test02_strict_no_authority(self): + uauth = UrlAuthority( 'http://example.org/dir/sitemap.xml', True ) self.assertFalse( uauth.has_authority_over( 'http://example.org/sitemap.xml' ) ) self.assertFalse( uauth.has_authority_over( 'http://sub.example.org/sitemap.xml' ) ) self.assertFalse( uauth.has_authority_over( 'https://example.org/dir/sitemap.xml' ) ) self.assertFalse( uauth.has_authority_over( 'unknown://example.org/dir/sitemap.xml' ) ) - def test3_domains(self): + def test03_strict_domains(self): + uauth = UrlAuthority( 'http://a.example.org/sitemap.xml', True ) + self.assertTrue( uauth.has_authority_over( 'http://a.example.org/sitemap.xml' ) ) + self.assertTrue( uauth.has_authority_over( 'http://sub.a.example.org/sitemap.xml' ) ) + self.assertFalse( uauth.has_authority_over( 'http://b.example.org/sitemap.xml' ) ) + self.assertFalse( uauth.has_authority_over( 'http://sub.b.example.org/sitemap.xml' ) ) + + def test04_lax_authority(self): + # Default is server check only + uauth = UrlAuthority( 'http://example.org/dir/sitemap.xml' ) + self.assertTrue( uauth.has_authority_over( 'http://example.org/sitemap.xml' ) ) + self.assertTrue( uauth.has_authority_over( 'http://example.org/sitemap.xml?anything' ) ) + self.assertTrue( uauth.has_authority_over( 'http://example.org/sitemap.xml#frag' ) ) + self.assertTrue( uauth.has_authority_over( 'http://example.org/dir/same_level' ) ) + self.assertTrue( uauth.has_authority_over( 'http://example.org/dir/one/deeper' ) ) + self.assertTrue( uauth.has_authority_over( 'http://example.org/dir/one/two/deeper' ) ) + self.assertTrue( uauth.has_authority_over( 'http://example.org/shallower' ) ) + self.assertTrue( uauth.has_authority_over( 'http://example.org/' ) ) + self.assertTrue( uauth.has_authority_over( 'http://sub.example.org/subdomain' ) ) + self.assertTrue( uauth.has_authority_over( 'http://sub.sub.example.org/subsubdomain' ) ) + + def test05_lax_no_authority(self): + uauth = UrlAuthority( 'http://example.org/dir/sitemap.xml' ) + self.assertFalse( uauth.has_authority_over( 'http://other.org/sitemap.xml' ) ) + self.assertFalse( uauth.has_authority_over( 'unknown://example.org/dir/sitemap.xml' ) ) + + def test06_lax_domains(self): uauth = UrlAuthority( 'http://a.example.org/sitemap.xml' ) self.assertTrue( uauth.has_authority_over( 'http://a.example.org/sitemap.xml' ) ) self.assertTrue( uauth.has_authority_over( 'http://sub.a.example.org/sitemap.xml' ) ) diff --git a/resync/url_authority.py b/resync/url_authority.py index 856f0b3..ff17ce0 100644 --- a/resync/url_authority.py +++ b/resync/url_authority.py @@ -1,12 +1,38 @@ -"""Determine whether one resource can speak authoritatively about another""" +"""Determine whether one resource can speak authoritatively +about another based on DNS hierarchy of server names and +path hierarchy within URLs. + +Two modes are supported: + +strict=True: requires that a query URL has the same URI +scheme (e.g. http) as the master, is on the same server +or one in a sub-domain, and that the path component is +at the same level or below the master. + +strict=False (default): requires only that a query URL +has the same URI scheme as the master, and is on the same +server or one in a sub-domain of the master. + +Example use: + + from resync.url_authority import UrlAuthority + + auth = UrlAuthority("http://example.org/master") + if (auth.has_authority_over("http://example.com/res1")): + # will be true + if (auth.has_authority_over("http://other.com/res1")): + # will be false +""" import urlparse import os.path class UrlAuthority(object): - def __init__(self, url=None): + def __init__(self, url=None, strict=False): + """Create object and optionally set master url and/or strict mode""" self.url = url + self.strict = strict if (self.url is not None): self.set_master(self.url) else: @@ -22,7 +48,12 @@ class UrlAuthority(object): self.master_path=os.path.dirname(m.path) def has_authority_over(self, url): - """Returns True of the current master has authority over url""" + """Returns True of the current master has authority over url + + In strict mode checks scheme, server and path. Otherwise checks + just that the server names match or the query url is a + sub-domain of the master + """ s = urlparse.urlparse(url) if (s.scheme != self.master_scheme): return(False) @@ -31,9 +62,8 @@ class UrlAuthority(object): return(False) #Maybe should allow parallel for 3+ components, eg. a.example.org, b.example.org path = os.path.dirname(s.path) - if (path != self.master_path and + if (self.strict and + path != self.master_path and not path.startswith(self.master_path)): return(False) return(True) - -