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

[PATCH] serf/ra_serf: add cansend callback to stop writing requests untill authn handshake is finished

From: Lieven Govaerts <lgo_at_mobsol.be>
Date: 2007-07-24 23:29:43 CEST

Attached two patches implement a new callback 'cansend' in serf which is
used in ra_serf to hold sending a bunch of requests before the NTLM
authentication handshake is finished.

We need this callback in this scenario: consider an apache setup with
NTLM authentication MaxRequestsPerChild set to 100. Now use svn to
checkout a directory with more than 50 files. Serf will make 100
requests (PROPFIND+GET) for all those files and sends them on one of the
connections. What happens is that the before the NTLM handshake is
finished, the connection will already max out on the number of requests.

The patch for serf adds the callback and uses it in
context.c/write_connection. The callback can be used for all connection
based authentication protocols, the NTLM part is the first
implementation using it.

The serf patch changes the serf_connection_create function declaration,
I suppose there's no API compatibility guarantee before serf 1.0?

Note that the ra_serf patch applies to current HEAD of the ra_serf-auth
branch of Subversion, While after this patch the code will be complete,
it's still not stable enough to integrate back to trunk.

Note2: I propose you add 'svn:eol-style' 'native' to the serf source
files, makes it a bit easier to patch them on Windows.

Lieven

[[[
Implement 'cansend' callback to wait before starting to send a bunch of
requests to the server until the NTLM handshake has finished.

* subversion/libsvn_ra_serf/auth.c
  (serf_auth_protocols): add cansend callback for NTLM, not used for basic
   authentication.

* subversion/libsvn_ra_serf/ra_serf.h
  (svn_ra_serf__conn_cansend): new function declaration
  (svn_serf__setup_cansend_func_t): new function typedef
  (serf_auth_protocol_t): add extra field for the 'cansend' callback

* subversion/libsvn_ra_serf/util.c
  (svn_ra_serf__conn_cansend): new function, wraps cansend callback for
   authentication protocols.
  
* subversion/libsvn_ra_serf/serf.c
  (svn_ra_serf__open): add cansend callback info in call to
   serf_connection_create.

* subversion/libsvn_ra_serf/update.c
  (finish_report): ditto

* subversion/libsvn_ra_serf/win32_auth_sspi.c
  (cansend_sspi): new function, implements cansend callback for ntlm

* subversion/libsvn_ra_serf/win32_auth_sspi.h
  (cansend_sspi): new function declaration.
]]]

Index: subversion/libsvn_ra_serf/auth.c
===================================================================
--- subversion/libsvn_ra_serf/auth.c (revision 25831)
+++ subversion/libsvn_ra_serf/auth.c (working copy)
@@ -53,6 +53,7 @@
     init_basic_connection,
     handle_basic_auth,
     setup_request_basic_auth,
+ NULL, /* cansend = TRUE */
   },
 #ifdef WIN32
   {
@@ -60,6 +61,7 @@
     init_sspi_connection,
     handle_sspi_auth,
     setup_request_sspi_auth,
+ cansend_sspi,
   },
 #endif /* WIN32 */
 
Index: subversion/libsvn_ra_serf/ra_serf.h
===================================================================
--- subversion/libsvn_ra_serf/ra_serf.h (revision 25831)
+++ subversion/libsvn_ra_serf/ra_serf.h (working copy)
@@ -255,6 +255,11 @@
                          apr_status_t why,
                          apr_pool_t *pool);
 
+int
+svn_ra_serf__conn_cansend(serf_connection_t *conn,
+ void *cansend_baton,
+ apr_pool_t *pool);
+
 apr_status_t
 svn_ra_serf__is_conn_closing(serf_bucket_t *response);
 
@@ -1109,6 +1114,16 @@
                                   serf_bucket_t *hdrs_bkt);
 
 /**
+ * For each authentication protocol we need a cansend function of type
+ * svn_serf__setup_cansend_func_t. This function will be called when serf
+ * wants to send pending requests to the server. It can be used with
+ * connection-based authentication to temporarely block sending requests until
+ * authentication is finished.
+ */
+typedef svn_boolean_t
+(*svn_serf__setup_cansend_func_t)(svn_ra_serf__connection_t *conn);
+
+/**
  * serf_auth_protocol_t: vtable for an authentication protocol provider.
  *
  */
@@ -1125,6 +1140,11 @@
 
   /* Function to set up the authentication header of a request */
   svn_serf__setup_request_func_t setup_request_func;
