Skip to content

Commit 679b496

Browse files
zandbeltzandbelt
committed
fix check OIDC_CONFIG_POS_TIMEOUT_UNSET for memcache TTL getter; #1345
add tests for memcache TTL Signed-off-by: Hans Zandbelt <[email protected]>
1 parent eff5b6d commit 679b496

File tree

10 files changed

+142
-36
lines changed

10 files changed

+142
-36
lines changed

‎ChangeLog‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
08/20/2025
2+
- fix check OIDC_CONFIG_POS_TIMEOUT_UNSET for memcache TTL getter; #1345
3+
add tests for memcache TTL
4+
15
08/18/2025
26
- add check and code coverage skeleton
37
- bump to 2.4.18dev

‎src/cache/memcache.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,8 +178,8 @@ static int oidc_cache_memcache_post_config(server_rec *s) {
178178
if (port == 0)
179179
port = 11211;
180180

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

184184
/* create the memcache server struct */
185185
rv = apr_memcache_server_create(p, host_str, port, min, smax, hmax, (apr_uint32_t)ttl, &st);

‎src/cfg/cache.c‎

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,9 @@ const char *oidc_cmd_cache_type_set(cmd_parms *cmd, void *ptr, const char *arg)
8080
return OIDC_CONFIG_DIR_RV(cmd, rv);
8181
}
8282

83-
#define OIDC_CFG_MEMBER_FUNC_CACHE_TYPE_GET(member, type, def_val) \
83+
#define OIDC_CFG_MEMBER_FUNC_CACHE_TYPE_GET(member, type, def_val, unset_val) \
8484
type oidc_cfg_cache_##member##_get(oidc_cfg_t *cfg) { \
85-
if (cfg->cache.member == OIDC_CONFIG_POS_INT_UNSET) \
85+
if (cfg->cache.member == unset_val) \
8686
return def_val; \
8787
return cfg->cache.member; \
8888
}
@@ -97,27 +97,29 @@ 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_PARSE(member, type, parse, def_val) \
100+
#define OIDC_CFG_MEMBER_FUNCS_CACHE_PARSE(member, type, parse, def_val, unset_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-
type 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, type, def_val)
111+
OIDC_CFG_MEMBER_FUNC_CACHE_TYPE_GET(member, type, def_val, unset_val)
112112

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)
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, OIDC_CONFIG_POS_INT_UNSET)
115115

116116
#define OIDC_CFG_MEMBER_FUNCS_CACHE_INT(member, min, max, def_val) \
117117
OIDC_CFG_MEMBER_FUNCS_CACHE_INT_EXT(member, oidc_cfg_parse_int_min_max(cmd->pool, arg, &v, min, max), def_val)
118118

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)
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, \
121+
oidc_cfg_parse_timeout_min_max(cmd->pool, arg, &v, min, max), def_val, \
122+
OIDC_CONFIG_POS_TIMEOUT_UNSET)
121123

122124
#define OIDC_CFG_MEMBER_FUNCS_CACHE_BOOL(member, def_val) \
123125
OIDC_CFG_MEMBER_FUNCS_CACHE_INT_EXT(member, oidc_cfg_parse_boolean(cmd->pool, arg, &v), def_val)
@@ -165,7 +167,8 @@ const char *oidc_cmd_cache_shm_entry_size_max_set(cmd_parms *cmd, void *ptr, con
165167
return OIDC_CONFIG_DIR_RV(cmd, rv);
166168
}
167169

168-
OIDC_CFG_MEMBER_FUNC_CACHE_TYPE_GET(shm_entry_size_max, int, OIDC_DEFAULT_CACHE_SHM_ENTRY_SIZE_MAX)
170+
OIDC_CFG_MEMBER_FUNC_CACHE_TYPE_GET(shm_entry_size_max, int, OIDC_DEFAULT_CACHE_SHM_ENTRY_SIZE_MAX,
171+
OIDC_CONFIG_POS_INT_UNSET)
169172

