Decimals parse . in the rounding position incorrectly

[CAD97]

rust-decimal/src/str.rs

Lines 358 to 364 in 4b71761

	fn maybe_round(mut data: u128, next_byte: u8, scale: u8, point: bool, negative: bool) -> Result<Decimal, crate::Error> {
	let digit = match next_byte {
	b'0'..=b'9' => u32::from(next_byte - b'0'),
	b'_' => 0, // this should be an invalid string?
	b'.' if point => 0,
	b => return tail_invalid_digit(b),
	};

[src\main.rs:109] Decimal::from_str("1.0000000000000000000000000000.5") = Ok(
    1.0000000000000000000000000000,
)

The condition here is inverted for the decimal point; it should be allowed if !point. Currently maybe_round is only called with point=true anyway.

Fixing this will reveal another case of #542 if maybe_round is ever called with point=false.

[Tony-Samuels]

I realise I'm coming to this fairly late, but what do you think the expected behaviour should be here? My thinking is that a number with multiple . should probably be rejected as invalid, however you saying that fixing this would produce another bug implies you think there is a valid way to parse a decimal with two decimal points?

[CAD97]

If I recall correctly, the fix I was getting at is

         b'_' => 0, // this should be an invalid string?
-        b'.' if point => 0,
+        b'.' if !point => 0,
         b => return tail_invalid_digit(b),

which would make 1.0000000000000000000000000000.5 correctly return an invalid parse.

The "another case of" is referring to how the => 0 is the cause of 1.0000000000000000000000000000_6 rounding down (the _ is treated as-if a 0), which would also now apply to . when point is true. I don't recall whether maybe_round is ever called with point=false (i.e. looking at a digit before the decimal point); the issue there would occur for 10000000000000000000000000000.6, which would currently fail to parse and after the above path round down instead of up.

IIRC I spotted the bug in the code rather than hitting an incorrect parse case. Thus the interestingly templated bug report focusing more on the fact that the code was written with the condition inverted than the actual observable bug.