Skip to content

Commit 1ce2e10

Browse files
rpluem-vfrpluem-vf
committed
Correctly parse the value set via OIDCMemCacheConnectionsTTL
Currently the value set via OIDCMemCacheConnectionsTTL is interpreted as microseconds and not as seconds. Correctly interpret this value as seconds via ap_timeout_parameter_parse and create the needed infrastructure to parse similar timeout fields in the future. * src/cache/memcache.c: - Change type of ttl to apr_interval_time_t. - The default value is now returned via oidc_cfg_cache_memcache_ttl_get. - Adjust format strings for apr_interval_time_t printing. * src/cfg/cache.c: - Generalize OIDC_CFG_MEMBER_FUNCS_CACHE_INT_EXT macro to OIDC_CFG_MEMBER_FUNCS_CACHE_PARSE which allows to specify the parsed type. - Add OIDC_CFG_MEMBER_FUNCS_CACHE_TIMEOUT macro which parses apr_interval_time_t values via oidc_cfg_parse_timeout_min_max. - Shorten maximum TTL via OIDC_CACHE_MEMCACHE_CONNECTIONS_TTL_MAX to 4292 seconds. - Change default via OIDC_DEFAULT_CACHE_MEMCACHE_CONNECTIONS_TTL from 0 to 60 seconds. - Use OIDC_CFG_MEMBER_FUNCS_CACHE_TIMEOUT instead of OIDC_CFG_MEMBER_FUNCS_CACHE_INT macro for memcache_ttl field. * src/cfg/cache.h: - Change type of memcache_ttl from int to apr_interval_time_t. * src/cfg/cfg.h: - Define OIDC_CONFIG_POS_TIMEOUT_UNSET to -2 (-1 might be used for unlimited timeout). * src/cfg/cfg_int.h: - Change type of memcache_ttl from int to apr_interval_time_t. * src/cfg/parse.c: - Add oidc_cfg_parse_timeout_min_max to parse a timeout string via ap_timeout_parameter_parse into an apr_interval_time_t if it is in a valid min/max range. * src/cfg/parse.h: - Add prototype for oidc_cfg_parse_timeout_min_max Signed-off-by: Ruediger Pluem <[email protected]>
1 parent aa3c2b7 commit 1ce2e10

File tree

7 files changed

+53
-17
lines changed

7 files changed

+53
-17
lines changed

