[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Allow custom user agent string

From: Stefan Küng <tortoisesvn_at_gmail.com>
Date: 2007-12-09 14:26:16 CET

Justin Erenkrantz wrote:
> On Dec 9, 2007 2:42 AM, Stefan Küng <tortoisesvn@gmail.com> wrote:
>> Well, I used the same approach as svn_wc_set_adm_dir() : that function
>> doesn't really need a pool argument either. IIRC, the discussion back
>> then was that all functions should take a pool argument, even if not
>> currently used so it can be extended more easily in the future.
>
> Well, used in the manner of this patch, a pool would be unsafe.

True. I've removed the pool argument.

>> AFAIK, there are not that many requests done for an operation. Which
>> means it shouldn't be a big problem?
>
> No - it is a big problem. There's at least 2x-per-file HTTP requests
> on an update (GET & PROPFIND).
>
> So, yah, that can be a lot of requests - so allocating from the
> session pool is badness - especially when we're dealing with a string
> that shouldn't change after initialization (based on the docstring).

I see. I only tested this with neon when I checked how much that code
part was called. That's why I didn't notice that. But it worked when I
ran some command with serf outside the debugger, so I assumed it was ok.

I've changed the patch to add a 'const char *useragent' to the
connection struct.

New patch attached.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net

[[[
Allow the user-agent string sent over http to be defined by the client.

* subversion/include/svn_ra.h
  (svn_ra_set_client_namestring): new function.
  (svn_ra_get_client_namestring): new function.

* subversion/libsvn_ra/clientstring.c : new file containing helper
                                        functions
* subversion/libsvn_ra_neon/session.c
  (svn_ra_neon__open) : use svn_ra_get_client_namestring to
                        create the useragent string.

* subversion/libsvn_ra_serf/ra_serf.h : add an entry for the useragent
                                        string to the connection struct.

* subversion/libsvn_ra_serf/propfind_buckets.c
  (become_request) : use the useragent string stored in the connection.

* subversion/libsvn_ra_serf/util.c
  (svn_ra_serf__conn_setup) : create the useragent string and store it
                              in the connection struct.
  (svn_ra_serf__setup_serf_req) : use the useragent string of the
                                  connection.

]]]
Index: subversion/include/svn_ra.h
===================================================================
--- subversion/include/svn_ra.h (revision 28326)
+++ subversion/include/svn_ra.h (working copy)
@@ -40,7 +40,28 @@
 
 /* Misc. declarations */
 
+
 /**
+ * Use @a name for the client string for http headers.
+ *
+ * @note This function changes global (per-process) state and must be
+ * called in a single-threaded context during the initialization of a
+ * Subversion client.
+ *
+ * @since New in 1.5.
+ */
+svn_error_t *
+svn_ra_set_client_namestring(const char *name);
+
+/**
+ * returns the svn client name string used in http headers
+ *
+ * @since New in 1.5.
+ */
+const char * svn_ra_get_client_namestring(void);
+
+
+/**
  * Get libsvn_ra version information.
  *
  * @since New in 1.1.
Index: subversion/libsvn_ra/clientstring.c
===================================================================
--- subversion/libsvn_ra/clientstring.c (revision 0)
+++ subversion/libsvn_ra/clientstring.c (revision 0)
@@ -0,0 +1,55 @@
+/*
+ * clientstring.c: client string handling for http headers.
+ *
+ * ====================================================================
+ * Copyright (c) 2007 CollabNet. All rights reserved.
+ *
+ * This software is licensed as described in the file COPYING, which
+ * you should have received as part of this distribution. The terms
+ * are also available at http://subversion.tigris.org/license-1.html.
+ * If newer versions of this license are posted there, you may use a
+ * newer version instead, at your option.
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals. For exact contribution history, see the revision
+ * history and logs, available at http://subversion.tigris.org/.
+ * ====================================================================
+ */
+
+/* ==================================================================== */
+
+/*** Includes. ***/
+#include <apr_pools.h>
+#include <apr_strings.h>
+
+#include "svn_error.h"
+#include "svn_error_codes.h"
+#include "svn_ra.h"
+
+
+/* The default client name string */
+static const char default_ra_client_name[] = "SVN";
+
+/* The client name string that is actually used. */
+static const char *ra_client_name = default_ra_client_name;
+
+
+
+const char *
+svn_ra_get_client_namestring()
+{
+ return ra_client_name;
+}
+
+
+svn_error_t *
+svn_ra_set_client_namestring(const char *name)
+{
+ apr_pool_t *rootpool;
+ if (apr_pool_create(&rootpool, NULL))
+ abort();
+
+ ra_client_name = apr_pstrdup(rootpool, name);
+
+ return SVN_NO_ERROR;
+}

Property changes on: subversion\libsvn_ra\clientstring.c
___________________________________________________________________
Added: svn:eol-style
   + native

Index: subversion/libsvn_ra_neon/session.c
===================================================================
--- subversion/libsvn_ra_neon/session.c (revision 28326)
+++ subversion/libsvn_ra_neon/session.c (working copy)
@@ -821,7 +821,15 @@
   unsigned int neon_auth_types = 0;
   neonprogress_baton_t *neonprogress_baton =
     apr_pcalloc(pool, sizeof(*neonprogress_baton));
+ const char * useragent = NULL;
 
+ useragent = apr_pstrcat(pool,
+ "SVN/",
+ SVN_VERSION,
+ "/",
+ svn_ra_get_client_namestring(pool),
+ NULL);
+
   /* Sanity check the URI */
   SVN_ERR(parse_url(uri, repos_URL));
 
@@ -930,8 +938,8 @@
     ne_set_read_timeout(sess2, timeout);
   }
 