+
+ /* Function that decides if requests can be send on the connection. If
+ cansend_func is NULL, ra_serf assumes that it can send requests at
+ all times */
+ svn_serf__setup_cansend_func_t cansend_func;
 };
 
 /**
Index: subversion/libsvn_ra_serf/serf.c
===================================================================
--- subversion/libsvn_ra_serf/serf.c (revision 25831)
+++ subversion/libsvn_ra_serf/serf.c (working copy)
@@ -175,6 +175,7 @@
       serf_connection_create(serf_sess->context, serf_sess->conns[0]->address,
                              svn_ra_serf__conn_setup, serf_sess->conns[0],
                              svn_ra_serf__conn_closed, serf_sess->conns[0],
+ svn_ra_serf__conn_cansend, serf_sess->conns[0],
                              serf_sess->pool);
 
   serf_sess->num_conns = 1;
Index: subversion/libsvn_ra_serf/update.c
===================================================================
--- subversion/libsvn_ra_serf/update.c (revision 25831)
+++ subversion/libsvn_ra_serf/update.c (working copy)
@@ -2120,6 +2120,8 @@
                                                     sess->conns[i],
                                                     svn_ra_serf__conn_closed,
                                                     sess->conns[i],
+ svn_ra_serf__conn_cansend,
+ sess->conns[i],
                                                     sess->pool);
       sess->num_conns++;
   }
Index: subversion/libsvn_ra_serf/util.c
===================================================================
--- subversion/libsvn_ra_serf/util.c (revision 25831)
+++ subversion/libsvn_ra_serf/util.c (working copy)
@@ -101,6 +101,24 @@
   return response;
 }
 
+int
+svn_ra_serf__conn_cansend(serf_connection_t *conn,
+ void *cansend_baton,
+ apr_pool_t *pool)
+{
+ svn_ra_serf__connection_t *our_conn = cansend_baton;
+
+ /* Allow sending requests when no authentication protocol is defined yet. */
+ if (! our_conn->session->auth_protocol)
+ return TRUE;
+
+ /* Allow sending requests when the authentication protocol doesn't care. */
+ if (! our_conn->session->auth_protocol->cansend_func)
+ return TRUE;
+
+ return our_conn->session->auth_protocol->cansend_func(our_conn);
+}
+
 void
 svn_ra_serf__conn_closed(serf_connection_t *conn,
                          void *closed_baton,
Index: subversion/libsvn_ra_serf/win32_auth_sspi.c
===================================================================
--- subversion/libsvn_ra_serf/win32_auth_sspi.c (revision 25831)
+++ subversion/libsvn_ra_serf/win32_auth_sspi.c (working copy)
@@ -202,6 +202,23 @@
   return SVN_NO_ERROR;
 }
 
+svn_boolean_t
+cansend_sspi(svn_ra_serf__connection_t *conn)
+{
+ /* Only allow sending requests when:
+ 1. Authorization hasn't started yet
+ 2. The next request will include authentication headers
+ 3. We successfully finished authentication */
+ if (conn->auth_header ||
+ conn->sspi_context->state == sspi_auth_not_started ||
+ conn->sspi_context->state == sspi_auth_completed)
+ {
+ return TRUE;
+ }
+
+ return FALSE;
+}
+
 svn_error_t *
 sspi_get_credentials(char *token, apr_size_t token_len, const char **buf,
                      apr_size_t *buf_len, serf_sspi_context_t *sspi_ctx)
Index: subversion/libsvn_ra_serf/win32_auth_sspi.h
===================================================================
--- subversion/libsvn_ra_serf/win32_auth_sspi.h (revision 25831)
+++ subversion/libsvn_ra_serf/win32_auth_sspi.h (working copy)
@@ -71,6 +71,12 @@
 setup_request_sspi_auth(svn_ra_serf__connection_t *conn,
                         serf_bucket_t *hdrs_bkt);
 
