Re: [REROLL PATCH 3/8] Pass unknown protocols to external protocol handlers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ilari Liusvaara <ilari.liusvaara@xxxxxxxxxxx> writes:

> +const char *remove_ext_force(const char *url)
> +{
> +	const char *url2 = url;
> +	const char *first_colon = NULL;
> +
> +	if (!url)
> +		return NULL;
> +
> +	while (*url2 && !first_colon)
> +		if (*url2 == ':')
> +			first_colon = url2;
> +		else
> +			url2++;
> +
> +	if (first_colon && first_colon[1] == ':')
> +		return first_colon + 2;
> +	else
> +		return url;
> +}

Sorry, I am slow today, but is this any different from:

	if (url) {
		const char *colon = strchr(url, ':');
	        if (colon && colon[1] == ':')
	        	return url2 + 2;
	}
	return url;

> diff --git a/transport.c b/transport.c
> index 3eea836..e42a82b 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -780,6 +780,58 @@ static int is_file(const char *url)
>  	return S_ISREG(buf.st_mode);
>  }
>  
> +static const char *strchrc(const char *str, int c)
> +{
> +	while (*str)
> +		if (*str == c)
> +			return str;
> +		else
> +			str++;
> +	return NULL;
> +}

I cannot spot how this is different from strchr()...

> +static int is_url(const char *url)
> +{
> +	const char *url2, *first_slash;
> +
> +	if (!url)
> +		return 0;
> +	url2 = url;
> +	first_slash = strchrc(url, '/');
> +
> +	/* Input with no slash at all or slash first can't be URL. */
> +	if (!first_slash || first_slash == url)
> +		return 0;
> +	/* Character before must be : and next must be /. */
> +	if (first_slash[-1] != ':' || first_slash[1] != '/')
> +		return 0;
> +	/* There must be something before the :// */
> +	if (first_slash == url + 1)
> +		return 0;
> +	/*
> +	 * Check all characters up to first slash. Only alpha, num and
> +	 * colon (":") are allowed. ":" must be followed by ":" or "/".
> +	 */

Hmm, so "a::::bc:://" is ok?

Is the reason this does not to check the string up to (first_slash-1) for
a valid syntax for <scheme> (as in "<scheme>://") because this potentially
has "<helper>::" in front of it?

I cannot tell if you want to allow "<helper1>::<helper2>::<scheme>://..."
by reading this code.

> +static int external_specification_len(const char *url)
> +{
> +	return strchrc(url, ':') - url;
> +}

Does the caller guarantee url has at least one colon in it?

>  struct transport *transport_get(struct remote *remote, const char *url)
>  {
>  	struct transport *ret = xcalloc(1, sizeof(*ret));
> @@ -805,30 +857,23 @@ struct transport *transport_get(struct remote *remote, const char *url)
>  
>  	if (remote && remote->foreign_vcs) {
>  		transport_helper_init(ret, remote->foreign_vcs);
> +	} else if (!prefixcmp(url, "rsync:")) {
>  		ret->get_refs_list = get_refs_via_rsync;
>  		ret->fetch = fetch_objs_via_rsync;
>  		ret->push = rsync_transport_push;
>  	} else if (is_local(url) && is_file(url)) {
>  		struct bundle_transport_data *data = xcalloc(1, sizeof(*data));
>  		ret->data = data;
>  		ret->get_refs_list = get_refs_from_bundle;
>  		ret->fetch = fetch_refs_from_bundle;
>  		ret->disconnect = close_bundle;
> +	} else if (!is_url(url)
> +		|| !prefixcmp(url, "file://")
> +		|| !prefixcmp(url, "git://")
> +		|| !prefixcmp(url, "ssh://")
> +		|| !prefixcmp(url, "git+ssh://")
> +		|| !prefixcmp(url, "ssh+git://")) {
> +		/* These are builtin smart transports. */

Hmm, what is !is_url(url) at the beginning for, if this lists "builtin"
smart transports?

> @@ -845,6 +890,21 @@ struct transport *transport_get(struct remote *remote, const char *url)
>  		data->receivepack = "git-receive-pack";
>  		if (remote->receivepack)
>  			data->receivepack = remote->receivepack;
> +	} else if (!prefixcmp(url, "http://";)
> +		|| !prefixcmp(url, "https://";)
> +		|| !prefixcmp(url, "ftp://";)) {
> +		/* These three are just plain special. */
> +		transport_helper_init(ret, "curl");
> +#ifdef NO_CURL
> +		error("git was compiled without libcurl support.");
> +#endif
> +	} else {
> +		/* Unknown protocol in URL. Pass to external handler. */
> +		int len = external_specification_len(url);
> +		char *handler = xmalloc(len + 1);
> +		handler[len] = 0;
> +		strncpy(handler, url, len);
> +		transport_helper_init(ret, handler);

Hmph, external_specification_len() may get passed a colon-less string
after all, I think.



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]