You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ph...@apache.org on 2017/07/30 22:31:46 UTC
svn commit: r1803471 - in /subversion/trunk/subversion:
libsvn_subr/dirent_uri.c tests/cmdline/externals_tests.py
tests/cmdline/externals_tests_data/
tests/cmdline/externals_tests_data/invalid_uris_in_repo.dump
tests/libsvn_subr/dirent_uri-test.c
Author: philip
Date: Sun Jul 30 22:31:46 2017
New Revision: 1803471
URL: http://svn.apache.org/viewvc?rev=1803471&view=rev
Log:
Make the port requirements of svn_uri_is_canonical() match the
port guarantees made by svn_uri_canonicalize(). This fixes a
client SEGV when handling invalid URLs in svn:externals.
The function svn_uri_canonicalize() always returns canonical URLs,
by definition, but being canonical does not guarantee that an URL
is valid. The function svn_uri_is_canonical() should always return
TRUE on canonical URLs even if the URL is invalid, i.e. it should
not be more stringent that svn_uri_canonicalize().
* subversion/libsvn_subr/dirent_uri.c
(svn_uri_is_canonical): Relax port checks to match svn_uri_canonicalize.
* subversion/tests/libsvn_subr/dirent_uri-test.c
(uri_canonical_tests): Add some invalid URLs that can still be canonical.
* subversion/tests/cmdline/externals_tests.py
(invalid_uris_in_repo): New.
(test_list): Add new tests.
* subversion/tests/cmdline/externals_tests_data/invalid_uris_in_repo.dump: New.
Added:
subversion/trunk/subversion/tests/cmdline/externals_tests_data/
subversion/trunk/subversion/tests/cmdline/externals_tests_data/invalid_uris_in_repo.dump
Modified:
subversion/trunk/subversion/libsvn_subr/dirent_uri.c
subversion/trunk/subversion/tests/cmdline/externals_tests.py
subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c
Modified: subversion/trunk/subversion/libsvn_subr/dirent_uri.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/dirent_uri.c?rev=1803471&r1=1803470&r2=1803471&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/dirent_uri.c (original)
+++ subversion/trunk/subversion/libsvn_subr/dirent_uri.c Sun Jul 30 22:31:46 2017
@@ -1862,18 +1862,18 @@ svn_uri_is_canonical(const char *uri, ap
ptr++;
}
- if (ptr == schema_data)
+ if (ptr == schema_data && (*ptr == '/' || *ptr == '\0'))
return FALSE; /* Fail on "http://host:" */
- if (*ptr && *ptr != '/')
- return FALSE; /* Not a port number */
-
if (port == 80 && strncmp(uri, "http:", 5) == 0)
return FALSE;
else if (port == 443 && strncmp(uri, "https:", 6) == 0)
return FALSE;
else if (port == 3690 && strncmp(uri, "svn:", 4) == 0)
return FALSE;
+
+ while (*ptr && *ptr != '/')
+ ++ptr; /* Allow "http://host:stuff" */
}
schema_data = ptr;
Modified: subversion/trunk/subversion/tests/cmdline/externals_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/externals_tests.py?rev=1803471&r1=1803470&r2=1803471&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/externals_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/externals_tests.py Sun Jul 30 22:31:46 2017
@@ -4365,6 +4365,39 @@ def external_externally_removed(sbox):
sbox.simple_propdel('svn:externals', '')
sbox.simple_update() # Should succeed
+def invalid_uris_in_repo(sbox):
+ "invalid URIs in repo"
+
+ sbox.build(empty=True),
+
+ # Using a dump file because the client may not allow adding invalid URIs.
+ svntest.actions.load_repo(sbox,
+ os.path.join(os.path.dirname(sys.argv[0]),
+ 'externals_tests_data',
+ 'invalid_uris_in_repo.dump'),
+ create_wc=False)
+
+ # 'foo://host:-/D X'
+ expected_output = svntest.wc.State(sbox.wc_dir, {
+ '' : Item(status=' U')
+ })
+ expected_disk = svntest.wc.State('', {
+ })
+ expected_error = ".*warning: W205011: Error handling externals definition.*"
+
+ # A repository might have invalid URIs and the client used to SEGV.
+ # r1 has 'foo://host:-/D X'
+ # r2 has 'foo://host::/D X'
+ # r3 has 'foo://host:123xx/D X'
+ # r4 has 'foo://host:123:123/D X'
+ for revision in range(1,4):
+ svntest.actions.run_and_verify_checkout(sbox.repo_url, sbox.wc_dir,
+ expected_output,
+ expected_disk,
+ expected_error,
+ "-r", revision)
+ svntest.main.safe_rmtree(sbox.wc_dir)
+
########################################################################
# Run the tests
@@ -4440,6 +4473,7 @@ test_list = [ None,
file_external_to_normal_file,
file_external_recorded_info,
external_externally_removed,
+ invalid_uris_in_repo,
]
if __name__ == '__main__':
Added: subversion/trunk/subversion/tests/cmdline/externals_tests_data/invalid_uris_in_repo.dump
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/externals_tests_data/invalid_uris_in_repo.dump?rev=1803471&view=auto
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/externals_tests_data/invalid_uris_in_repo.dump (added)
+++ subversion/trunk/subversion/tests/cmdline/externals_tests_data/invalid_uris_in_repo.dump Sun Jul 30 22:31:46 2017
@@ -0,0 +1,142 @@
+SVN-fs-dump-format-version: 2
+
+UUID: 9402b9d1-cda2-4b37-b4d4-cbe5373b6650
+
+Revision-number: 0
+Prop-content-length: 56
+Content-length: 56
+
+K 8
+svn:date
+V 27
+2017-07-30T21:45:44.035017Z
+PROPS-END
+
+Revision-number: 1
+Prop-content-length: 97
+Content-length: 97
+
+K 10
+svn:author
+V 2
+pm
+K 8
+svn:date
+V 27
+2017-07-30T21:45:44.071811Z
+K 7
+svn:log
+V 1
+m
+PROPS-END
+
+Node-path:
+Node-kind: dir
+Node-action: change
+Prop-content-length: 56
+Content-length: 56
+
+K 13
+svn:externals
+V 21
+foo:://host:-/path X
+
+PROPS-END
+
+
+Revision-number: 2
+Prop-content-length: 97
+Content-length: 97
+
+K 10
+svn:author
+V 2
+pm
+K 8
+svn:date
+V 27
+2017-07-30T21:45:44.117957Z
+K 7
+svn:log
+V 1
+m
+PROPS-END
+
+Node-path:
+Node-kind: dir
+Node-action: change
+Prop-content-length: 55
+Content-length: 55
+
+K 13
+svn:externals
+V 20
+foo://host::/path X
+
+PROPS-END
+
+
+Revision-number: 3
+Prop-content-length: 97
+Content-length: 97
+
+K 10
+svn:author
+V 2
+pm
+K 8
+svn:date
+V 27
+2017-07-30T21:45:44.157176Z
+K 7
+svn:log
+V 1
+m
+PROPS-END
+
+Node-path:
+Node-kind: dir
+Node-action: change
+Prop-content-length: 59
+Content-length: 59
+
+K 13
+svn:externals
+V 24
+foo://host:123xx/path X
+
+PROPS-END
+
+
+Revision-number: 4
+Prop-content-length: 97
+Content-length: 97
+
+K 10
+svn:author
+V 2
+pm
+K 8
+svn:date
+V 27
+2017-07-30T21:45:44.199565Z
+K 7
+svn:log
+V 1
+m
+PROPS-END
+
+Node-path:
+Node-kind: dir
+Node-action: change
+Prop-content-length: 61
+Content-length: 61
+
+K 13
+svn:externals
+V 26
+foo://host:123:123/path X
+
+PROPS-END
+
+
Modified: subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c?rev=1803471&r1=1803470&r2=1803471&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c Sun Jul 30 22:31:46 2017
@@ -888,13 +888,6 @@ static const testcase_canonicalize_t uri
{ "http://server:1", "http://server:1" },
{ "http://server:443", "http://server:443" },
{ "http://server:81/", "http://server:81" },
-#if 0
- /* These pass svn_uri_canonicalize() but fail svn_uri_is_canonical() */
- { "http://server:81:81/", "http://server:81:81" },
- { "http://server:81foo/", "http://server:81foo" },
- { "http://server::/", "http://server::" },
- { "http://server:-/", "http://server:-" },
-#endif
{ "http://SERVER:3690/", "http://server:3690" },
{ "https://server:3690", "https://server:3690" },
{ "https://SERVER:80/", "https://server:80" },
@@ -945,6 +938,13 @@ static const testcase_canonicalize_t uri
/* Hostnames that look like non-canonical paths */
{ "file://./foo", "file://./foo" },
{ "http://./foo", "http://./foo" },
+ /* Some invalid URLs, these still have a canonical form */
+ { "http://server:81:81/", "http://server:81:81" },
+ { "http://server:81foo/", "http://server:81foo" },
+ { "http://server::/", "http://server::" },
+ { "http://server:-/", "http://server:-" },
+ { "http://hst:1.2.3.4.5/", "http://hst:1.2.3.4.5"},
+ { "http://hst:1.2.999.4/", "http://hst:1.2.999.4"},
/* svn_uri_is_canonical() was a private function in the 1.6 API, and
has since taken a MAJOR change of direction, namely that only
absolute URLs are considered canonical uris now. */