170173
static void oidc_cfg_cache_shm_create_server_config(oidc_cfg_t *c) {
171174
c->cache.shm_size_max = OIDC_DEFAULT_CACHE_SHM_SIZE;
@@ -252,7 +255,8 @@ OIDC_CFG_MEMBER_FUNCS_CACHE_INT(memcache_hmax, OIDC_CACHE_MEMCACHE_CONNECTIONS_H
252255
#define OIDC_DEFAULT_CACHE_MEMCACHE_CONNECTIONS_TTL apr_time_from_sec(60)
253256

254257
OIDC_CFG_MEMBER_FUNCS_CACHE_TIMEOUT(memcache_ttl, OIDC_CACHE_MEMCACHE_CONNECTIONS_TTL_MIN,
255-
OIDC_CACHE_MEMCACHE_CONNECTIONS_TTL_MAX, OIDC_DEFAULT_CACHE_MEMCACHE_CONNECTIONS_TTL)
258+
OIDC_CACHE_MEMCACHE_CONNECTIONS_TTL_MAX,
259+
OIDC_DEFAULT_CACHE_MEMCACHE_CONNECTIONS_TTL)
256260

257261
static void oidc_cfg_cache_memcache_create_server_config(oidc_cfg_t *c) {
258262
c->cache.memcache_servers = NULL;
@@ -271,8 +275,8 @@ static void oidc_cfg_cache_memcache_merge_server_config(oidc_cfg_t *c, oidc_cfg_
271275
: base->cache.memcache_smax;
272276
c->cache.memcache_hmax = add->cache.memcache_hmax != OIDC_CONFIG_POS_INT_UNSET ? add->cache.memcache_hmax
273277
: base->cache.memcache_hmax;
274-
c->cache.memcache_ttl =
275-
add->cache.memcache_ttl != OIDC_CONFIG_POS_TIMEOUT_UNSET ? add->cache.memcache_ttl : base->cache.memcache_ttl;
278+
c->cache.memcache_ttl = add->cache.memcache_ttl != OIDC_CONFIG_POS_TIMEOUT_UNSET ? add->cache.memcache_ttl
279+
: base->cache.memcache_ttl;
276280
}
277281

278282
#endif
@@ -313,8 +317,8 @@ const char *oidc_cmd_cache_redis_connect_timeout_set(cmd_parms *cmd, void *struc
313317
return OIDC_CONFIG_DIR_RV(cmd, rv);
314318
}
315319

316-
OIDC_CFG_MEMBER_FUNC_CACHE_TYPE_GET(redis_connect_timeout, int, OIDC_CONFIG_POS_INT_UNSET)
317-
OIDC_CFG_MEMBER_FUNC_CACHE_TYPE_GET(redis_keepalive, int, OIDC_CONFIG_POS_INT_UNSET)
320+
OIDC_CFG_MEMBER_FUNC_CACHE_TYPE_GET(redis_connect_timeout, int, OIDC_CONFIG_POS_INT_UNSET, OIDC_CONFIG_POS_INT_UNSET)
321+
OIDC_CFG_MEMBER_FUNC_CACHE_TYPE_GET(redis_keepalive, int, OIDC_CONFIG_POS_INT_UNSET, OIDC_CONFIG_POS_INT_UNSET)
318322

319323
#define OIDC_REDIS_TIMEOUT_MIN 1
320324
#define OIDC_REDIS_TIMEOUT_MAX 3600

‎src/cfg/cfg.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@
5353

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

5858
#define OIDCPublicKeyFiles "OIDCPublicKeyFiles"
5959
#define OIDCDefaultLoggedOutURL "OIDCDefaultLoggedOutURL"

‎src/cfg/parse.c‎

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -201,22 +201,26 @@ const char *oidc_cfg_parse_int_min_max(apr_pool_t *pool, const char *arg, int *i
201201
* apr_interval_time_t if it is in a valid min/max range
202202
*/
203203
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;
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,
212+
"timeout value %" APR_TIME_T_FMT
213+
" is smaller than the minimum allowed value %" APR_TIME_T_FMT,
214+
timeout, min_value);
215+
}
216+
if (timeout > max_value) {
217+
return apr_psprintf(pool,
218+
"timeout value %" APR_TIME_T_FMT
219+
" is greater than the maximum allowed value %" APR_TIME_T_FMT,
220+
timeout, max_value);
221+
}
222+
*timeout_value = (int)timeout;
223+
return NULL;
220224
}
221225

222226
/*

‎src/cfg/parse.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ const char *oidc_cfg_parse_boolean(apr_pool_t *pool, const char *arg, int *bool_
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);
7272
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);
73+
apr_interval_time_t min_value, apr_interval_time_t max_value);
7474
const char *oidc_cfg_parse_dirname(apr_pool_t *pool, const char *arg, char **value);
7575
const char *oidc_cfg_parse_filename(apr_pool_t *pool, const char *arg, char **value);
7676
const char *oidc_cfg_parse_relative_or_absolute_url(apr_pool_t *pool, const char *arg, char **value);

‎test/helper.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,10 @@ request_rec *oidc_test_request_get() {
141141
return request;
142142
}
143143

144+
oidc_cfg_t *oidc_test_cfg_get() {
145+
return (oidc_cfg_t *)ap_get_module_config(request->server->module_config, &auth_openidc_module);
146+
}
147+
144148
cmd_parms *oidc_test_cmd_get(const char *primitive) {
145149
request_rec *r = oidc_test_request_get();
146150
cmd_parms *cmd = apr_pcalloc(r->pool, sizeof(cmd_parms));

‎test/helper.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,14 @@
5050
#include <stdbool.h>
5151
#include <stdlib.h>
5252

53+
#include "cfg/cfg.h"
54+
5355
void oidc_test_setup(void);
5456
void oidc_test_teardown(void);
5557
int oidc_test_suite_run(Suite *s);
5658
apr_pool_t *oidc_test_pool_get();
5759
request_rec *oidc_test_request_get();
60+
oidc_cfg_t *oidc_test_cfg_get();
5861
cmd_parms *oidc_test_cmd_get(const char *primitive);
5962

6063
#endif // _MOD_AUTH_OPENIDC_TEST_COMMON_H_

‎test/stub.c‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,46 @@ AP_DECLARE(apr_status_t) ap_mpm_query(int query_code, int *result) {
311311
#if AP_MODULE_MAGIC_AT_LEAST(20080920, 2)
312312
AP_DECLARE(apr_status_t)
313313
ap_timeout_parameter_parse(const char *timeout_parameter, apr_interval_time_t *timeout, const char *default_time_unit) {
314-
*timeout = 0;
314+
char *endp;
315+
const char *time_str;
316+
apr_int64_t tout;
317+
318+
tout = apr_strtoi64(timeout_parameter, &endp, 10);
319+
if (errno) {
320+
return errno;
321+
}
322+
if (!endp || !*endp) {
323+
time_str = default_time_unit;
324+
} else {
325+
time_str = endp;
326+
}
327+
328+
switch (*time_str) {
329+
/* Time is in seconds */
330+
case 's':
331+
*timeout = (apr_interval_time_t)apr_time_from_sec(tout);
332+
break;
333+
case 'h':
334+
/* Time is in hours */
335+
*timeout = (apr_interval_time_t)apr_time_from_sec(tout * 3600);
336+
break;
337+
case 'm':
338+
switch (*(++time_str)) {
339+
/* Time is in milliseconds */
340+
case 's':
341+
*timeout = (apr_interval_time_t)tout * 1000;
342+
break;
343+
/* Time is in minutes */
344+
case 'i':
345+
*timeout = (apr_interval_time_t)apr_time_from_sec(tout * 60);
346+
break;
347+
default:
348+
return APR_EGENERAL;
349+
}
350+
break;
351+
default:
352+
return APR_EGENERAL;
353+
}
315354
return APR_SUCCESS;
316355
}
317356
#endif

