Author: brane
Date: Sun Jun 29 13:37:25 2025
New Revision: 1926858
URL: http://svn.apache.org/viewvc?rev=1926858&view=rev
Log:
On the user-defined-authn branch: In the interaction diagram, correct the
position in the handshake sequence where pipelining is optionally turned
off on a connection. Add the option to reset pipelining on two more
callbacks; current internal schemes sometimes restore it during the 'handle'
phase, and sometimes (notably spnego) during the response validation phase.
This makes the API for user-defined implementations more flexible at the
cost of having to deal with far too long argument lists. Ain't no fun if
it's less than six parameters, right?
* serf.h: Update the authentication interaction diagram.
(serf_authn_handle_func_t): Add the reset_pipelining argument and update
the docstring to hopefully make it clear what that argument means.
(serf_authn_setup_request_func_t): Add the reset_pipelining argument.
(serf_authn_validate_response_func_t): Update the docstring.
* auth/auth_user_defined.c
(struct authn_baton_wrapper::pipelining_was_reset): Rename from the shorter
but more ambiguous pipelining_reset.
(validate_handler): Add docstring.
(maybe_reset_pipelining): New. Now common code extracted from one of
the callbacks.
(serf__authn_user__handle,
serf__authn_user__setup_request): Implement the reset_pipelining argument.
(serf__authn_user__validate_response): Call maybe_reset_pipelining.
* test/test_auth.c
(user_authn_handle,
user_authn_setup_request): Implement the reset_pipelining argument.
Modified:
serf/branches/user-defined-authn/auth/auth_user_defined.c
serf/branches/user-defined-authn/serf.h
serf/branches/user-defined-authn/test/test_auth.c
Modified: serf/branches/user-defined-authn/auth/auth_user_defined.c
URL:
http://svn.apache.org/viewvc/serf/branches/user-defined-authn/auth/auth_user_defined.c?rev=1926858&r1=1926857&r2=1926858&view=diff
==============================================================================
--- serf/branches/user-defined-authn/auth/auth_user_defined.c (original)
+++ serf/branches/user-defined-authn/auth/auth_user_defined.c Sun Jun 29
13:37:25 2025
@@ -53,13 +53,14 @@ struct authn_baton_wrapper {
int pipelining;
/* Was pipelining reset to its previous value?. */
- bool pipelining_reset;
+ bool pipelining_was_reset;
/* The user-defined scheme's per-connection baton. */
void *user_authn_baton;
};
+/* Common callback parameter validation. */
typedef apr_status_t (*callback_fn_t)();
static apr_status_t validate_handler(serf_config_t *config,
const serf__authn_scheme_t *scheme,
@@ -89,6 +90,34 @@ static apr_status_t validate_handler(ser
}
+/* Reset request pipelining on the connection if required. */
+static void maybe_reset_pipelining(const serf__authn_scheme_t *scheme,
+ struct authn_baton_wrapper *authn_baton,
+ serf_connection_t *conn,
+ apr_status_t status,
+ int reset_pipelining)
+{
+ if (!reset_pipelining
+ || status != APR_SUCCESS
+ || authn_baton->pipelining_was_reset
+ || (scheme->user_flags & SERF_AUTHN_FLAG_PIPE))
+ {
+ serf__log(LOGLVL_DEBUG, LOGCOMP_AUTHN, __FILE__, conn->config,
+ "User-defined scheme %s: no pipelining change for %s\n",
+ scheme->name, conn->host_url);
+ return;
+ }
+
+ serf__log(LOGLVL_DEBUG, LOGCOMP_AUTHN, __FILE__, conn->config,
+ "User-defined scheme %s: pipelining reset to %s for %s\n",
+ scheme->name,
+ authn_baton->pipelining ? "on" : "off",
+ conn->host_url);
+ serf__connection_set_pipelining(conn, authn_baton->pipelining);
+ authn_baton->pipelining_was_reset = true;
+}
+
+
apr_status_t
serf__authn_user__init_conn(const serf__authn_scheme_t *scheme,
int code,
@@ -135,9 +164,9 @@ serf__authn_user__init_conn(const serf__
"User-defined scheme %s: pipelining off for %s\n",
scheme->name, conn->host_url);
authn_baton->pipelining = serf__connection_set_pipelining(conn, 0);
- authn_baton->pipelining_reset = false;
+ authn_baton->pipelining_was_reset = false;
} else {
- authn_baton->pipelining_reset = true;
+ authn_baton->pipelining_was_reset = true;
}
return status;
}
@@ -156,6 +185,7 @@ serf__authn_user__handle(const serf__aut
serf__authn_info_t *const authn_info = get_authn_info(code, conn);
serf_context_t *const ctx = conn->ctx;
struct authn_baton_wrapper *authn_baton;
+ int reset_pipelining = 0;
char *username, *password;
apr_pool_t *scratch_pool;
apr_hash_t *auth_param;
@@ -222,7 +252,8 @@ serf__authn_user__handle(const serf__aut
username = password = NULL;
}
- status = scheme->user_handle_func(scheme->user_baton,
+ status = scheme->user_handle_func(&reset_pipelining,
+ scheme->user_baton,
authn_baton->user_authn_baton, code,
auth_hdr, auth_param,
SERF__HEADER_FROM_CODE(code),
@@ -231,6 +262,7 @@ serf__authn_user__handle(const serf__aut
pool, scratch_pool);
cleanup:
+ maybe_reset_pipelining(scheme, authn_baton, conn, status,
reset_pipelining);
apr_pool_destroy(scratch_pool);
return status;
}
@@ -249,6 +281,7 @@ serf__authn_user__setup_request(const se
const int peer_id = SERF__CODE_FROM_PEER(peer);
serf__authn_info_t *const authn_info = get_authn_info(peer_id, conn);
struct authn_baton_wrapper *authn_baton;
+ int reset_pipelining = 0;
apr_pool_t *scratch_pool;
apr_status_t status;
@@ -271,11 +304,14 @@ serf__authn_user__setup_request(const se
authn_baton = authn_info->baton;
apr_pool_create(&scratch_pool, conn->pool);
- status = scheme->user_setup_request_func(scheme->user_baton,
+ status = scheme->user_setup_request_func(&reset_pipelining,
+ scheme->user_baton,
authn_baton->user_authn_baton,
conn, request,
method, uri, hdrs_bkt,
scratch_pool);
+
+ maybe_reset_pipelining(scheme, authn_baton, conn, status,
reset_pipelining);
apr_pool_destroy(scratch_pool);
return status;
}
@@ -334,19 +370,7 @@ serf__authn_user__validate_response(cons
request, response,
scratch_pool);
- /* Reset pipelining if the scheme requires it. */
- if (status == APR_SUCCESS && reset_pipelining
- && !authn_baton->pipelining_reset
- && !(scheme->user_flags & SERF_AUTHN_FLAG_PIPE)) {
- serf__log(LOGLVL_DEBUG, LOGCOMP_AUTHN, __FILE__, conn->config,
- "User-defined scheme %s: pipelining reset to %s for %s\n",
- scheme->name,
- authn_baton->pipelining ? "on" : "off",
- conn->host_url);
- serf__connection_set_pipelining(conn, authn_baton->pipelining);
- authn_baton->pipelining_reset = true;
- }
-
+ maybe_reset_pipelining(scheme, authn_baton, conn, status,
reset_pipelining);
apr_pool_destroy(scratch_pool);
return status;
}
Modified: serf/branches/user-defined-authn/serf.h
URL:
http://svn.apache.org/viewvc/serf/branches/user-defined-authn/serf.h?rev=1926858&r1=1926857&r2=1926858&view=diff
==============================================================================
--- serf/branches/user-defined-authn/serf.h (original)
+++ serf/branches/user-defined-authn/serf.h Sun Jun 29 13:37:25 2025
@@ -962,19 +962,22 @@ serf_bucket_t *serf_request_bucket_reque
* | +<---- authenticate <----+ (401 or 407)
* +<------ init-conn <------+ |
* | | |
+ * | (pipelining off) <--+ (optional) |
* | (get credentials) <--+ (optional) |
* | | |
* +<-------- handle <-------+ |
* | | |
- * | (pipelining off) <--+ (optional) |
+ * | (reset pipelining) <--+ (optional) |
* | | |
* +<---- setup-requiest <---+ |
+ * | | |
+ * | (reset pipelining) <--+ (optional) |
* | +---> request + authn -->+
* | | |
* | +<------ response <------+
* +<-- validate-response <--+ |
* | | |
- * | (pipelining on) <--+ (optional) |
+ * | (reset pipelining) <--+ (optional) |
* | | |
* ```
*/
@@ -1077,12 +1080,22 @@ typedef apr_status_t
* @a request is the pending request and @a response is the response that
* caused this callback to be called.
*
+ * If the scheme flag @c SERF_AUTHN_FLAG_PIPE is *not* set, return a boolean
+ * value in @a reset_pipelining to indicate whether pipelining on @a conn
+ * should be restored to the value before the init-conn callback was invoked.
+ * The value of this flag is set to @c false by the caller, so the
+ * implementation does not have to modify it if the pipelining state should
+ * remain unchanged. This parameter has the same meaning in the
+ * serf_authn_setup_request_func_t and serf_authn_validate_response_func_t
+ * callbacks and will only take effect the first time its value @c true.
+ *
* Use @a scratch_pool for temporary allocations.
*
* @since New in 1.4.
*/
typedef apr_status_t
-(*serf_authn_handle_func_t)(void *baton,
+(*serf_authn_handle_func_t)(int *reset_pipelining,
+ void *baton,
void *authn_baton,
int code,
const char *authn_header,
@@ -1106,12 +1119,15 @@ typedef apr_status_t
* @a method and @a uri are the requests attributes and @a headers are the
* request headers where the credentials are usually set.
*
+ * For the meaning of @a reset_pipelining, see serf_authn_handle_func_t.
+ *
* Use @a scratch_pool for temporary allocations.
*
* @since New in 1.4.
*/
typedef apr_status_t
-(*serf_authn_setup_request_func_t)(void *baton,
+(*serf_authn_setup_request_func_t)(int *reset_pipelining,
+ void *baton,
void *authn_baton,
serf_connection_t *conn,
serf_request_t *request,
@@ -1134,9 +1150,7 @@ typedef apr_status_t
* response header. This argument will be NULL @a response does not contain
* one of those headers.
*
- * If the scheme flag @c SERF_AUTHN_FLAG_PIPE is *not* set, return a boolean
- * value in @a reset_pipelining to indicate whether pipelining on @a conn
should
- * be restored to the value before the init-conn callback was invoked.
+ * For the meaning of @a reset_pipelining, see serf_authn_handle_func_t.
*
* Use @a scratch_pool for temporary allocations.
*
Modified: serf/branches/user-defined-authn/test/test_auth.c
URL:
http://svn.apache.org/viewvc/serf/branches/user-defined-authn/test/test_auth.c?rev=1926858&r1=1926857&r2=1926858&view=diff
==============================================================================
--- serf/branches/user-defined-authn/test/test_auth.c (original)
+++ serf/branches/user-defined-authn/test/test_auth.c Sun Jun 29 13:37:25 2025
@@ -733,7 +733,8 @@ static apr_status_t user_authn_get_realm
return APR_SUCCESS;
}
-static apr_status_t user_authn_handle(void *baton,
+static apr_status_t user_authn_handle(int *reset_pipelining,
+ void *baton,
void *authn_baton,
int code,
const char *authn_header,
@@ -753,10 +754,12 @@ static apr_status_t user_authn_handle(vo
ab->header = apr_pstrdup(result_pool, response_header);
ab->value = apr_pstrcat(result_pool, b->name, " ", password, NULL);
+ *reset_pipelining = 1;
return APR_SUCCESS;
}
-static apr_status_t user_authn_setup_request(void *baton,
+static apr_status_t user_authn_setup_request(int *reset_pipelining,
+ void *baton,
void *authn_baton,
serf_connection_t *conn,
serf_request_t *request,
@@ -777,6 +780,8 @@ static apr_status_t user_authn_setup_req
ab->header, ab->value);
serf_bucket_headers_setn(headers, ab->header, ab->value);
+
+ *reset_pipelining = 1;
return APR_SUCCESS;
}
