You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
json.hpp, line 18822 ..
1/ the third parameter is named byte, 'byte' is also a 'type' and may lead to confusion.
It is also a public variable used in parse_error class which isn't an 8-bit value (json.hpp, line 4440)
2/ JSON_ASSERT(byte < utf8d.size()); (json.hpp, line 18844)
Since byte is declared as an uint8_t, its range is 0 < byte < 256.
This means the assert on an array of size 400 will never fail. VS2019 sometimes produces a warning for this.
3/ JSON_ASSERT(index < 400); (json.hpp, line 18852)
Shouldn't this be
JSON_ASSERT(index < utf8d.size());
Simon
Reproduction steps
"Build"->"Run Code Analyzer on Solution"
Expected vs. actual results
n/a
Minimal code example
No response
Error messages
Warning C28020 The expression '0<=_Param_(1)&&_Param_(1)<=400-1' is not true at this call.
This message doesn't always show up, I ended up analyzing the code ("Build"->"Run Code Analyzer on Solution")
Compiler and operating system
Windows 10, Microsoft Visual Studio Professional 2019
Library version
version 3.10.4
Validation
The bug also occurs if the latest version from the develop branch is used.
json.hpp, line 18822 .. 1/ the third parameter is named byte, 'byte' is also a 'type' and may lead to confusion. It is also a public variable used in parse_error class which isn't an 8-bit value (json.hpp, line 4440)
I'm not too concerned about confusion with std::byte, especially since decode() is an internal function.
We can't change a public member variable name until 4.0.0.
2/ JSON_ASSERT(byte < utf8d.size()); (json.hpp, line 18844) Since byte is declared as an uint8_t, its range is 0 < byte < 256. This means the assert on an array of size 400 will never fail. VS2019 sometimes produces a warning for this.
I'd be OK with either re-writing or completely removing the assertion.
3/ JSON_ASSERT(index < 400); (json.hpp, line 18852) Shouldn't this be JSON_ASSERT(index < utf8d.size());
It is also a public variable used in parse_error class which isn't an 8-bit value (json.hpp, line 4440)
That's fine. byte by itself is not a reserved word. It is referring to the particular byte in the stream, so it is a perfectly reasonable name. We could change it to byte_index but I don't think that's necessary.
The name byte makes sense here: the variable actually is a byte; this name is also used on http://bjoern.hoehrmann.de/utf-8/decoder/dfa/ where we got the code from. I would leave it as is. As @gregmarr pointed out, we can't rename detail::parse_error::byte as it would be a breaking change.
I'd like to leave the assertion there, because it documents that we are on the safe side to access the array at that index. Removing the assertion because it just cannot happen would then yield a comment that basically states the same: it is safe to use that untrusted value as array index. I'd rather have code than comments here.
Right - 400 is an unnecessary magic number here, and utf8d.size() much more expressive.
Description
Quite minor.. BUT
json.hpp, line 18822 ..
1/ the third parameter is named byte, 'byte' is also a 'type' and may lead to confusion.
It is also a public variable used in parse_error class which isn't an 8-bit value (json.hpp, line 4440)
2/ JSON_ASSERT(byte < utf8d.size()); (json.hpp, line 18844)
Since byte is declared as an uint8_t, its range is 0 < byte < 256.
This means the assert on an array of size 400 will never fail. VS2019 sometimes produces a warning for this.
3/ JSON_ASSERT(index < 400); (json.hpp, line 18852)
Shouldn't this be
JSON_ASSERT(index < utf8d.size());
Simon
Reproduction steps
"Build"->"Run Code Analyzer on Solution"
Expected vs. actual results
n/a
Minimal code example
No response
Error messages
Compiler and operating system
Windows 10, Microsoft Visual Studio Professional 2019
Library version
version 3.10.4
Validation
develop
branch is used.The text was updated successfully, but these errors were encountered: