Skip to content

Commit

Permalink
fix(formatting/#2820): Range overflow when parsing ranges from extens…
Browse files Browse the repository at this point in the history
…ion host

__Issue:__ Onivim 2 wasn't correctly handling ranges from the extension host that were created with `Number.MAX_VALUE` (or, any value with any exponent...). A `ParseFailedException("Expected null or:\n while decoding a list:\n element 0:\n in field \"range\":\n in field \"endColumn\":\n Expected an int, but got 1.7976931248623157e+308\n".` was being produced.

__Defect:__ Our JSON parsing was only handling integers

__Fix:__ Use the `float` JSON decoder, which does handle exponents, and cast to an `int` - handling overflows.

This fixes the parse failure, and allows for some basic formatting - but I'm still seeing some quirks with the ReScript formatter, when updating the buffer and running the formatter again. 

Related #2820
  • Loading branch information
bryphe authored Dec 12, 2020
1 parent ff003ff commit f980d9f
Showing 1 changed file with 61 additions and 12 deletions.
73 changes: 61 additions & 12 deletions src/Exthost/OneBasedRange.re
Original file line number Diff line number Diff line change
Expand Up @@ -48,18 +48,67 @@ let toRange = ({startLineNumber, endLineNumber, startColumn, endColumn}) => {
};

let decode =
Json.Decode.(
{
obj(({field, _}) =>
{
startLineNumber: field.required("startLineNumber", int),
endLineNumber: field.required("endLineNumber", int),
startColumn: field.required("startColumn", int),
endColumn: field.required("endColumn", int),
}
);
}
);
Json.Decode.
// The default `int` decoder, or `Number` json type,
// doesn't handle exponents. So we need to use the float decoder / type,
// and coerce it to an int.
(
{
let intWithExponent =
float
|> map(f
// If the value is outside of `Int.max_int` or `Int.min_int`,
// `float_of_int` returns 0:
// https://stackoverflow.com/questions/48871692/how-to-handle-integer-overflow-in-ocaml-when-converting-from-float-using-int-of
=>
if (f >= float_of_int(Int.max_int)) {
Int.max_int;
} else if (f <= float_of_int(Int.min_int)) {
Int.min_int;
} else {
int_of_float(f);
}
);
obj(({field, _}) =>
{
startLineNumber:
field.required("startLineNumber", intWithExponent),
endLineNumber: field.required("endLineNumber", intWithExponent),
startColumn: field.required("startColumn", intWithExponent),
endColumn: field.required("endColumn", intWithExponent),
}
);
}
);

let%test_module "decode" =
(module
{
let%test "#2820 - Number.MAX_VALUE handling" = {
let json = {|
{
"startLineNumber": 0,
"startColumn": 0,
"endLineNumber": 1.7976931348623157e+308,
"endColumn": 1.7976931348623157e+308
}
|};

let range =
json
|> Yojson.Safe.from_string
|> Json.Decode.decode_value(decode)
|> Result.get_ok;

range
== {
startLineNumber: 0,
startColumn: 0,
endLineNumber: Int.max_int,
endColumn: Int.max_int,
};
};
});

let encode = range =>
Json.Encode.(
Expand Down

0 comments on commit f980d9f

Please sign in to comment.