- ne_set_useragent(sess, "SVN/" SVN_VERSION);
- ne_set_useragent(sess2, "SVN/" SVN_VERSION);
+ ne_set_useragent(sess, useragent);
+ ne_set_useragent(sess2, useragent);
 
   /* clean up trailing slashes from the URL */
   len = strlen(uri->path);
Index: subversion/libsvn_ra_serf/propfind_buckets.c
===================================================================
--- subversion/libsvn_ra_serf/propfind_buckets.c (revision 28326)
+++ subversion/libsvn_ra_serf/propfind_buckets.c (working copy)
@@ -158,7 +158,7 @@
   hdrs_bkt = serf_bucket_request_get_headers(bucket);
 
   serf_bucket_headers_setn(hdrs_bkt, "Host", ctx->conn->hostinfo);
- serf_bucket_headers_setn(hdrs_bkt, "User-Agent", USER_AGENT);
+ serf_bucket_headers_setn(hdrs_bkt, "User-Agent", ctx->conn->useragent);
   if (ctx->conn->using_compression == TRUE)
     {
       serf_bucket_headers_setn(hdrs_bkt, "Accept-Encoding", "gzip");
Index: subversion/libsvn_ra_serf/ra_serf.h
===================================================================
--- subversion/libsvn_ra_serf/ra_serf.h (revision 28326)
+++ subversion/libsvn_ra_serf/ra_serf.h (working copy)
@@ -105,6 +105,9 @@
   /* Current authorization value used for the proxy server; may be NULL */
   char *proxy_auth_value;
 
+ /* user agent string */
+ const char *useragent;
+
 } svn_ra_serf__connection_t;
 
 /*
Index: subversion/libsvn_ra_serf/util.c
===================================================================
--- subversion/libsvn_ra_serf/util.c (revision 28326)
+++ subversion/libsvn_ra_serf/util.c (working copy)
@@ -49,6 +49,13 @@
   serf_bucket_t *bucket;
   svn_ra_serf__connection_t *conn = baton;
 
+ conn->useragent = apr_pstrcat(pool,
+ "SVN/",
+ SVN_VERSION,
+ "/",
+ svn_ra_get_client_namestring(pool),
+ NULL);
+
   bucket = serf_bucket_socket_create(sock, conn->bkt_alloc);
   if (conn->using_ssl)
     {
@@ -271,7 +278,7 @@
 
   hdrs_bkt = serf_bucket_request_get_headers(*req_bkt);
   serf_bucket_headers_setn(hdrs_bkt, "Host", conn->hostinfo);
- serf_bucket_headers_setn(hdrs_bkt, "User-Agent", USER_AGENT);
+ serf_bucket_headers_setn(hdrs_bkt, "User-Agent", conn->useragent);
   if (content_type)
     {
       serf_bucket_headers_setn(hdrs_bkt, "Content-Type", content_type);

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Dec 9 14:26:38 2007

This is an archived mail posted to the Subversion Dev mailing list.