From 5e8a69d547461c757abe2870ecbff2aa7a1fea55 Mon Sep 17 00:00:00 2001 From: Nikolai Kondrashov Date: Wed, 1 Oct 2014 11:51:51 -0400 Subject: [PATCH 2/4] Access union value_data members consistently Use the same, appropriate union value_data member for each access of BOOLEAN, BYTE and SHORT PW_TYPEs, without assuming they're interchangeable with "integer", as that is only true on little-endian architectures. This fixes at least this wimax unit test failure on s390x and ppc64: Mismatch in line 11 of src/tests/unit/wimax.txt, got: 1a 0c 00 00 60 b5 01 06 00 02 03 00 expected: 1a 0c 00 00 60 b5 01 06 00 02 03 01 --- src/lib/print.c | 56 ++++++++++++------ src/lib/radius.c | 8 +-- src/lib/valuepair.c | 83 +++++++++++++++++++-------- src/main/evaluate.c | 4 +- src/main/valuepair.c | 4 ++ src/main/xlat.c | 4 +- src/modules/rlm_couchbase/mod.c | 17 +++++- src/modules/rlm_eap/types/rlm_eap_ttls/ttls.c | 4 +- 8 files changed, 128 insertions(+), 52 deletions(-) diff --git a/src/lib/print.c b/src/lib/print.c index 67263bc..fc1ae42 100644 --- a/src/lib/print.c +++ b/src/lib/print.c @@ -314,6 +314,7 @@ size_t vp_data_prints_value(char *out, size_t outlen, char const *a = NULL; time_t t; struct tm s_tm; + unsigned int i; size_t len = 0, freespace = outlen; @@ -365,15 +366,24 @@ size_t vp_data_prints_value(char *out, size_t outlen, return fr_print_string(data->strvalue, data_len, out, outlen); case PW_TYPE_INTEGER: - case PW_TYPE_BYTE: + i = data->integer; + goto print_int; + case PW_TYPE_SHORT: + i = data->ushort; + goto print_int; + + case PW_TYPE_BYTE: + i = data->byte; + +print_int: /* Normal, non-tagged attribute */ - if ((v = dict_valbyattr(da->attr, da->vendor, data->integer)) != NULL) { + if ((v = dict_valbyattr(da->attr, da->vendor, i)) != NULL) { a = v->name; len = strlen(a); } else { /* should never be truncated */ - len = snprintf(buf, sizeof(buf), "%u", data->integer); + len = snprintf(buf, sizeof(buf), "%u", i); a = buf; } break; @@ -590,12 +600,20 @@ size_t vp_prints_value_json(char *out, size_t outlen, VALUE_PAIR const *vp) if (!vp->da->flags.has_tag) { switch (vp->da->type) { case PW_TYPE_INTEGER: - case PW_TYPE_BYTE: - case PW_TYPE_SHORT: if (vp->da->flags.has_value) break; return snprintf(out, freespace, "%u", vp->vp_integer); + case PW_TYPE_SHORT: + if (vp->da->flags.has_value) break; + + return snprintf(out, freespace, "%u", (unsigned int) vp->vp_short); + + case PW_TYPE_BYTE: + if (vp->da->flags.has_value) break; + + return snprintf(out, freespace, "%u", (unsigned int) vp->vp_byte); + case PW_TYPE_SIGNED: return snprintf(out, freespace, "%d", vp->vp_signed); @@ -834,6 +852,8 @@ void vp_printlist(FILE *fp, VALUE_PAIR const *vp) char *vp_aprint_value(TALLOC_CTX *ctx, VALUE_PAIR const *vp, bool escape) { char *p; + unsigned int i; + DICT_VALUE const *dv; switch (vp->da->type) { case PW_TYPE_STRING: @@ -860,19 +880,23 @@ char *vp_aprint_value(TALLOC_CTX *ctx, VALUE_PAIR const *vp, bool escape) break; } - case PW_TYPE_BYTE: - case PW_TYPE_SHORT: case PW_TYPE_INTEGER: - { - DICT_VALUE *dv; + i = vp->vp_integer; + goto print_int; - dv = dict_valbyattr(vp->da->attr, vp->da->vendor, - vp->vp_integer); - if (dv) { - p = talloc_typed_strdup(ctx, dv->name); - } else { - p = talloc_typed_asprintf(ctx, "%u", vp->vp_integer); - } + case PW_TYPE_SHORT: + i = vp->vp_short; + goto print_int; + + case PW_TYPE_BYTE: + i = vp->vp_byte; + + print_int: + dv = dict_valbyattr(vp->da->attr, vp->da->vendor, i); + if (dv) { + p = talloc_typed_strdup(ctx, dv->name); + } else { + p = talloc_typed_asprintf(ctx, "%u", i); } break; diff --git a/src/lib/radius.c b/src/lib/radius.c index 0a40682..aabc545 100644 --- a/src/lib/radius.c +++ b/src/lib/radius.c @@ -3984,18 +3984,18 @@ ssize_t rad_vp2data(uint8_t const **out, VALUE_PAIR const *vp) } case PW_TYPE_BOOLEAN: - buffer[0] = vp->vp_integer & 0x01; + buffer[0] = vp->vp_byte & 0x01; *out = buffer; break; case PW_TYPE_BYTE: - buffer[0] = vp->vp_integer & 0xff; + buffer[0] = vp->vp_byte & 0xff; *out = buffer; break; case PW_TYPE_SHORT: - buffer[0] = (vp->vp_integer >> 8) & 0xff; - buffer[1] = vp->vp_integer & 0xff; + buffer[0] = (vp->vp_short >> 8) & 0xff; + buffer[1] = vp->vp_short & 0xff; *out = buffer; break; diff --git a/src/lib/valuepair.c b/src/lib/valuepair.c index 9dcae70..7d6ee88 100644 --- a/src/lib/valuepair.c +++ b/src/lib/valuepair.c @@ -1369,65 +1369,100 @@ int pairparsevalue(VALUE_PAIR *vp, char const *value, size_t inlen) case PW_TYPE_BYTE: { char *p; - vp->length = 1; + unsigned int i; /* * Note that ALL integers are unsigned! */ - vp->vp_integer = fr_strtoul(value, &p); - if (!*p) { - if (vp->vp_integer > 255) { + i = fr_strtoul(value, &p); + + /* + * Look for the named value for the given + * attribute. + */ + if (*p && !is_whitespace(p)) { + if ((dval = dict_valbyname(vp->da->attr, vp->da->vendor, value)) == NULL) { + fr_strerror_printf("Unknown value '%s' for attribute '%s'", value, vp->da->name); + return -1; + } + + vp->vp_byte = dval->value; + } else { + if (i > 255) { fr_strerror_printf("Byte value \"%s\" is larger than 255", value); return -1; } - break; + + vp->vp_byte = i; } - if (is_whitespace(p)) break; + + vp->length = 1; + break; } - goto check_for_value; case PW_TYPE_SHORT: { char *p; + unsigned int i; /* * Note that ALL integers are unsigned! */ - vp->vp_integer = fr_strtoul(value, &p); - vp->length = 2; - if (!*p) { - if (vp->vp_integer > 65535) { - fr_strerror_printf("Byte value \"%s\" is larger than 65535", value); + i = fr_strtoul(value, &p); + + /* + * Look for the named value for the given + * attribute. + */ + if (*p && !is_whitespace(p)) { + if ((dval = dict_valbyname(vp->da->attr, vp->da->vendor, value)) == NULL) { + fr_strerror_printf("Unknown value '%s' for attribute '%s'", value, vp->da->name); return -1; } - break; + + vp->vp_short = dval->value; + } else { + if (i > 65535) { + fr_strerror_printf("Short value \"%s\" is larger than 65535", value); + return -1; + } + + vp->vp_short = i; } - if (is_whitespace(p)) break; + + vp->length = 2; + break; } - goto check_for_value; case PW_TYPE_INTEGER: { char *p; + unsigned int i; /* * Note that ALL integers are unsigned! */ - vp->vp_integer = fr_strtoul(value, &p); - vp->length = 4; - if (!*p) break; - if (is_whitespace(p)) break; + i = fr_strtoul(value, &p); - check_for_value: /* * Look for the named value for the given * attribute. */ - if ((dval = dict_valbyname(vp->da->attr, vp->da->vendor, value)) == NULL) { - fr_strerror_printf("Unknown value '%s' for attribute '%s'", value, vp->da->name); - return -1; + if (*p && !is_whitespace(p)) { + if ((dval = dict_valbyname(vp->da->attr, vp->da->vendor, value)) == NULL) { + fr_strerror_printf("Unknown value '%s' for attribute '%s'", value, vp->da->name); + return -1; + } + + vp->vp_integer = dval->value; + } else { + /* + * Value is always within the limits + */ + vp->vp_integer = i; } - vp->vp_integer = dval->value; + + vp->length = 4; } break; diff --git a/src/main/evaluate.c b/src/main/evaluate.c index 5cf597d..a100c70 100644 --- a/src/main/evaluate.c +++ b/src/main/evaluate.c @@ -485,11 +485,11 @@ static int do_cast_copy(VALUE_PAIR *dst, VALUE_PAIR const *src) break; case PW_TYPE_SHORT: - dst->vp_integer = ntohs(*(uint16_t const *) src->vp_octets); + dst->vp_short = ntohs(*(uint16_t const *) src->vp_octets); break; case PW_TYPE_BYTE: - dst->vp_integer = src->vp_octets[0]; + dst->vp_byte = src->vp_octets[0]; break; default: diff --git a/src/main/valuepair.c b/src/main/valuepair.c index dc2bfc7..2dd517a 100644 --- a/src/main/valuepair.c +++ b/src/main/valuepair.c @@ -180,7 +180,11 @@ int radius_compare_vps(UNUSED REQUEST *request, VALUE_PAIR *check, VALUE_PAIR *v break; case PW_TYPE_BYTE: + ret = vp->vp_byte - check->vp_byte; + break; case PW_TYPE_SHORT: + ret = vp->vp_short - check->vp_short; + break; case PW_TYPE_INTEGER: ret = vp->vp_integer - check->vp_integer; break; diff --git a/src/main/xlat.c b/src/main/xlat.c index f2c8aff..a069919 100644 --- a/src/main/xlat.c +++ b/src/main/xlat.c @@ -177,9 +177,11 @@ static ssize_t xlat_integer(UNUSED void *instance, REQUEST *request, case PW_TYPE_INTEGER: case PW_TYPE_DATE: + return snprintf(out, outlen, "%u", vp->vp_integer); case PW_TYPE_BYTE: + return snprintf(out, outlen, "%u", (unsigned int) vp->vp_byte); case PW_TYPE_SHORT: - return snprintf(out, outlen, "%u", vp->vp_integer); + return snprintf(out, outlen, "%u", (unsigned int) vp->vp_short); /* * Ethernet is weird... It's network related, so we assume to it should be diff --git a/src/modules/rlm_couchbase/mod.c b/src/modules/rlm_couchbase/mod.c index cc14677..36406a0 100644 --- a/src/modules/rlm_couchbase/mod.c +++ b/src/modules/rlm_couchbase/mod.c @@ -296,22 +296,33 @@ json_object *mod_value_pair_to_json_object(REQUEST *request, VALUE_PAIR *vp) /* add this attribute/value pair to our json output */ if (!vp->da->flags.has_tag) { + unsigned int i; + switch (vp->da->type) { case PW_TYPE_INTEGER: - case PW_TYPE_BYTE: + i = vp->vp_integer; + goto print_int; + case PW_TYPE_SHORT: + i = vp->vp_short; + goto print_int; + + case PW_TYPE_BYTE: + i = vp->vp_byte; + + print_int: /* skip if we have flags */ if (vp->da->flags.has_value) break; #ifdef HAVE_JSON_OBJECT_NEW_INT64 /* debug */ RDEBUG3("creating new int64 for unsigned 32 bit int/byte/short '%s'", vp->da->name); /* return as 64 bit int - JSON spec does not support unsigned ints */ - return json_object_new_int64(vp->vp_integer); + return json_object_new_int64(i); #else /* debug */ RDEBUG3("creating new int for unsigned 32 bit int/byte/short '%s'", vp->da->name); /* return as 64 bit int - JSON spec does not support unsigned ints */ - return json_object_new_int(vp->vp_integer); + return json_object_new_int(i); #endif break; case PW_TYPE_SIGNED: diff --git a/src/modules/rlm_eap/types/rlm_eap_ttls/ttls.c b/src/modules/rlm_eap/types/rlm_eap_ttls/ttls.c index 152f4ca..55e8e14 100644 --- a/src/modules/rlm_eap/types/rlm_eap_ttls/ttls.c +++ b/src/modules/rlm_eap/types/rlm_eap_ttls/ttls.c @@ -325,12 +325,12 @@ static VALUE_PAIR *diameter2vp(REQUEST *request, REQUEST *fake, SSL *ssl, case PW_TYPE_BYTE: if (size != vp->length) goto raw; - vp->vp_integer = data[0]; + vp->vp_byte = data[0]; break; case PW_TYPE_SHORT: if (size != vp->length) goto raw; - vp->vp_integer = (data[0] * 256) + data[1]; + vp->vp_short = (data[0] * 256) + data[1]; break; case PW_TYPE_SIGNED: -- 2.1.0