+/* Implement a slow start mechanism, so serf doesn't start sending a bunch of
+ requests to the server while the initial authentication handshake hasn't
+ finished yet for the connection */
+svn_boolean_t
+cansend_sspi(svn_ra_serf__connection_t *conn);
+
 /* Provides the necessary information for the http authentication headers
    for both the initial request to open an authentication connection, as
    the response to the server's authentication challenge.

[[[
Add a new callback 'cansend', which serf uses to check if it's ok to send a
request on a connection. This callback can be used with connection-based
authentication scheme's to avoid sending lots of requests to the server before
the authentication handshake has finished.

* context.c
  (struct serf_connection_t): add fields to store the cansend callback function
   and baton.
  (write_to_connection): call the callback before writing a request on the
   connection.
  (serf_connection_create): extra parameters for the cansend callback function
   and baton.

* serf.h
  (serf_connection_cansend_t): new function typedef
  (serf_connection_create): add extra parameters

* test/serf_get.c
  (main): update call to serf_connection_create, pass NULL to indicate the test
   is not using the cansend callback.

* test/serf_spider.c
  (main): ditto
]]]

Index: context.c
===================================================================
--- context.c (revision 1121)
+++ context.c (working copy)
@@ -128,6 +128,8 @@
     void *setup_baton;
     serf_connection_closed_t closed;
     void *closed_baton;
+ serf_connection_cansend_t cansend;
+ void *cansend_baton;
 };
 
 /* cleanup for sockets */
@@ -529,6 +531,10 @@
            request->req_bkt == NULL && request->setup == NULL)
         request = request->next;
 
+ if (conn->cansend &&
+ conn->cansend(conn, conn->cansend_baton, conn->pool) == 0)
+ return APR_SUCCESS;
+
     /* assert: request != NULL || conn->vec_len */
 
     /* Keep reading and sending until we run out of stuff to read, or
@@ -539,6 +545,13 @@
         apr_status_t status;
         apr_status_t read_status;
 
+ /* If this is a new request, check that the connection is ready. */
+ if ((request == NULL ||
+ (request != NULL && request->req_bkt == NULL)) &&
+ (conn->cansend &&
+ conn->cansend(conn, conn->cansend_baton, conn->pool) == 0))
+ return APR_SUCCESS;
+
         /* If we have unwritten data, then write what we can. */
         while (conn->vec_len) {
             status = socket_writev(conn);
@@ -984,6 +997,8 @@
     void *setup_baton,
     serf_connection_closed_t closed,
     void *closed_baton,
+ serf_connection_cansend_t cansend,
+ void *cansend_baton,
     apr_pool_t *pool)
 {
     serf_connection_t *conn = apr_pcalloc(pool, sizeof(*conn));
@@ -994,6 +1009,8 @@
     conn->setup_baton = setup_baton;
     conn->closed = closed;
     conn->closed_baton = closed_baton;
+ conn->cansend = cansend;
+ conn->cansend_baton = cansend_baton;
     conn->pool = pool;
     conn->allocator = serf_bucket_allocator_create(pool, NULL, NULL);
 
Index: serf.h
===================================================================
--- serf.h (revision 1121)
+++ serf.h (working copy)
@@ -192,6 +192,21 @@
                                          apr_pool_t *pool);
 
 /**
+ * This callback is used when serf wants to send a request on a @a conn
+ * connection.
+ * It can be used with connection-based authentication to
+ * temporarely block sending requests until authentication is finished.
+ * The return value is expected to be:
+ * 1 if serf can send the next request on the connection
+ * 0 if serf should stop sending requests on the connection temporarely.
+ *
+ * All temporary allocations should be made in @a pool.
+ */
+typedef int (*serf_connection_cansend_t)(serf_connection_t *conn,
+ void *cansend_baton,
+ apr_pool_t *pool);
+
+/**
  * Response data has arrived and should be processed.
  *
  * Whenever response data for @a request arrives (initially, or continued data
@@ -252,6 +267,8 @@
     void *setup_baton,
     serf_connection_closed_t closed,
     void *closed_baton,
+ serf_connection_cansend_t cansend,
+ void *cansend_baton,
     apr_pool_t *pool);
 
 /**
Index: test/serf_get.c
===================================================================
--- test/serf_get.c (revision 1121)
+++ test/serf_get.c (working copy)
@@ -366,6 +366,7 @@
     connection = serf_connection_create(context, address,
                                         conn_setup, &app_ctx,
                                         closed_connection, &app_ctx,
+ NULL, NULL,
                                         pool);
 
     handler_ctx.requests_outstanding = 0;
Index: test/serf_spider.c
===================================================================
--- test/serf_spider.c (revision 1121)
+++ test/serf_spider.c (working copy)
@@ -714,6 +714,7 @@
     connection = serf_connection_create(context, address,
                                         conn_setup, &app_ctx,
                                         closed_connection, &app_ctx,
+ NULL, NULL,
                                         pool);
 
     handler_ctx = (handler_baton_t*)serf_bucket_mem_alloc(app_ctx.bkt_alloc,

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Jul 24 23:25:11 2007

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