‎test/test_cfg.c‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@
4141
*
4242
**************************************************************************/
4343

44+
#include "cfg/cache.h"
45+
#include "cfg/cfg_int.h"
4446
#include "cfg/provider.h"
4547
#include "helper.h"
4648

@@ -70,11 +72,57 @@ START_TEST(test_cmd_provider_token_endpoint_auth_set) {
7072
}
7173
END_TEST
7274

75+
START_TEST(test_cfg_cache_connections_ttl) {
76+
const char *rv = NULL;
77+
void *ptr = NULL;
78+
const char *arg = NULL;
79+
apr_interval_time_t ttl = 0;
80+
oidc_cfg_t *cfg = NULL;
81+
82+
cfg = oidc_test_cfg_get();
83+
ck_assert_msg(cfg->cache.memcache_ttl == OIDC_CONFIG_POS_TIMEOUT_UNSET,
84+
"default not set to OIDC_CONFIG_POS_TIMEOUT_UNSET: %d", (int)cfg->cache.memcache_ttl);
85+
86+
cmd_parms *cmd = oidc_test_cmd_get(OIDCMemCacheConnectionsTTL);
87+
88+
ttl = oidc_cfg_cache_memcache_ttl_get(cfg);
89+
ck_assert_msg(ttl == apr_time_from_sec(60), "default not set to 60s: %d", (int)ttl);
90+
91+
arg = "bogus";
92+
rv = oidc_cmd_cache_memcache_ttl_set(cmd, ptr, arg);
93+
ck_assert_msg(rv != NULL, "set to \"bogus\" did not fail");
94+
95+
arg = "-2";
96+
rv = oidc_cmd_cache_memcache_ttl_set(cmd, ptr, arg);
97+
ck_assert_msg(rv != NULL, "set to \"-2\" did not fail");
98+
99+
arg = "120";
100+
rv = oidc_cmd_cache_memcache_ttl_set(cmd, ptr, arg);
101+
ck_assert_msg(rv == NULL, "set to 120 failed");
102+
103+
arg = "4294";
104+
rv = oidc_cmd_cache_memcache_ttl_set(cmd, ptr, arg);
105+
ck_assert_msg(rv == NULL, "set to 4294 failed");
106+
107+
arg = "4295";
108+
rv = oidc_cmd_cache_memcache_ttl_set(cmd, ptr, arg);
109+
ck_assert_msg(rv != NULL, "set to 4295 did not fail");
110+
111+
arg = "180s";
112+
rv = oidc_cmd_cache_memcache_ttl_set(cmd, ptr, arg);
113+
ck_assert_msg(rv == NULL, "set to 180s failed");
114+
115+
ttl = oidc_cfg_cache_memcache_ttl_get(cfg);
116+
ck_assert_msg(ttl == apr_time_from_sec(180), "get 180s failed: %d", (int)ttl);
117+
}
118+
END_TEST
119+
73120
int main(void) {
74121
TCase *core = tcase_create("core");
75122
tcase_add_checked_fixture(core, oidc_test_setup, oidc_test_teardown);
76123

77124
tcase_add_test(core, test_cmd_provider_token_endpoint_auth_set);
125+
tcase_add_test(core, test_cfg_cache_connections_ttl);
78126

79127
Suite *s = suite_create("cfg");
80128
suite_add_tcase(s, core);

0 commit comments

Comments
 (0)