Skip to content

Commit

Permalink
MONGOCRYPT-703 Error if precision-mode encoding cannot be used (#854)
Browse files Browse the repository at this point in the history
* pass `use_range_v2` to `mc_getTypeInfoDouble`

* add failing test for double

* return error for double

Only applies when rangev2 enabled. Get double test passing.

* pass `use_range_v2` to `mc_getTypeInfoDecimal128`

* add failing test for decimal

* return error for decimal

Only applies when rangev2 enabled. Get decimal test passing.

* refactor: move domain check into `canUsePrecisionMode` helper

Matches server implementation. Intended to be used in tests to check for invalid domains.

* check for invalid domains in mincover tests
  • Loading branch information
kevinAlbs authored Jul 3, 2024
1 parent 1337914 commit 2036b62
Show file tree
Hide file tree
Showing 14 changed files with 256 additions and 110 deletions.
4 changes: 2 additions & 2 deletions src/mc-range-edge-generation-private.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ typedef struct {

// mc_getEdgesDouble implements the Edge Generation algorithm described in
// SERVER-67751 for double.
mc_edges_t *mc_getEdgesDouble(mc_getEdgesDouble_args_t args, mongocrypt_status_t *status);
mc_edges_t *mc_getEdgesDouble(mc_getEdgesDouble_args_t args, mongocrypt_status_t *status, bool use_range_v2);

#if MONGOCRYPT_HAVE_DECIMAL128_SUPPORT
typedef struct {
Expand All @@ -86,7 +86,7 @@ typedef struct {
uint32_t trimFactor;
} mc_getEdgesDecimal128_args_t;

mc_edges_t *mc_getEdgesDecimal128(mc_getEdgesDecimal128_args_t args, mongocrypt_status_t *status);
mc_edges_t *mc_getEdgesDecimal128(mc_getEdgesDecimal128_args_t args, mongocrypt_status_t *status, bool use_range_v2);
#endif // MONGOCRYPT_HAVE_DECIMAL128_SUPPORT

BSON_STATIC_ASSERT2(ull_is_u64, sizeof(uint64_t) == sizeof(unsigned long long));
Expand Down
10 changes: 6 additions & 4 deletions src/mc-range-edge-generation.c
Original file line number Diff line number Diff line change
Expand Up @@ -182,14 +182,15 @@ mc_edges_t *mc_getEdgesInt64(mc_getEdgesInt64_args_t args, mongocrypt_status_t *
return ret;
}

mc_edges_t *mc_getEdgesDouble(mc_getEdgesDouble_args_t args, mongocrypt_status_t *status) {
mc_edges_t *mc_getEdgesDouble(mc_getEdgesDouble_args_t args, mongocrypt_status_t *status, bool use_range_v2) {
mc_OSTType_Double got;
if (!mc_getTypeInfoDouble((mc_getTypeInfoDouble_args_t){.value = args.value,
.min = args.min,
.max = args.max,
.precision = args.precision},
&got,
status)) {
status,
use_range_v2)) {
return NULL;
}

Expand All @@ -206,7 +207,7 @@ mc_edges_t *mc_getEdgesDouble(mc_getEdgesDouble_args_t args, mongocrypt_status_t
}

#if MONGOCRYPT_HAVE_DECIMAL128_SUPPORT
mc_edges_t *mc_getEdgesDecimal128(mc_getEdgesDecimal128_args_t args, mongocrypt_status_t *status) {
mc_edges_t *mc_getEdgesDecimal128(mc_getEdgesDecimal128_args_t args, mongocrypt_status_t *status, bool use_range_v2) {
mc_OSTType_Decimal128 got;
if (!mc_getTypeInfoDecimal128(
(mc_getTypeInfoDecimal128_args_t){
Expand All @@ -216,7 +217,8 @@ mc_edges_t *mc_getEdgesDecimal128(mc_getEdgesDecimal128_args_t args, mongocrypt_
.precision = args.precision,
},
&got,
status)) {
status,
use_range_v2)) {
return NULL;
}

Expand Down
12 changes: 10 additions & 2 deletions src/mc-range-encoding-private.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,15 @@ typedef struct {
mc_optional_uint32_t precision;
} mc_getTypeInfoDouble_args_t;

// `mc_canUsePrecisionModeDouble` returns true if the domain can be represented in fewer than 64 bits.
bool mc_canUsePrecisionModeDouble(double min, double max, uint32_t precision, uint32_t *maxBitsOut);

/* mc_getTypeInfoDouble encodes the double `args.value` into an OSTType_Double
* `out`. Returns false and sets `status` on error. */
bool mc_getTypeInfoDouble(mc_getTypeInfoDouble_args_t args,
mc_OSTType_Double *out,
mongocrypt_status_t *status) MONGOCRYPT_WARN_UNUSED_RESULT;
mongocrypt_status_t *status,
bool use_range_v2) MONGOCRYPT_WARN_UNUSED_RESULT;

#if MONGOCRYPT_HAVE_DECIMAL128_SUPPORT
/**
Expand All @@ -106,6 +110,9 @@ typedef struct {
mc_optional_uint32_t precision;
} mc_getTypeInfoDecimal128_args_t;

// `mc_canUsePrecisionModeDecimal` returns true if the domain can be represented in fewer than 128 bits.
bool mc_canUsePrecisionModeDecimal(mc_dec128 min, mc_dec128 max, uint32_t precision, uint32_t *maxBitsOut);

/**
* @brief Obtain the OST encoding of a finite Decimal128 value.
*
Expand All @@ -116,7 +123,8 @@ typedef struct {
*/
bool mc_getTypeInfoDecimal128(mc_getTypeInfoDecimal128_args_t args,
mc_OSTType_Decimal128 *out,
mongocrypt_status_t *status) MONGOCRYPT_WARN_UNUSED_RESULT;
mongocrypt_status_t *status,
bool use_range_v2) MONGOCRYPT_WARN_UNUSED_RESULT;
#endif // MONGOCRYPT_HAVE_DECIMAL128_SUPPORT

#endif /* MC_RANGE_ENCODING_PRIVATE_H */
148 changes: 94 additions & 54 deletions src/mc-range-encoding.c
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,38 @@ bool mc_getTypeInfo64(mc_getTypeInfo64_args_t args, mc_OSTType_Int64 *out, mongo

#define exp10Double(x) pow(10, x)

bool mc_getTypeInfoDouble(mc_getTypeInfoDouble_args_t args, mc_OSTType_Double *out, mongocrypt_status_t *status) {
bool mc_canUsePrecisionModeDouble(double min, double max, uint32_t precision, uint32_t *maxBitsOut) {
BSON_ASSERT_PARAM(maxBitsOut);

bool use_precision_mode = false;
double range = max - min;

// We can overflow if max = max double and min = min double so make sure
// we have finite number after we do subtraction
// Ignore conversion warnings to fix error with glibc.
if (mc_isfinite(range)) {
// This creates a range which is wider then we permit by our min/max
// bounds check with the +1 but it is as the algorithm is written in
// WRITING-11907.
double rangeAndPrecision = (range + 1) * exp10Double(precision);

if (mc_isfinite(rangeAndPrecision)) {
double bits_range_double = log2(rangeAndPrecision);
*maxBitsOut = (uint32_t)ceil(bits_range_double);

if (*maxBitsOut < 64) {
use_precision_mode = true;
}
}
}

return use_precision_mode;
}

bool mc_getTypeInfoDouble(mc_getTypeInfoDouble_args_t args,
mc_OSTType_Double *out,
mongocrypt_status_t *status,
bool use_range_v2) {
if (args.min.set != args.max.set || args.min.set != args.precision.set) {
CLIENT_ERR("min, max, and precision must all be set or must all be unset");
return false;
Expand Down Expand Up @@ -216,25 +247,15 @@ bool mc_getTypeInfoDouble(mc_getTypeInfoDouble_args_t args, mc_OSTType_Double *o
return false;
}

double range = args.max.value - args.min.value;

// We can overflow if max = max double and min = min double so make sure
// we have finite number after we do subtraction
// Ignore conversion warnings to fix error with glibc.
if (mc_isfinite(range)) {
// This creates a range which is wider then we permit by our min/max
// bounds check with the +1 but it is as the algorithm is written in
// WRITING-11907.
double rangeAndPrecision = (range + 1) * exp10Double(args.precision.value);

if (mc_isfinite(rangeAndPrecision)) {
double bits_range_double = log2(rangeAndPrecision);
bits_range = (uint32_t)ceil(bits_range_double);

if (bits_range < 64) {
use_precision_mode = true;
}
}
use_precision_mode =
mc_canUsePrecisionModeDouble(args.min.value, args.max.value, args.precision.value, &bits_range);
if (!use_precision_mode && use_range_v2) {
CLIENT_ERR("The domain of double values specified by the min, max, and precision cannot be represented in "
"fewer than 64 bits. min: %g, max: %g, precision: %" PRIu32,
args.min.value,
args.max.value,
args.precision.value);
return false;
}
}

Expand Down Expand Up @@ -324,9 +345,48 @@ static mlib_int128 dec128_to_int128(mc_dec128 dec) {
return ret;
}

bool mc_canUsePrecisionModeDecimal(mc_dec128 min, mc_dec128 max, uint32_t precision, uint32_t *maxBitsOut) {
BSON_ASSERT_PARAM(maxBitsOut);

bool use_precision_mode = false;
// max - min
mc_dec128 bounds_n1 = mc_dec128_sub(max, min);
// The size of [min, max]: (max - min) + 1
mc_dec128 bounds = mc_dec128_add(bounds_n1, MC_DEC128_ONE);

// We can overflow if max = max_dec128 and min = min_dec128 so make sure
// we have finite number after we do subtraction
if (mc_dec128_is_finite(bounds)) {
// This creates a range which is wider then we permit by our min/max
// bounds check with the +1 but it is as the algorithm is written in
// WRITING-11907.
mc_dec128 precision_scaled_bounds = mc_dec128_scale(bounds, precision);
/// The number of bits required to hold the result for the given
/// precision (as decimal)
mc_dec128 bits_range_dec = mc_dec128_log2(precision_scaled_bounds);

if (mc_dec128_is_finite(bits_range_dec) && mc_dec128_less(bits_range_dec, MC_DEC128(128))) {
// We need fewer than 128 bits to hold the result. But round up,
// just to be sure:
int64_t r = mc_dec128_to_int64(mc_dec128_round_integral_ex(bits_range_dec, MC_DEC128_ROUND_UPWARD, NULL));
BSON_ASSERT(r >= 0);
BSON_ASSERT(r <= UINT8_MAX);
// We've computed the proper 'bits_range'
*maxBitsOut = (uint8_t)r;

if (*maxBitsOut < 128) {
use_precision_mode = true;
}
}
}

return use_precision_mode;
}

bool mc_getTypeInfoDecimal128(mc_getTypeInfoDecimal128_args_t args,
mc_OSTType_Decimal128 *out,
mongocrypt_status_t *status) {
mongocrypt_status_t *status,
bool use_range_v2) {
/// Basic param checks
if (args.min.set != args.max.set || args.min.set != args.precision.set) {
CLIENT_ERR("min, max, and precision must all be set or must all be unset");
Expand Down Expand Up @@ -376,44 +436,24 @@ bool mc_getTypeInfoDecimal128(mc_getTypeInfoDecimal128_args_t args,
// full range.
bool use_precision_mode = false;
// The number of bits required to hold the result (used for precision mode)
uint8_t bits_range = 0;
uint32_t bits_range = 0;
if (args.precision.set) {
// Subnormal representations can support up to 5x10^-6182 as a number
if (args.precision.value > 6182) {
CLIENT_ERR("Precision must be between 0 and 6182 inclusive, got: %" PRIu32, args.precision.value);
return false;
}

// max - min
mc_dec128 bounds_n1 = mc_dec128_sub(args.max.value, args.min.value);
// The size of [min, max]: (max - min) + 1
mc_dec128 bounds = mc_dec128_add(bounds_n1, MC_DEC128_ONE);

// We can overflow if max = max_dec128 and min = min_dec128 so make sure
// we have finite number after we do subtraction
if (mc_dec128_is_finite(bounds)) {
// This creates a range which is wider then we permit by our min/max
// bounds check with the +1 but it is as the algorithm is written in
// WRITING-11907.
mc_dec128 precision_scaled_bounds = mc_dec128_scale(bounds, args.precision.value);
/// The number of bits required to hold the result for the given
/// precision (as decimal)
mc_dec128 bits_range_dec = mc_dec128_log2(precision_scaled_bounds);

if (mc_dec128_is_finite(bits_range_dec) && mc_dec128_less(bits_range_dec, MC_DEC128(128))) {
// We need fewer than 128 bits to hold the result. But round up,
// just to be sure:
int64_t r =
mc_dec128_to_int64(mc_dec128_round_integral_ex(bits_range_dec, MC_DEC128_ROUND_UPWARD, NULL));
BSON_ASSERT(r >= 0);
BSON_ASSERT(r <= UINT8_MAX);
// We've computed the proper 'bits_range'
bits_range = (uint8_t)r;

if (bits_range < 128) {
use_precision_mode = true;
}
}
use_precision_mode =
mc_canUsePrecisionModeDecimal(args.min.value, args.max.value, args.precision.value, &bits_range);

if (!use_precision_mode && use_range_v2) {
CLIENT_ERR("The domain of decimal values specified by the min, max, and precision cannot be represented in "
"fewer than 128 bits. min: %s, max: %s, precision: %" PRIu32,
mc_dec128_to_string(args.min.value).str,
mc_dec128_to_string(args.max.value).str,
args.precision.value);
return false;
}
}

Expand Down Expand Up @@ -457,9 +497,9 @@ bool mc_getTypeInfoDecimal128(mc_getTypeInfoDecimal128_args_t args,
v_prime2 = mc_dec128_round_integral_ex(v_prime2, MC_DEC128_ROUND_TOWARD_ZERO, NULL);

BSON_ASSERT(mc_dec128_less(mc_dec128_log2(v_prime2), MC_DEC128(128)));

BSON_ASSERT(bits_range < 128);
// Resulting OST maximum
mlib_int128 ost_max = mlib_int128_sub(mlib_int128_pow2(bits_range), i128_one);
mlib_int128 ost_max = mlib_int128_sub(mlib_int128_pow2((uint8_t)bits_range), i128_one);

// Now we need to get the Decimal128 out as a 128-bit integer
// But Decimal128 does not support conversion to Int128.
Expand Down
6 changes: 4 additions & 2 deletions src/mc-range-mincover-private.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ typedef struct {
// mc_getMincoverDouble implements the Mincover Generation algorithm described
// in SERVER-68600 for double.
mc_mincover_t *mc_getMincoverDouble(mc_getMincoverDouble_args_t args,
mongocrypt_status_t *status) MONGOCRYPT_WARN_UNUSED_RESULT;
mongocrypt_status_t *status,
bool use_range_v2) MONGOCRYPT_WARN_UNUSED_RESULT;

#if MONGOCRYPT_HAVE_DECIMAL128_SUPPORT
typedef struct {
Expand All @@ -100,7 +101,8 @@ typedef struct {
// mc_getMincoverDecimal128 implements the Mincover Generation algorithm
// described in SERVER-68600 for Decimal128 (as mc_dec128).
mc_mincover_t *mc_getMincoverDecimal128(mc_getMincoverDecimal128_args_t args,
mongocrypt_status_t *status) MONGOCRYPT_WARN_UNUSED_RESULT;
mongocrypt_status_t *status,
bool use_range_v2) MONGOCRYPT_WARN_UNUSED_RESULT;
#endif // MONGOCRYPT_HAVE_DECIMAL128_SUPPORT

#endif /* MC_RANGE_MINCOVER_PRIVATE_H */
17 changes: 11 additions & 6 deletions src/mc-range-mincover.c
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ mc_mincover_t *mc_getMincoverInt64(mc_getMincoverInt64_args_t args, mongocrypt_s

// mc_getMincoverDouble implements the Mincover Generation algorithm described
// in SERVER-68600 for double.
mc_mincover_t *mc_getMincoverDouble(mc_getMincoverDouble_args_t args, mongocrypt_status_t *status) {
mc_mincover_t *mc_getMincoverDouble(mc_getMincoverDouble_args_t args, mongocrypt_status_t *status, bool use_range_v2) {
BSON_ASSERT_PARAM(status);
CHECK_BOUNDS(args, "g", IDENTITY, LESSTHAN);

Expand All @@ -203,15 +203,17 @@ mc_mincover_t *mc_getMincoverDouble(mc_getMincoverDouble_args_t args, mongocrypt
.max = args.max,
.precision = args.precision},
&a,
status)) {
status,
use_range_v2)) {
return NULL;
}
if (!mc_getTypeInfoDouble((mc_getTypeInfoDouble_args_t){.value = args.upperBound,
.min = args.min,
.max = args.max,
.precision = args.precision},
&b,
status)) {
status,
use_range_v2)) {
return NULL;
}

Expand All @@ -233,7 +235,8 @@ mc_mincover_t *mc_getMincoverDouble(mc_getMincoverDouble_args_t args, mongocrypt
}

#if MONGOCRYPT_HAVE_DECIMAL128_SUPPORT
mc_mincover_t *mc_getMincoverDecimal128(mc_getMincoverDecimal128_args_t args, mongocrypt_status_t *status) {
mc_mincover_t *
mc_getMincoverDecimal128(mc_getMincoverDecimal128_args_t args, mongocrypt_status_t *status, bool use_range_v2) {
BSON_ASSERT_PARAM(status);
#define ToString(Dec) (mc_dec128_to_string(Dec).str)
CHECK_BOUNDS(args, "s", ToString, mc_dec128_less);
Expand All @@ -244,15 +247,17 @@ mc_mincover_t *mc_getMincoverDecimal128(mc_getMincoverDecimal128_args_t args, mo
.max = args.max,
.precision = args.precision},
&a,
status)) {
status,
use_range_v2)) {
return NULL;
}
if (!mc_getTypeInfoDecimal128((mc_getTypeInfoDecimal128_args_t){.value = args.upperBound,
.min = args.min,
.max = args.max,
.precision = args.precision},
&b,
status)) {
status,
use_range_v2)) {
return NULL;
}

Expand Down
3 changes: 2 additions & 1 deletion src/mc-rangeopts-private.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ bool mc_RangeOpts_appendTrimFactor(const mc_RangeOpts_t *ro,
bson_type_t valueType,
const char *fieldName,
bson_t *out,
mongocrypt_status_t *status);
mongocrypt_status_t *status,
bool use_range_v2);

void mc_RangeOpts_cleanup(mc_RangeOpts_t *ro);

Expand Down
Loading

0 comments on commit 2036b62

Please sign in to comment.