{"thread":{"id":"15955","subject":"git-svn url-encodes ~ when it should not","startedAt":"2008-10-18T21:39:19Z","lastAt":"2008-10-24T09:13:30Z","messageCount":9,"participants":["Björn Steinbrink","Eric Wong","Junio C Hamano","Mike Hommey"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"93392","messageId":"20081018213919.GC3107@atjola.homenet","threadId":"15955","inReplyTo":null,"subject":"git-svn url-encodes ~ when it should not","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2008-10-18T21:39:19Z","receivedAt":"2008-10-18T21:39:19Z","isPatch":false,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"Hi,\n\nJose Carlos Garcia Sogo reported on #git that a git-svn clone of this\nsvn repo fails for him:\nhttps://sucs.org/~welshbyte/svn/backuptool/trunk\n\nI can reproduce that here with:\ngit-svn version 1.6.0.2.541.g46dc1.dirty (svn 1.5.1)\n\nThe error message I get is:\nApache got a malformed URI: Unusable URI: it does not refer to this\nrepository at /usr/local/libexec/git-core/git-svn line 4057\n\nstrace revealed that git-svn url-encodes ~ while svn does not do that.\n\nFor svn we have:\nwrite(5, \"\nhttps://sucs.org/~welshbyte/svn/backuptool/trunk...\n\nWhile git-svn shows:\nwrite(7, \"\nhttps://sucs.org/%7Ewelshbyte/svn/backuptool/trunk...\n\nBjörn\n"},{"id":"93396","messageId":"20081018224728.GD3107@atjola.homenet","threadId":"15955","inReplyTo":"20081018213919.GC3107@atjola.homenet","subject":"Re: git-svn url-encodes ~ when it should not","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2008-10-18T22:47:28Z","receivedAt":"2008-10-18T22:47:28Z","isPatch":false,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"[Adding Jose to Cc:, didn't have his address earlier]\n\nOn 2008.10.18 23:39:19 +0200, Björn Steinbrink wrote:\n> Hi,\n> \n> Jose Carlos Garcia Sogo reported on #git that a git-svn clone of this\n> svn repo fails for him:\n> https://sucs.org/~welshbyte/svn/backuptool/trunk\n> \n> I can reproduce that here with:\n> git-svn version 1.6.0.2.541.g46dc1.dirty (svn 1.5.1)\n> \n> The error message I get is:\n> Apache got a malformed URI: Unusable URI: it does not refer to this\n> repository at /usr/local/libexec/git-core/git-svn line 4057\n> \n> strace revealed that git-svn url-encodes ~ while svn does not do that.\n> \n> For svn we have:\n> write(5, \"\n> https://sucs.org/~welshbyte/svn/backuptool/trunk...\n> \n> While git-svn shows:\n> write(7, \"\n> https://sucs.org/%7Ewelshbyte/svn/backuptool/trunk...\n> \n> Björn\n"},{"id":"93636","messageId":"20081021211131.GA21606@yp-box.dyndns.org","threadId":"15955","inReplyTo":"20081018224728.GD3107@atjola.homenet","subject":"[PATCH] git-svn: don't escape tilde ('~') for http(s) URLs","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2008-10-21T21:12:15Z","receivedAt":"2008-10-21T21:12:15Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Thanks to Jose Carlos Garcia Sogo and Björn Steinbrink for the\nbug report.\n\nOn 2008.10.18 23:39:19 +0200, Björn Steinbrink wrote:\n> Hi,\n>\n> Jose Carlos Garcia Sogo reported on #git that a git-svn clone of this\n> svn repo fails for him:\n> https://sucs.org/~welshbyte/svn/backuptool/trunk\n>\n> I can reproduce that here with:\n> git-svn version 1.6.0.2.541.g46dc1.dirty (svn 1.5.1)\n>\n> The error message I get is:\n> Apache got a malformed URI: Unusable URI: it does not refer to this\n> repository at /usr/local/libexec/git-core/git-svn line 4057\n>\n> strace revealed that git-svn url-encodes ~ while svn does not do that.\n>\n> For svn we have:\n> write(5, \"\n> https://sucs.org/~welshbyte/svn/backuptool/trunk...\n>\n> While git-svn shows:\n> write(7, \"\n> https://sucs.org/%7Ewelshbyte/svn/backuptool/trunk...\n\nSigned-off-by: Eric Wong \n---\n git-svn.perl | 6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex ef6d773..a97049a 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -852,7 +852,7 @@ sub escape_uri_only {\n \tmy ($uri) = @_;\n \tmy @tmp;\n \tforeach (split m{/}, $uri) {\n-\t\ts/([^\\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf(\"%%%02X\",ord($1))/eg;\n+\t\ts/([^~\\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf(\"%%%02X\",ord($1))/eg;\n \t\tpush @tmp, $_;\n \t}\n \tjoin('/', @tmp);\n@@ -3537,7 +3537,7 @@ sub repo_path {\n sub url_path {\n \tmy ($self, $path) = @_;\n \tif ($self->{url} =~ m#^https?://#) {\n-\t\t$path =~ s/([^a-zA-Z0-9_.-])/uc sprintf(\"%%%02x\",ord($1))/eg;\n+\t\t$path =~ s/([^~a-zA-Z0-9_.-])/uc sprintf(\"%%%02x\",ord($1))/eg;\n \t}\n \t$self->{url} . '/' . $self->repo_path($path);\n }\n@@ -3890,7 +3890,7 @@ sub escape_uri_only {\n \tmy ($uri) = @_;\n \tmy @tmp;\n \tforeach (split m{/}, $uri) {\n-\t\ts/([^\\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf(\"%%%02X\",ord($1))/eg;\n+\t\ts/([^~\\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf(\"%%%02X\",ord($1))/eg;\n \t\tpush @tmp, $_;\n \t}\n \tjoin('/', @tmp);\n-- \nEric Wong\n"},{"id":"93638","messageId":"7vtzb5wr6v.fsf@gitster.siamese.dyndns.org","threadId":"15955","inReplyTo":"20081021211131.GA21606@yp-box.dyndns.org","subject":"Re: [PATCH] git-svn: don't escape tilde ('~') for http(s) URLs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-10-21T21:53:28Z","receivedAt":"2008-10-21T21:53:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n>> strace revealed that git-svn url-encodes ~ while svn does not do that.\n>>\n>> For svn we have:\n>> write(5, \"\n>> https://sucs.org/~welshbyte/svn/backuptool/trunk...\n>>\n>> While git-svn shows:\n>> write(7, \"\n>> https://sucs.org/%7Ewelshbyte/svn/backuptool/trunk...\n\nThis looks like an XML based request sequence to me (and svn is talking\nWebDAV here, right?); it makes me wonder what exact quoting rules are used\nthere. I would expect $path in $path to quote\na letters in it e.g. '<' as \"<\" --- which is quite different from what\nthe s/// substitutions in the patch seem to be doing.\n\n> diff --git a/git-svn.perl b/git-svn.perl\n> index ef6d773..a97049a 100755\n> --- a/git-svn.perl\n> +++ b/git-svn.perl\n> @@ -852,7 +852,7 @@ sub escape_uri_only {\n> -\t\ts/([^\\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf(\"%%%02X\",ord($1))/eg;\n> +\t\ts/([^~\\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf(\"%%%02X\",ord($1))/eg;\n\nAdmittedly I do not know git-svn (nor Perl svn bindings it uses), and I\nsuspect that some of the XML-level escaping is done in the libsvn side,\nbut it would be nice if somebody can at least verify that the code after\nthe patch works with repositories with funny characters in pathnames\n(perhaps list all the printables including \"<&>?*!@.+-%^\"). Even nicer\nwould be a log message that says \"the resulting code covers all cases\nbecause it follows _that_ spec to escape _all_ problematic letters\",\npointing at some in svn (or libsvn-perl) resource.\n\nThe patch may make a path with '~' work, but it (neither in the patch text\nnor in the commit log message) does not have much to give readers enough\nconfidence that the code after the patch is the _final_ one, as opposed to\nbeing just a band-aid for a single symptom that happened to have been\ndiscovered this time.\n"},{"id":"93649","messageId":"20081022061325.GC8225@glandium.org","threadId":"15955","inReplyTo":"7vtzb5wr6v.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-svn: don't escape tilde ('~') for http(s) URLs","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2008-10-22T06:13:25Z","receivedAt":"2008-10-22T06:13:25Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Tue, Oct 21, 2008 at 02:53:28PM -0700, Junio C Hamano wrote:\n> Eric Wong writes:\n> \n> >> strace revealed that git-svn url-encodes ~ while svn does not do that.\n> >>\n> >> For svn we have:\n> >> write(5, \"\n> >> https://sucs.org/~welshbyte/svn/backuptool/trunk...\n> >>\n> >> While git-svn shows:\n> >> write(7, \"\n> >> https://sucs.org/%7Ewelshbyte/svn/backuptool/trunk...\n> \n> This looks like an XML based request sequence to me (and svn is talking\n> WebDAV here, right?);\n\nXML based would be &126;, not %7E.\nAnyways, aren't there ready-to-use url quoting functions in perl ?\n\nMike\n"},{"id":"93651","messageId":"7vskqpuoyb.fsf@gitster.siamese.dyndns.org","threadId":"15955","inReplyTo":"20081022061325.GC8225@glandium.org","subject":"Re: [PATCH] git-svn: don't escape tilde ('~') for http(s) URLs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-10-22T06:24:44Z","receivedAt":"2008-10-22T06:24:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mike Hommey writes:\n\n> On Tue, Oct 21, 2008 at 02:53:28PM -0700, Junio C Hamano wrote:\n>\n>> >> For svn we have:\n>> >> write(5, \"\n>> >> https://sucs.org/~welshbyte/svn/backuptool/trunk...\n>> >>\n>> >> While git-svn shows:\n>> >> write(7, \"\n>> >> https://sucs.org/%7Ewelshbyte/svn/backuptool/trunk...\n>> \n>> This looks like an XML based request sequence to me (and svn is talking\n>> WebDAV here, right?);\n>\n> XML based would be &126;, not %7E.\n\nRead what you quoted again and realize you are agreeing with me ;-).\n\nThe former (with \"~\") is what svn expects, the latter (with %7E) is what\ngit-svn incorrectly threw at the server causing problems. I am wondering\nthe whole %xx escaping thing, which does not seem to match what svn seems\nto expect.\n\n> Anyways, aren't there ready-to-use url quoting functions in perl ?\n\nMy question is not about correct \"url quoting\", but if using \"url quoting\"\nis correct in this context.\n"},{"id":"93657","messageId":"20081022081653.GC14966@untitled","threadId":"15955","inReplyTo":"7vtzb5wr6v.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-svn: don't escape tilde ('~') for http(s) URLs","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2008-10-22T08:16:53Z","receivedAt":"2008-10-22T08:16:53Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Junio C Hamano wrote:\n> Eric Wong writes:\n> \n> >> strace revealed that git-svn url-encodes ~ while svn does not do that.\n> >>\n> >> For svn we have:\n> >> write(5, \"\n> >> https://sucs.org/~welshbyte/svn/backuptool/trunk...\n> >>\n> >> While git-svn shows:\n> >> write(7, \"\n> >> https://sucs.org/%7Ewelshbyte/svn/backuptool/trunk...\n> \n> This looks like an XML based request sequence to me (and svn is talking\n> WebDAV here, right?); it makes me wonder what exact quoting rules are used\n> there. I would expect $path in $path to quote\n> a letters in it e.g. '<' as \"<\" --- which is quite different from what\n> the s/// substitutions in the patch seem to be doing.\n\nI agree. I haven't checked if the SVN libraries do proper XML escaping\nfor us (but the problem hasn't shown up yet :). I was already\ncompletely disappointed that git-svn had to do its own escaping when\ntransmitting data using the SVN libraries (and dependent on the protocol\nbeing used, too!).\n\n> > diff --git a/git-svn.perl b/git-svn.perl\n> > index ef6d773..a97049a 100755\n> > --- a/git-svn.perl\n> > +++ b/git-svn.perl\n> > @@ -852,7 +852,7 @@ sub escape_uri_only {\n> > -\t\ts/([^\\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf(\"%%%02X\",ord($1))/eg;\n> > +\t\ts/([^~\\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf(\"%%%02X\",ord($1))/eg;\n> \n> Admittedly I do not know git-svn (nor Perl svn bindings it uses), and I\n> suspect that some of the XML-level escaping is done in the libsvn side,\n> but it would be nice if somebody can at least verify that the code after\n> the patch works with repositories with funny characters in pathnames\n> (perhaps list all the printables including \"<&>?*!@.+-%^\"). Even nicer\n> would be a log message that says \"the resulting code covers all cases\n> because it follows _that_ spec to escape _all_ problematic letters\",\n> pointing at some in svn (or libsvn-perl) resource.\n\nHelp with looking at what SVN does and writing testcases would\ndefinitely be appreciated on this matter. Or perhaps this can be done\nat GitTogether :)\n\n> The patch may make a path with '~' work, but it (neither in the patch text\n> nor in the commit log message) does not have much to give readers enough\n> confidence that the code after the patch is the _final_ one, as opposed to\n> being just a band-aid for a single symptom that happened to have been\n> discovered this time.\n\nThis is definitely a band-aid fix until I or somebody else takes the\ntime to figure out:\n\n 1. exactly which characters need to be escaped\n 2. for which protocols those characters need to be escaped\n 3. which part(s) of the URI they need to be escaped for\n (repository root vs SVN path)\n 4. which versions of SVN needs more (or less) escaping rules\n\n(I vote for somebody else, especially for #4 :)\n\n-- \nEric Wong\n"},{"id":"93705","messageId":"7viqrkv4vd.fsf@gitster.siamese.dyndns.org","threadId":"15955","inReplyTo":"20081022081653.GC14966@untitled","subject":"Re: [PATCH] git-svn: don't escape tilde ('~') for http(s) URLs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-10-22T18:53:10Z","receivedAt":"2008-10-22T18:53:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n> Junio C Hamano wrote:\n>\n> Help with looking at what SVN does and writing testcases would\n> definitely be appreciated on this matter. Or perhaps this can be done\n> at GitTogether :)\n\nI'm not sure it would be a good use of time at GitTogether to do something\nwhose spec is pretty much self-evident (essentially for this one it boils\ndown to \"define what are the 'funny' bytes, and list the protocols\nsupported by svn, and come up with paths with funny bytes in it and \nsee what libsvn-perl gives to the underlying svn library, and what the svn\nlibrary does over the wire\"). Ongoiong \"fix start-up sequence around\nworktree area\" might be a better fit; I dunno.\n\n>> The patch may make a path with '~' work, but it (neither in the patch text\n>> nor in the commit log message) does not have much to give readers enough\n>> confidence that the code after the patch is the _final_ one, as opposed to\n>> being just a band-aid for a single symptom that happened to have been\n>> discovered this time.\n>\n> This is definitely a band-aid fix until I or somebody else takes the\n> time to figure out:\n>\n> 1. exactly which characters need to be escaped\n> 2. for which protocols those characters need to be escaped\n> 3. which part(s) of the URI they need to be escaped for\n> (repository root vs SVN path)\n> 4. which versions of SVN needs more (or less) escaping rules\n>\n> (I vote for somebody else, especially for #4 :)\n\nItem 3. above disturbs me. Do you mean that in:\n\n https://sucs.org/~welshbyte/svn/backuptool/trunk/foo~bar.txt\n\nthe two tildes might have to be sent to libsvn-perl differently?\n\nEven if that is the case, I am inclined suggest taking the patch in the\nmeantime as an interim workaround, with the understanding that we know the\npatch improves the situation for the tilde before welshbyte and even\nthough we do not know if the patch regresses for the latter one between\nfoo and bar, it would be much rarer to have tilde in such places.\n\nCare to come up with an updated log message?\n"},{"id":"93839","messageId":"20081024091330.GA27362@untitled","threadId":"15955","inReplyTo":"7viqrkv4vd.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-svn: don't escape tilde ('~') for http(s) URLs","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2008-10-24T09:13:30Z","receivedAt":"2008-10-24T09:13:30Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Junio C Hamano wrote:\n> Eric Wong writes:\n> > Junio C Hamano wrote:\n> >> The patch may make a path with '~' work, but it (neither in the patch text\n> >> nor in the commit log message) does not have much to give readers enough\n> >> confidence that the code after the patch is the _final_ one, as opposed to\n> >> being just a band-aid for a single symptom that happened to have been\n> >> discovered this time.\n> >\n> > This is definitely a band-aid fix until I or somebody else takes the\n> > time to figure out:\n> >\n> > 1. exactly which characters need to be escaped\n> > 2. for which protocols those characters need to be escaped\n> > 3. which part(s) of the URI they need to be escaped for\n> > (repository root vs SVN path)\n> > 4. which versions of SVN needs more (or less) escaping rules\n> >\n> > (I vote for somebody else, especially for #4 :)\n> \n> Item 3. above disturbs me. Do you mean that in:\n> \n> https://sucs.org/~welshbyte/svn/backuptool/trunk/foo~bar.txt\n> \n> the two tildes might have to be sent to libsvn-perl differently?\n\nYes, something like this is unfortunately a possibility (as is\nhaving to worry about this at all in git-svn).\n\n> Even if that is the case, I am inclined suggest taking the patch in the\n> meantime as an interim workaround, with the understanding that we know the\n> patch improves the situation for the tilde before welshbyte and even\n> though we do not know if the patch regresses for the latter one between\n> foo and bar, it would be much rarer to have tilde in such places.\n> \n> Care to come up with an updated log message?\n\nFrom aa4f2cdcf64934e13886fabb3b5e986a5cda79f6 Mon Sep 17 00:00:00 2001\nFrom: Eric Wong \nDate: Tue, 21 Oct 2008 14:12:15 -0700\nSubject: [PATCH] git-svn: don't escape tilde ('~') for http(s) URLs\nMIME-Version: 1.0\nContent-Type: text/plain; charset=utf-8\nContent-Transfer-Encoding: 8bit\n\nThis change only fixes the tilde case in the repository URL. A\nmore comprehensive set of escaping rules and tests will be\nneeded in the future for complete compatibility when using\nuncommon characters.\n\nThanks to Jose Carlos Garcia Sogo and Björn Steinbrink for the\nbug report.\n\nOn 2008.10.18 23:39:19 +0200, Björn Steinbrink wrote:\n> Hi,\n>\n> Jose Carlos Garcia Sogo reported on #git that a git-svn clone of this\n> svn repo fails for him:\n> https://sucs.org/~welshbyte/svn/backuptool/trunk\n>\n> I can reproduce that here with:\n> git-svn version 1.6.0.2.541.g46dc1.dirty (svn 1.5.1)\n>\n> The error message I get is:\n> Apache got a malformed URI: Unusable URI: it does not refer to this\n> repository at /usr/local/libexec/git-core/git-svn line 4057\n>\n> strace revealed that git-svn url-encodes ~ while svn does not do that.\n>\n> For svn we have:\n> write(5, \"\n> https://sucs.org/~welshbyte/svn/backuptool/trunk...\n>\n> While git-svn shows:\n> write(7, \"\n> https://sucs.org/%7Ewelshbyte/svn/backuptool/trunk...\n\nSigned-off-by: Eric Wong \n---\n git-svn.perl | 6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex ef6d773..a97049a 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -852,7 +852,7 @@ sub escape_uri_only {\n \tmy ($uri) = @_;\n \tmy @tmp;\n \tforeach (split m{/}, $uri) {\n-\t\ts/([^\\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf(\"%%%02X\",ord($1))/eg;\n+\t\ts/([^~\\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf(\"%%%02X\",ord($1))/eg;\n \t\tpush @tmp, $_;\n \t}\n \tjoin('/', @tmp);\n@@ -3537,7 +3537,7 @@ sub repo_path {\n sub url_path {\n \tmy ($self, $path) = @_;\n \tif ($self->{url} =~ m#^https?://#) {\n-\t\t$path =~ s/([^a-zA-Z0-9_.-])/uc sprintf(\"%%%02x\",ord($1))/eg;\n+\t\t$path =~ s/([^~a-zA-Z0-9_.-])/uc sprintf(\"%%%02x\",ord($1))/eg;\n \t}\n \t$self->{url} . '/' . $self->repo_path($path);\n }\n@@ -3890,7 +3890,7 @@ sub escape_uri_only {\n \tmy ($uri) = @_;\n \tmy @tmp;\n \tforeach (split m{/}, $uri) {\n-\t\ts/([^\\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf(\"%%%02X\",ord($1))/eg;\n+\t\ts/([^~\\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf(\"%%%02X\",ord($1))/eg;\n \t\tpush @tmp, $_;\n \t}\n \tjoin('/', @tmp);\n-- \nEric Wong\n"}]}