[PATCH] Allow '+', '-' and '.' in remote helper names

[PATCH] Allow '+', '-' and '.' in remote helper names

[Ilari Liusvaara]

According to relevant RFCs, in addition to alphanumerics, the following
characters are valid in URL scheme parts: '+', '-' and '.', but
currently only alphanumerics are allowed in remote helper names.

Allow those three characters in remote helper names (both 'foo://' and
'foo::' syntax).

Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
---
 transport.c |   19 +++++++++++++++++--
 1 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/transport.c b/transport.c
index 08e4fa0..00acff1 100644
--- a/transport.c
+++ b/transport.c
@@ -872,6 +872,21 @@ static int is_file(const char *url)
 	return S_ISREG(buf.st_mode);
 }
 
+static int isurlschemechar(int first_flag, int ch)
+{
+	/*
+	 * The set of valid URL schemes, as per STD66 (RFC3986) is
+	 * '[A-Za-z][A-Za-z0-9+.-]*'. But use sightly looser check
+	 * of '[A-Za-z0-9][A-Za-z0-9+.-]*' because earlier version
+	 * of check used '[A-Za-z0-9]+' so not to break any remote
+	 * helpers.
+	 */
+	int alphanumeric, special;
+	alphanumeric = ch > 0 && isalnum(ch);
+	special = ch == '+' || ch == '-' || ch == '.';
+	return alphanumeric || (!first_flag && special);
+}
+
 static int is_url(const char *url)
 {
 	const char *url2, *first_slash;
@@ -896,7 +911,7 @@ static int is_url(const char *url)
 	 */
 	url2 = url;
 	while (url2 < first_slash - 1) {
-		if (!isalnum((unsigned char)*url2))
+		if (!isurlschemechar(url2 == url, (unsigned char)*url2))
 			return 0;
 		url2++;
 	}
@@ -930,7 +945,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
 	if (url) {
 		const char *p = url;
 
-		while (isalnum(*p))
+		while (isurlschemechar(p == url, *p))
 			p++;
 		if (!prefixcmp(p, "::"))
 			helper = xstrndup(url, p - url);
-- 
1.7.0.86.gb35cab

Re: [PATCH] Allow '+', '-' and '.' in remote helper names

[Johannes Schindelin]

Hi,

On Tue, 23 Feb 2010, Ilari Liusvaara wrote:

> According to relevant RFCs, in addition to alphanumerics, the following
> characters are valid in URL scheme parts: '+', '-' and '.', but
> currently only alphanumerics are allowed in remote helper names.

May I caution against allowing "+" as part of filenames? On Windows, 
thanks to the DOS garb^Wheritage, "+" is not really allowed...

Or maybe I misunderstood the intent of the patch?

Ciao,
Dscho

Re: [PATCH] Allow '+', '-' and '.' in remote helper names

[Sverre Rabbelier]

Heya,

On Tue, Feb 23, 2010 at 14:03, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> May I caution against allowing "+" as part of filenames? On Windows,
> thanks to the DOS garb^Wheritage, "+" is not really allowed...

Would it be safe to say "raplce all occurences of '+' and '.' with
'-'? Is it feasible that we would want to support two protocols with a
different helper that map to the same 'name' using that scheme? So,
would there ever be a case where we want to handle "bzr+ssh"
and "bzr-ssh" with a different helper? I reckon not, and if it does
occur it's always possible to put a simple dispatcher in between?

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH] Allow '+', '-' and '.' in remote helper names

[Johannes Schindelin]

Hi,

On Tue, 23 Feb 2010, Sverre Rabbelier wrote:

> On Tue, Feb 23, 2010 at 14:03, Johannes Schindelin 
> <Johannes.Schindelin@gmx.de> wrote:
> > May I caution against allowing "+" as part of filenames? On Windows, 
> > thanks to the DOS garb^Wheritage, "+" is not really allowed...
> 
> Would it be safe to say "raplce all occurences of '+' and '.' with
> '-'?

I'd rather only replace '+' with '-'. While "short" DOS filenames can only 
have one dot, and only up to three (upper-case) characters after that, 
"long" ones do not share that restriction.

> Is it feasible that we would want to support two protocols with a 
> different helper that map to the same 'name' using that scheme? So, 
> would there ever be a case where we want to handle "bzr+ssh" and 
> "bzr-ssh" with a different helper? I reckon not, and if it does occur 
> it's always possible to put a simple dispatcher in between?

Possible.

Ciao,
Dscho

Re: [PATCH] Allow '+', '-' and '.' in remote helper names

[Johannes Sixt]

Johannes Schindelin schrieb:
> May I caution against allowing "+" as part of filenames? On Windows, 
> thanks to the DOS garb^Wheritage, "+" is not really allowed...

I don't think that's true (and I also think it never was).

You may be refering to the copy command, where you can say

   copy a.txt+b.txt+c.txt abc.txt

to concatenate the source files. But this does not restrict how you can
name your files. If you have a file a+b.txt and want to use it with copy,
then you must use double-quotes:

   copy "a+b.txt" elsewhere.txt
   copy "a+b.txt"+c.txt abc.txt

-- Hannes

Re: [PATCH] Allow '+', '-' and '.' in remote helper names

[Erik Faye-Lund]

On Tue, Feb 23, 2010 at 2:19 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Johannes Schindelin schrieb:
>> May I caution against allowing "+" as part of filenames? On Windows,
>> thanks to the DOS garb^Wheritage, "+" is not really allowed...
>
> I don't think that's true (and I also think it never was).
>

Wikipedia doesn't seem to think so either:
http://en.wikipedia.org/wiki/Filename#Reserved_characters_and_words

-- 
Erik "kusma" Faye-Lund

Re: [PATCH] Allow '+', '-' and '.' in remote helper names

[Johannes Schindelin]

Hi,

On Tue, 23 Feb 2010, Erik Faye-Lund wrote:

> On Tue, Feb 23, 2010 at 2:19 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> > Johannes Schindelin schrieb:
> >> May I caution against allowing "+" as part of filenames? On Windows,
> >> thanks to the DOS garb^Wheritage, "+" is not really allowed...
> >
> > I don't think that's true (and I also think it never was).
> >
> 
> Wikipedia doesn't seem to think so either:
> http://en.wikipedia.org/wiki/Filename#Reserved_characters_and_words

Okay, I stand corrected, and therefore gladly retract my objections!

Ciao,
Dscho

Re: [PATCH] Allow '+', '-' and '.' in remote helper names

[Paolo Bonzini]

On 02/23/2010 01:33 PM, Ilari Liusvaara wrote:
> According to relevant RFCs, in addition to alphanumerics, the following
> characters are valid in URL scheme parts: '+', '-' and '.', but
> currently only alphanumerics are allowed in remote helper names.
>
> Allow those three characters in remote helper names (both 'foo://' and
> 'foo::' syntax).

I think '+' could be special-cased in that, for example, "svn+ssh://" 
should still invoke an hypothetic git-remote-svn helper.  There is no 
use yet for this feature, but I'm sure that foreign VCS helpers would 
use it.

Paolo

Re: [PATCH] Allow '+', '-' and '.' in remote helper names

[Gabriel Filion]

Paolo Bonzini wrote:
> On 02/23/2010 01:33 PM, Ilari Liusvaara wrote:
>> According to relevant RFCs, in addition to alphanumerics, the following
>> characters are valid in URL scheme parts: '+', '-' and '.', but
>> currently only alphanumerics are allowed in remote helper names.
>>
>> Allow those three characters in remote helper names (both 'foo://' and
>> 'foo::' syntax).
> 
> I think '+' could be special-cased in that, for example, "svn+ssh://"
> should still invoke an hypothetic git-remote-svn helper.  There is no
> use yet for this feature, but I'm sure that foreign VCS helpers would
> use it.
> 

Special-casing the + could be useful to simplify support for
"svn::ssh://"-style addresses as both could receive the same URL
("ssh://..").

It would also mean less clutter in the script directory. Instead of
having two scripts, one to catch "helper://.." remote URLs and another
to catch "helper+ssh://.." URLs, the same script would differentiate
between used protocols.

-- 
Gabriel Filion