‎src/cache/memcache.c‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ static int oidc_cache_memcache_post_config(server_rec *s) {
8989
apr_uint32_t min = 0;
9090
apr_uint32_t smax = 0;
9191
apr_uint32_t hmax = 0;
92-
apr_uint32_t ttl = 0;
92+
apr_interval_time_t ttl = 0;
9393

9494
if (oidc_cfg_cache_memcache_servers_get(cfg) == NULL) {
9595
oidc_serror(s, "cache type is set to \"memcache\", but no valid " OIDCMemCacheServers
@@ -148,9 +148,6 @@ static int oidc_cache_memcache_post_config(server_rec *s) {
148148
smax = 1;
149149
}
150150
}
151-
if (ttl == 0) {
152-
ttl = apr_time_from_sec(60);
153-
}
154151
if (smax > hmax) {
155152
smax = hmax;
156153
}
@@ -181,11 +178,11 @@ static int oidc_cache_memcache_post_config(server_rec *s) {
181178
if (port == 0)
182179
port = 11211;
183180

184-
oidc_sdebug(s, "creating server: %s:%d, min=%d, smax=%d, hmax=%d, ttl=%d", host_str, port, min, smax,
181+
oidc_sdebug(s, "creating server: %s:%d, min=%d, smax=%d, hmax=%d, ttl=%" APR_TIME_T_FMT, host_str, port, min, smax,
185182
hmax, ttl);
186183

187184
/* create the memcache server struct */
188-
rv = apr_memcache_server_create(p, host_str, port, min, smax, hmax, ttl, &st);
185+
rv = apr_memcache_server_create(p, host_str, port, min, smax, hmax, (apr_uint32_t)ttl, &st);
189186
if (rv != APR_SUCCESS) {
190187
oidc_serror(s, "failed to create cache server: %s:%d", host_str, port);
191188
return HTTP_INTERNAL_SERVER_ERROR;

‎src/cfg/cache.c‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -97,22 +97,28 @@ const char *oidc_cmd_cache_type_set(cmd_parms *cmd, void *ptr, const char *arg)
9797
return OIDC_CONFIG_DIR_RV(cmd, rv); \
9898
}
9999

100-
#define OIDC_CFG_MEMBER_FUNCS_CACHE_INT_EXT(member, parse, def_val) \
100+
#define OIDC_CFG_MEMBER_FUNCS_CACHE_PARSE(member, type, parse, def_val) \
101101
const char *oidc_cmd_cache_##member##_set(cmd_parms *cmd, void *ptr, const char *arg) { \
102102
oidc_cfg_t *cfg = \
103103
(oidc_cfg_t *)ap_get_module_config(cmd->server->module_config, &auth_openidc_module); \
104-
int v = -1; \
104+
type v = -1; \
105105
const char *rv = parse; \
106106
if (rv == NULL) \
107107
cfg->cache.member = v; \
108108
return OIDC_CONFIG_DIR_RV(cmd, rv); \
109109
} \
110110
\
111-
OIDC_CFG_MEMBER_FUNC_CACHE_TYPE_GET(member, int, def_val)
111+
OIDC_CFG_MEMBER_FUNC_CACHE_TYPE_GET(member, type, def_val)
112+
113+
#define OIDC_CFG_MEMBER_FUNCS_CACHE_INT_EXT(member, parse, def_val) \
114+
OIDC_CFG_MEMBER_FUNCS_CACHE_PARSE(member, int, parse, def_val)
112115

113116
#define OIDC_CFG_MEMBER_FUNCS_CACHE_INT(member, min, max, def_val) \
114117
OIDC_CFG_MEMBER_FUNCS_CACHE_INT_EXT(member, oidc_cfg_parse_int_min_max(cmd->pool, arg, &v, min, max), def_val)
115118

119+
#define OIDC_CFG_MEMBER_FUNCS_CACHE_TIMEOUT(member, min, max, def_val) \
120+
OIDC_CFG_MEMBER_FUNCS_CACHE_PARSE(member, apr_interval_time_t, oidc_cfg_parse_timeout_min_max(cmd->pool, arg, &v, min, max), def_val)
121+
116122
#define OIDC_CFG_MEMBER_FUNCS_CACHE_BOOL(member, def_val) \
117123
OIDC_CFG_MEMBER_FUNCS_CACHE_INT_EXT(member, oidc_cfg_parse_boolean(cmd->pool, arg, &v), def_val)
118124

@@ -235,19 +241,25 @@ OIDC_CFG_MEMBER_FUNCS_CACHE_INT(memcache_smax, OIDC_CACHE_MEMCACHE_CONNECTIONS_S
235241
OIDC_CFG_MEMBER_FUNCS_CACHE_INT(memcache_hmax, OIDC_CACHE_MEMCACHE_CONNECTIONS_HMAX_MIN,
236242
OIDC_CACHE_MEMCACHE_CONNECTIONS_HMAX_MAX, OIDC_DEFAULT_CACHE_MEMCACHE_CONNECTIONS_HMAX)
237243

238-
#define OIDC_CACHE_MEMCACHE_CONNECTIONS_TTL_MIN 0
239-
#define OIDC_CACHE_MEMCACHE_CONNECTIONS_TTL_MAX 3600 * 24 * 7
240-
#define OIDC_DEFAULT_CACHE_MEMCACHE_CONNECTIONS_TTL 0
244+
#define OIDC_CACHE_MEMCACHE_CONNECTIONS_TTL_MIN (apr_interval_time_t)0
245+
/*
246+
* Due to a design error in the apr-util 1.x apr_memcache_server_create prototype
247+
* (it uses an apr_uint32_t instead of an apr_interval_time_t) we need to limit
248+
* the maximum value to 4292 seconds which is the maximum value in microseconds
249+
* that can be represented by an apr_uint32_t.
250+
*/
251+
#define OIDC_CACHE_MEMCACHE_CONNECTIONS_TTL_MAX apr_time_from_sec(4294)
252+
#define OIDC_DEFAULT_CACHE_MEMCACHE_CONNECTIONS_TTL apr_time_from_sec(60)
241253

242-
OIDC_CFG_MEMBER_FUNCS_CACHE_INT(memcache_ttl, OIDC_CACHE_MEMCACHE_CONNECTIONS_TTL_MIN,
254+
OIDC_CFG_MEMBER_FUNCS_CACHE_TIMEOUT(memcache_ttl, OIDC_CACHE_MEMCACHE_CONNECTIONS_TTL_MIN,
243255
OIDC_CACHE_MEMCACHE_CONNECTIONS_TTL_MAX, OIDC_DEFAULT_CACHE_MEMCACHE_CONNECTIONS_TTL)
244256

245257
static void oidc_cfg_cache_memcache_create_server_config(oidc_cfg_t *c) {
246258
c->cache.memcache_servers = NULL;
247259
c->cache.memcache_min = OIDC_CONFIG_POS_INT_UNSET;
248260
c->cache.memcache_smax = OIDC_CONFIG_POS_INT_UNSET;
249261
c->cache.memcache_hmax = OIDC_CONFIG_POS_INT_UNSET;
250-
c->cache.memcache_ttl = OIDC_CONFIG_POS_INT_UNSET;
262+
c->cache.memcache_ttl = OIDC_CONFIG_POS_TIMEOUT_UNSET;
251263
}
252264

253265
static void oidc_cfg_cache_memcache_merge_server_config(oidc_cfg_t *c, oidc_cfg_t *base, oidc_cfg_t *add) {
@@ -260,7 +272,7 @@ static void oidc_cfg_cache_memcache_merge_server_config(oidc_cfg_t *c, oidc_cfg_
260272
c->cache.memcache_hmax = add->cache.memcache_hmax != OIDC_CONFIG_POS_INT_UNSET ? add->cache.memcache_hmax
261273
: base->cache.memcache_hmax;
262274
c->cache.memcache_ttl =
263-
add->cache.memcache_ttl != OIDC_CONFIG_POS_INT_UNSET ? add->cache.memcache_ttl : base->cache.memcache_ttl;
275+
add->cache.memcache_ttl != OIDC_CONFIG_POS_TIMEOUT_UNSET ? add->cache.memcache_ttl : base->cache.memcache_ttl;
264276
}
265277

266278
#endif

‎src/cfg/cache.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ OIDC_CFG_MEMBER_FUNCS_DECL(cache_memcache_servers, const char *)
9292
OIDC_CFG_MEMBER_FUNCS_DECL(cache_memcache_min, int)
9393
OIDC_CFG_MEMBER_FUNCS_DECL(cache_memcache_smax, int)
9494
OIDC_CFG_MEMBER_FUNCS_DECL(cache_memcache_hmax, int)
95-
OIDC_CFG_MEMBER_FUNCS_DECL(cache_memcache_ttl, int)
95+
OIDC_CFG_MEMBER_FUNCS_DECL(cache_memcache_ttl, apr_interval_time_t)
9696

9797
#endif // USE_MEMCACHE
9898

‎src/cfg/cfg.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@
5252
#include "cache/cache.h"
5353

5454
#define OIDC_CONFIG_POS_INT_UNSET -1
55+
/* -1 might be used for unlimited timeout */
56+
#define OIDC_CONFIG_POS_TIMEOUT_UNSET (apr_interval_time_t)-2
5557

5658
#define OIDCPublicKeyFiles "OIDCPublicKeyFiles"
5759
#define OIDCDefaultLoggedOutURL "OIDCDefaultLoggedOutURL"

‎src/cfg/cfg_int.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ struct oidc_cfg_cache_t {
9494
int memcache_hmax;
9595
/* cache_type= memcache: maximum time in microseconds a connection to a memcache server can be idle before being
9696
* closed */
97-
int memcache_ttl;
97+
apr_interval_time_t memcache_ttl;
9898
#endif
9999

100100
/*

‎src/cfg/parse.c‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,29 @@ const char *oidc_cfg_parse_int_min_max(apr_pool_t *pool, const char *arg, int *i
196196
return NULL;
197197
}
198198

199+
/*
200+
* parse a timeout string via ap_timeout_parameter_parse into an
201+
* apr_interval_time_t if it is in a valid min/max range
202+
*/
203+
const char *oidc_cfg_parse_timeout_min_max(apr_pool_t *pool, const char *arg, apr_interval_time_t *timeout_value,
204+
apr_interval_time_t min_value, apr_interval_time_t max_value) {
205+
apr_interval_time_t timeout;
206+
207+
if (ap_timeout_parameter_parse(arg, &timeout, "s") != APR_SUCCESS) {
208+
return apr_psprintf(pool, "not a valid timeout parameter: %s", arg);
209+
}
210+
if (timeout < min_value) {
211+
return apr_psprintf(pool, "timeout value %" APR_TIME_T_FMT " is smaller than the minimum allowed value %" APR_TIME_T_FMT, timeout,
212+
min_value);
213+
}
214+
if (timeout > max_value) {
215+
return apr_psprintf(pool, "timeout value %" APR_TIME_T_FMT " is greater than the maximum allowed value %" APR_TIME_T_FMT, timeout,
216+
max_value);
217+
}
218+
*timeout_value = (int)timeout;
219+
return NULL;
220+
}
221+
199222
/*
200223
* check if a string is a valid URL starting with either scheme1 or scheme2 (if not NULL)
201224
*/

‎src/cfg/parse.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ const char *oidc_cfg_parse_is_valid_encrypted_response_enc(apr_pool_t *pool, con
6969
const char *oidc_cfg_parse_boolean(apr_pool_t *pool, const char *arg, int *bool_value);
7070
const char *oidc_cfg_parse_int(apr_pool_t *pool, const char *arg, int *int_value);
7171
const char *oidc_cfg_parse_int_min_max(apr_pool_t *pool, const char *arg, int *int_value, int min_value, int max_value);
72+
const char *oidc_cfg_parse_timeout_min_max(apr_pool_t *pool, const char *arg, apr_interval_time_t *timeout_value,
73+
apr_interval_time_t min_value, apr_interval_time_t max_value);
7274
const char *oidc_cfg_parse_dirname(apr_pool_t *pool, const char *arg, char **value);
7375
const char *oidc_cfg_parse_filename(apr_pool_t *pool, const char *arg, char **value);
7476
const char *oidc_cfg_parse_relative_or_absolute_url(apr_pool_t *pool, const char *arg, char **value);

0 commit comments

Comments
 (0)