[PATCH v2] config: add support for http.url.* settings

2013-07-11 Thread Kyle J. McKay
From: Kyle J. McKay mack...@gmail.com

The url value is considered a match to a url if the url value
is either an exact match or a prefix of the url which ends on a
path component boundary ('/').  So https://example.com/test; will
match https://example.com/test; and https://example.com/test/too;
but not https://example.com/testextra;.

Longer matches take precedence over shorter matches with
environment variable settings taking precedence over all.

With this configuration:

[http]
useragent = other-agent
noEPSV = true
[http https://example.com;]
useragent = example-agent
sslVerify = false
[http https://example.com/path;]
useragent = path-agent

The https://other.example.com/; url will have useragent
other-agent and sslVerify will be on.

The https://example.com/; url will have useragent
example-agent and sslVerify will be off.

The https://example.com/path/sub; url will have useragent
path-agent and sslVerify will be off.

All three of the examples will have noEPSV enabled.

Signed-off-by: Kyle J. McKay mack...@gmail.com
---

The credentials configuration values already support url-specific
configuration items in the form credential.url.*.  This patch
adds similar support for http configuration values.

 Documentation/config.txt |  11 
 http.c   | 140 +--
 2 files changed, 134 insertions(+), 17 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index b4d4887..3731a3a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1531,6 +1531,17 @@ http.useragent::
of common USER_AGENT strings (but not including those like git/1.7.1).
Can be overridden by the 'GIT_HTTP_USER_AGENT' environment variable.
 
+http.url.*::
+   Any of the http.* options above can be applied selectively to some urls.
+   For example http.https://example.com.useragent; would set the user
+   agent only for https connections to example.com.  The url value
+   matches a url if it is an exact match or a prefix of the url matching
+   at a '/' boundary.  Longer url matches take precedence over shorter
+   ones with the environment variable settings taking precedence over all.
+   Note that url must match the url exactly (other than possibly being a
+   prefix).  This means any user, password and/or port setting that appears
+   in a url must also appear in url to have a successful match.
+
 i18n.commitEncoding::
Character encoding the commit messages are stored in; Git itself
does not care per se, but this information is necessary e.g. when
diff --git a/http.c b/http.c
index 2d086ae..68dc789 100644
--- a/http.c
+++ b/http.c
@@ -30,6 +30,34 @@ static CURL *curl_default;
 
 char curl_errorstr[CURL_ERROR_SIZE];
 
+enum http_option_type {
+   opt_post_buffer = 0,
+   opt_min_sessions,
+#ifdef USE_CURL_MULTI
+   opt_max_requests,
+#endif
+   opt_ssl_verify,
+   opt_ssl_try,
+   opt_ssl_cert,
+#if LIBCURL_VERSION_NUM = 0x070903
+   opt_ssl_key,
+#endif
+#if LIBCURL_VERSION_NUM = 0x070908
+   opt_ssl_capath,
+#endif
+   opt_ssl_cainfo,
+   opt_low_speed,
+   opt_low_time,
+   opt_no_epsv,
+   opt_http_proxy,
+   opt_cookie_file,
+   opt_user_agent,
+   opt_passwd_req,
+   opt_max
+};
+
+static int http_option_maxlen[opt_max];
+
 static int curl_ssl_verify = -1;
 static int curl_ssl_try;
 static const char *ssl_cert;
@@ -141,34 +169,99 @@ static void process_curl_messages(void)
 }
 #endif
 
+static int http_options_url_match_prefix(const char *url, const char 
*url_prefix)
+{
+   /*
+* url_prefix matches url if url_prefix is an exact match for url or it
+* is a prefix of url and the match ends on a path component boundary.
+* url_prefix is considered to have an implicit '/' on the end for
+* matching purposes if it does not already and it is shorter than url.
+* the return value is the length of the match in characters (excluding
+* any final '/') or 0 for no match.
+*/
+   size_t url_len, url_prefix_len = strlen(url_prefix);
+   if (!url_prefix_len || strncmp(url, url_prefix, url_prefix_len))
+   return 0;
+   url_len = strlen(url);
+   if (url_len == url_prefix_len || url[url_prefix_len - 1] == '/' || 
url[url_prefix_len] == '/')
+   return url[url_prefix_len - 1] == '/' ? url_prefix_len - 1 : 
url_prefix_len;
+   return 0;
+}
+
 static int http_options(const char *var, const char *value, void *cb)
 {
-   if (!strcmp(http.sslverify, var)) {
+/*
+ * Macro to ignore matches with a match length less than previously seen
+ * for the same option type and to remember the largest match length seen so
+ * far for each option type
+ */
+#define CHECK_MATCHLEN(opt) \
+   if (http_option_maxlen[opt]  matchlen) return 0; \
+   else http_option_maxlen[opt] = 

Re: [PATCH v2] config: add support for http.url.* settings

2013-07-11 Thread Junio C Hamano
Kyle J. McKay mack...@gmail.com writes:

 Longer matches take precedence over shorter matches with
 environment variable settings taking precedence over all.

OK.

 diff --git a/Documentation/config.txt b/Documentation/config.txt
 index b4d4887..3731a3a 100644
 --- a/Documentation/config.txt
 +++ b/Documentation/config.txt
 @@ -1531,6 +1531,17 @@ http.useragent::
   of common USER_AGENT strings (but not including those like git/1.7.1).
   Can be overridden by the 'GIT_HTTP_USER_AGENT' environment variable.
  
 +http.url.*::
 + Any of the http.* options above can be applied selectively to some urls.
 + For example http.https://example.com.useragent; would set the user
 + agent only for https connections to example.com.  The url value
 + matches a url if it is an exact match or a prefix of the url matching
 + at a '/' boundary.  Longer url matches take precedence over shorter
 + ones with the environment variable settings taking precedence over all.
 + Note that url must match the url exactly (other than possibly being a
 + prefix).  This means any user, password and/or port setting that appears
 + in a url must also appear in url to have a successful match.
 +

OK.

 diff --git a/http.c b/http.c
 index 2d086ae..68dc789 100644
 --- a/http.c
 +++ b/http.c
 @@ -30,6 +30,34 @@ static CURL *curl_default;
  
  char curl_errorstr[CURL_ERROR_SIZE];
  
 +enum http_option_type {
 + opt_post_buffer = 0,
 + opt_min_sessions,
 +#ifdef USE_CURL_MULTI
 + opt_max_requests,
 +#endif
 + opt_ssl_verify,
 + opt_ssl_try,
 + opt_ssl_cert,
 +#if LIBCURL_VERSION_NUM = 0x070903
 + opt_ssl_key,
 +#endif
 +#if LIBCURL_VERSION_NUM = 0x070908
 + opt_ssl_capath,
 +#endif
 + opt_ssl_cainfo,
 + opt_low_speed,
 + opt_low_time,
 + opt_no_epsv,
 + opt_http_proxy,
 + opt_cookie_file,
 + opt_user_agent,
 + opt_passwd_req,
 + opt_max
 +};
 +
 +static int http_option_maxlen[opt_max];

I understand that this is to keep track of the length of the longest
one that has matched (hence the current candidate).  The name maxlen
captures the longest part, but has matched is somehow lost.

Can we have a better name here please, or at least a comment to
clarify what this variable keeps track of.

  static int curl_ssl_verify = -1;
  static int curl_ssl_try;
  static const char *ssl_cert;
 @@ -141,34 +169,99 @@ static void process_curl_messages(void)
  }
  #endif
  
 +static int http_options_url_match_prefix(const char *url, const char 
 *url_prefix)
 +{
 + /*
 +  * url_prefix matches url if url_prefix is an exact match for url or it
 +  * is a prefix of url and the match ends on a path component boundary.
 +  * url_prefix is considered to have an implicit '/' on the end for
 +  * matching purposes if it does not already and it is shorter than url.
 +  * the return value is the length of the match in characters (excluding
 +  * any final '/') or 0 for no match.
 +  */
 + size_t url_len, url_prefix_len = strlen(url_prefix);
 + if (!url_prefix_len || strncmp(url, url_prefix, url_prefix_len))
 + return 0;

Should url=http://git.or.xz/git; match url_prefix=http://git.or.xz/git/;?

 + url_len = strlen(url);
 + if (url_len == url_prefix_len || url[url_prefix_len - 1] == '/' || 
 url[url_prefix_len] == '/')
 + return url[url_prefix_len - 1] == '/' ? url_prefix_len - 1 : 
 url_prefix_len;

Overlong lines that are somewhat hard to read.

 + return 0;
 +}
 +
  static int http_options(const char *var, const char *value, void *cb)
  {
 - if (!strcmp(http.sslverify, var)) {
 +/*
 + * Macro to ignore matches with a match length less than previously seen
 + * for the same option type and to remember the largest match length seen so
 + * far for each option type
 + */
 +#define CHECK_MATCHLEN(opt) \
 + if (http_option_maxlen[opt]  matchlen) return 0; \
 + else http_option_maxlen[opt] = matchlen

Avoid defining a macro _inside_ a function.  Also if you can make it
a static function, that would be much easier to read.

 +
 + const char *url = (const char *)cb;
 + const char *key, *dot;
 + int matchlen = 0;

 + key = skip_prefix(var, http.);
 + if (!key)
 + return git_default_config(var, value, cb);
 +
 + /*
 +  * If the part following the leading http. contains a '.' then check
 +  * to see if the part between http. and the last '.' is a match to
 +  * url.  if it's not then ignore the setting.  Otherwise set key to
 +  * point to the option which is the final part after the final '.' and
 +  * use key in subsequent comparisons to determine the option type.
 +  */
 + dot = strrchr(key, '.');
 + if (dot) {
 + char *config_url = xmemdupz(key, dot - key);
 + matchlen = http_options_url_match_prefix(url, config_url);
 + free(config_url);

Yikes.  http_options_url_match_prefix() 

Re: [PATCH v2] config: add support for http.url.* settings

2013-07-11 Thread Kyle J. McKay

On Jul 11, 2013, at 12:26, Junio C Hamano wrote:

Kyle J. McKay mack...@gmail.com writes:


+static int http_option_maxlen[opt_max];


I understand that this is to keep track of the length of the longest
one that has matched (hence the current candidate).  The name maxlen
captures the longest part, but has matched is somehow lost.

Can we have a better name here please, or at least a comment to
clarify what this variable keeps track of.


Will do.  How about http_option_max_matched_len?

+static int http_options_url_match_prefix(const char *url, const  
char *url_prefix)

+{
+   /*
+	 * url_prefix matches url if url_prefix is an exact match for url  
or it
+	 * is a prefix of url and the match ends on a path component  
boundary.

+* url_prefix is considered to have an implicit '/' on the end for
+	 * matching purposes if it does not already and it is shorter  
than url.
+	 * the return value is the length of the match in characters  
(excluding

+* any final '/') or 0 for no match.
+*/
+   size_t url_len, url_prefix_len = strlen(url_prefix);
+   if (!url_prefix_len || strncmp(url, url_prefix, url_prefix_len))
+   return 0;


Should url=http://git.or.xz/git; match url_prefix=http://git.or.xz/git/ 
?


My initial thought was no.  But your example is persuasive and  
probably should match.  Fix forthcoming.



+   url_len = strlen(url);
+	if (url_len == url_prefix_len || url[url_prefix_len - 1] == '/'  
|| url[url_prefix_len] == '/')
+		return url[url_prefix_len - 1] == '/' ? url_prefix_len - 1 :  
url_prefix_len;


Overlong lines that are somewhat hard to read.


OK.


static int http_options(const char *var, const char *value, void *cb)
{
-   if (!strcmp(http.sslverify, var)) {
+/*
+ * Macro to ignore matches with a match length less than  
previously seen
+ * for the same option type and to remember the largest match  
length seen so

+ * far for each option type
+ */
+#define CHECK_MATCHLEN(opt) \
+   if (http_option_maxlen[opt]  matchlen) return 0; \
+   else http_option_maxlen[opt] = matchlen


Avoid defining a macro _inside_ a function.  Also if you can make it
a static function, that would be much easier to read.


Was modeled after the credential_match() function from credential.c  
that defines a CHECK macro inside it and then undefines it at the end  
of the function.  I will change it to a static function.



+   if (dot) {
+   char *config_url = xmemdupz(key, dot - key);
+   matchlen = http_options_url_match_prefix(url, config_url);
+   free(config_url);


Yikes.  http_options_url_match_prefix() could take a counted string
to config_url to avoid this repeated allocation and deallocation,
no?


Again, modeled after the credential_config_callback() function from  
credential.c that does this exact thing.


I will update http_options_url_match_prefix to take a size_t.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html