Skip to content

Commit

Permalink
use compact encoding and preserve order for most calls to json_dumps
Browse files Browse the repository at this point in the history
correct usage of free() for json_dumps return values instead of
cjose_get_dealloc()()

Signed-off-by: Hans Zandbelt <hans.zandbelt@openidc.com>
  • Loading branch information
zandbelt committed Sep 27, 2024
1 parent 0da9545 commit 5f3abf6
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 23 deletions.
4 changes: 4 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
09/27/2024
- correct usage of free() for json_dumps return values instead of cjose_get_dealloc()()
- use compact encoding and preserve order where appropriate for most calls to json_dumps

09/26/2024
- fix oidc_jwk_copy wrt. "x5t", which broke private_key_jwt authentication to Azure AD since 2.4.13
see #1269; thanks @uoe-pjackson
Expand Down
2 changes: 1 addition & 1 deletion src/http.c
Original file line number Diff line number Diff line change
Expand Up @@ -926,7 +926,7 @@ apr_byte_t oidc_http_post_json(request_rec *r, const char *url, json_t *json, co
long *response_code, apr_hash_t *response_hdrs, oidc_http_timeout_t *http_timeout,
const oidc_http_outgoing_proxy_t *outgoing_proxy, const apr_array_header_t *pass_cookies,
const char *ssl_cert, const char *ssl_key, const char *ssl_key_pwd) {
char *data = json != NULL ? oidc_util_encode_json_object(r, json, JSON_COMPACT) : NULL;
char *data = json != NULL ? oidc_util_encode_json_object(r, json, JSON_PRESERVE_ORDER | JSON_COMPACT) : NULL;
return oidc_http_request(r, url, data, OIDC_HTTP_CONTENT_TYPE_JSON, basic_auth, access_token, dpop,
ssl_validate_server, response, response_code, response_hdrs, http_timeout,
outgoing_proxy, pass_cookies, ssl_cert, ssl_key, ssl_key_pwd);
Expand Down
12 changes: 6 additions & 6 deletions src/jose.c
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ char *oidc_jwt_serialize(apr_pool_t *pool, oidc_jwt_t *jwt, oidc_jose_error_t *e
cser = apr_pstrmemdup(pool, out, out_len);
cjose_get_dealloc()(out);

cjose_get_dealloc()(s_payload);
free(s_payload);

cser = apr_psprintf(pool, "%s.%s.", OIDC_JOSE_HDR_ALG_NONE, cser);
}
Expand Down Expand Up @@ -340,7 +340,7 @@ oidc_jwk_t *oidc_jwk_parse(apr_pool_t *pool, json_t *json, oidc_jose_error_t *er
char *use = NULL;
json_t *v = NULL;

char *s_json = json_dumps(json, 0);
char *s_json = json_dumps(json, JSON_PRESERVE_ORDER | JSON_COMPACT);
if (s_json == NULL) {
oidc_jose_error(err, "could not serialize JWK");
goto end;
Expand Down Expand Up @@ -523,7 +523,7 @@ apr_byte_t oidc_jwk_to_json(apr_pool_t *pool, const oidc_jwk_t *jwk, char **s_js
if (s == NULL)
return FALSE;
*s_json = apr_pstrdup(pool, s);
cjose_get_dealloc()(s);
free(s);
return TRUE;
}

Expand Down Expand Up @@ -1059,7 +1059,7 @@ apr_byte_t oidc_jwt_parse(apr_pool_t *pool, const char *input_json, oidc_jwt_t *
jwt->header.value.json = json_deep_copy((json_t *)hdr);
char *str = json_dumps(jwt->header.value.json, JSON_PRESERVE_ORDER | JSON_COMPACT);
jwt->header.value.str = apr_pstrdup(pool, str);
cjose_get_dealloc()(str);
free(str);

jwt->header.alg = apr_pstrdup(pool, cjose_header_get(hdr, CJOSE_HDR_ALG, &cjose_err));
jwt->header.enc = apr_pstrdup(pool, cjose_header_get(hdr, CJOSE_HDR_ENC, &cjose_err));
Expand Down Expand Up @@ -1146,7 +1146,7 @@ apr_byte_t oidc_jwt_sign(apr_pool_t *pool, oidc_jwt_t *jwt, oidc_jwk_t *jwk, apr
if (compress == TRUE) {
if (oidc_jose_compress(pool, (char *)plaintext, _oidc_strlen(plaintext), &s_payload, &payload_len,
err) == FALSE) {
cjose_get_dealloc()(plaintext);
free(plaintext);
return FALSE;
}
} else {
Expand All @@ -1156,7 +1156,7 @@ apr_byte_t oidc_jwt_sign(apr_pool_t *pool, oidc_jwt_t *jwt, oidc_jwk_t *jwk, apr
}

jwt->cjose_jws = cjose_jws_sign(jwk->cjose_jwk, hdr, (const uint8_t *)s_payload, payload_len, &cjose_err);
cjose_get_dealloc()(plaintext);
free(plaintext);

if (jwt->cjose_jws == NULL) {
oidc_jose_error(err, "cjose_jws_sign failed: %s", oidc_cjose_e2s(pool, cjose_err));
Expand Down
2 changes: 1 addition & 1 deletion src/oauth.c
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ static apr_byte_t oidc_oauth_resolve_access_token(request_rec *r, oidc_cfg_t *c,
}

/* stringify the response */
*response = oidc_util_encode_json_object(r, *token, JSON_COMPACT);
*response = oidc_util_encode_json_object(r, *token, JSON_PRESERVE_ORDER | JSON_COMPACT);

return TRUE;
}
Expand Down
5 changes: 3 additions & 2 deletions src/proto/request.c
Original file line number Diff line number Diff line change
Expand Up @@ -361,8 +361,9 @@ static char *oidc_request_uri_request_object(request_rec *r, struct oidc_provide
apr_table_do(oidc_request_uri_delete_from_request, &data, delete_from_query_params, NULL);

/* debug logging */
oidc_debug(r, "request object: %s",
oidc_util_encode_json_object(r, request_object->payload.value.json, JSON_COMPACT));
oidc_debug(
r, "request object: %s",
oidc_util_encode_json_object(r, request_object->payload.value.json, JSON_PRESERVE_ORDER | JSON_COMPACT));

char *serialized_request_object = NULL;
oidc_jose_error_t err;
Expand Down
6 changes: 4 additions & 2 deletions src/session.c
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,9 @@ void oidc_session_set_filtered_claims(request_rec *r, oidc_session_t *z, const c
}

if (is_allowed == TRUE) {
s = value ? oidc_util_encode_json_object(r, value, JSON_COMPACT | JSON_ENCODE_ANY) : "";
s = value ? oidc_util_encode_json_object(r, value,
JSON_PRESERVE_ORDER | JSON_COMPACT | JSON_ENCODE_ANY)
: "";
if (_oidc_strlen(s) > warn_claim_size)
oidc_warn(r,
"(encoded) value size of [%s] claim \"%s\" is larger than %d; consider "
Expand All @@ -558,7 +560,7 @@ void oidc_session_set_filtered_claims(request_rec *r, oidc_session_t *z, const c
iter = json_object_iter_next(src, iter);
}

const char *filtered_claims = oidc_util_encode_json_object(r, dst, JSON_COMPACT);
const char *filtered_claims = oidc_util_encode_json_object(r, dst, JSON_PRESERVE_ORDER | JSON_COMPACT);
filtered_claims = oidc_util_jq_filter(r, filtered_claims,
oidc_util_apr_expr_exec(r, oidc_cfg_filter_claims_expr_get(c), TRUE));
json_decref(dst);
Expand Down
20 changes: 9 additions & 11 deletions src/util.c
Original file line number Diff line number Diff line change
Expand Up @@ -945,8 +945,9 @@ apr_byte_t oidc_util_request_parameter_get(request_rec *r, char *name, char **va
static apr_byte_t oidc_util_json_string_print(request_rec *r, json_t *result, const char *key, const char *log) {
json_t *value = json_object_get(result, key);
if (value != NULL && !json_is_null(value)) {
oidc_error(r, "%s: response contained an \"%s\" entry with value: \"%s\"", log, key,
oidc_util_encode_json_object(r, value, JSON_ENCODE_ANY));
oidc_error(
r, "%s: response contained an \"%s\" entry with value: \"%s\"", log, key,
oidc_util_encode_json_object(r, value, JSON_PRESERVE_ORDER | JSON_COMPACT | JSON_ENCODE_ANY));
return TRUE;
}
return FALSE;
Expand Down Expand Up @@ -1540,10 +1541,6 @@ void oidc_util_set_app_infos(request_rec *r, json_t *j_attrs, const char *claim_
s_key = json_object_iter_key(iter);
j_value = json_object_iter_value(iter);

// char *s_value= json_dumps(j_value, JSON_ENCODE_ANY);
// oidc_util_set_app_info(r, s_key, s_value, claim_prefix);
// free(s_value);

/* check if it is a single value string */
if (json_is_string(j_value)) {

Expand Down Expand Up @@ -1576,8 +1573,9 @@ void oidc_util_set_app_infos(request_rec *r, json_t *j_attrs, const char *claim_
} else if (json_is_object(j_value)) {

/* set json value in the application header whose name is based on the key and the prefix */
oidc_util_set_app_info(r, s_key, oidc_util_encode_json_object(r, j_value, 0), claim_prefix,
pass_in, encoding);
oidc_util_set_app_info(
r, s_key, oidc_util_encode_json_object(r, j_value, JSON_PRESERVE_ORDER | JSON_COMPACT),
claim_prefix, pass_in, encoding);

/* check if it is a multi-value string */
} else if (json_is_array(j_value)) {
Expand Down Expand Up @@ -1749,8 +1747,8 @@ apr_byte_t oidc_util_json_merge(request_rec *r, json_t *src, json_t *dst) {
if ((src == NULL) || (dst == NULL))
return FALSE;

oidc_debug(r, "src=%s, dst=%s", oidc_util_encode_json_object(r, src, JSON_COMPACT),
oidc_util_encode_json_object(r, dst, JSON_COMPACT));
oidc_debug(r, "src=%s, dst=%s", oidc_util_encode_json_object(r, src, JSON_PRESERVE_ORDER | JSON_COMPACT),
oidc_util_encode_json_object(r, dst, JSON_PRESERVE_ORDER | JSON_COMPACT));

iter = json_object_iter(src);
while (iter) {
Expand All @@ -1760,7 +1758,7 @@ apr_byte_t oidc_util_json_merge(request_rec *r, json_t *src, json_t *dst) {
iter = json_object_iter_next(src, iter);
}

oidc_debug(r, "result dst=%s", oidc_util_encode_json_object(r, dst, JSON_COMPACT));
oidc_debug(r, "result dst=%s", oidc_util_encode_json_object(r, dst, JSON_PRESERVE_ORDER | JSON_COMPACT));

return TRUE;
}
Expand Down

0 comments on commit 5f3abf6

Please sign in to comment.