Why does hex_string_to_byte require an even number of characters?

[d-e-s-o]

Below is the "head" of the hex_string_to_byte function:

::std::vector<uint8_t> hex_string_to_byte(const char* hexString){
    const size_t big_string_size = 257; //arbitrary 'big' number
    const size_t s_size = strnlen(hexString, big_string_size);
    const size_t d_size = s_size/2;
    if (s_size%2!=0 || s_size>=big_string_size){
        throw InvalidHexString(0);
    }
    ...

Can you please clarify why this function imposes a requirement that the provided hexString have an even number of characters? Isn't that an unnecessary limitation? Why would ace, for example, not be valid? Isn't it well defined that 0xa == 0x0a? Can we please adjust this function to be more friendly to user input?

This restriction is causing issues with input to nitrocli, which invokes write_OTP_slot_no_authorize as part of the otp set command which in turn uses hex_string_to_byte internally.

[szszszsz]

Hi! Sorry for the late reply!

The assumption is, that the user input validation and correction is done at the client application, which is further the caller for the libnitrokey. It comes from the reasoning, that library should be limited to only one task and to do it good, which is in this case handling the behavior of multiple devices' models, with different firmware versions. Here the validation is only in the assert mode, to make sure the hex->bin converting code (which is as simple as possible) is working in the laid assumptions of the input data.

Additionally when starting the data correction on the library side, it is hard to say what user wanted. It is true the 0 should be prefixed to the string, but was this the user intention? I think the client application should confirm that IMHO. I would even say, that there was a problem during copying the secret - why any web client would even return odd hex string?
But let's think about other (not proposed here) rules - should slot -1 be treated as first slot, or as the last? What would be the correction of the library? Such complicates not only the code of the it, but the usage as well, while it was planned to be an as raw interface to the device as possible.
This does not exclude 'higher' level functions, e.g. to access to slot by name which I remember was requested too, but it has to be marked as such.

This is especially true while it can be fixed easily, as you have mentioned. I am sorry it was missed from the documentation - this is indeed unwelcomed. I plan to work on the docs with the nearest release.

Closing, for me it is wontfix for the low-level interface, but could potentially add it as a 'high-level' interface on request. I find it unnecessary though, since it will expand the library needlessly in the direction it was not developed for in the initial plan. I am open to arguments.

[szszszsz]

BTW feel free to ping me, when needed. I remembered to respond to that, but had to take down some other tasks first.