Re: [REROLL PATCH 3/8] Pass unknown protocols to external protocol handlers
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 8, 2009, 23:35 UTC
- Message-ID
- <7vskbl59ai.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1260278177-9029-4-git-send-email-ilari.liusvaara@elisanet.fi>
Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:
Show 19 quoted lines
> +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;Show 17 quoted lines
> 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()...
Show 22 quoted lines
> +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?
Show 24 quoted lines
> 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?
Show 19 quoted lines
> @@ -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.