Major cleanup in our baseXX APIs
Our base16/32/64 APIs are quite inconsistent and a timebomb of possible errors and issues. Here is some recommendation with this:
- We should have for each type of encoding a function that returns the encoded size using a source length. We have such function for baese32 and base64 but they are not consistent:
base32_encoded_size()returns the size including the NUL byte
base64_encode_size()has a different name and does NOT add the NUL byte.
They should all return the size with the extra NUL byte because every
_encode() function we have requires it in the first place. The other solution is to have a new function
baseXX_encoded_string_size() or something that return the NUL byte and the non string function returns the value without it.
Else we end up with this kind of code pattern:
len = base64_encoded_size() + 1 buf = tor_malloc_zero(len) ret = base64_encode() tor_assert(ret == len - 1)
or the +1 added explicitly like so which is not good.
base32_encode(serviceid, REND_SERVICE_ID_LEN_BASE32+1, circuit->rend_data->rend_pk_digest, REND_SERVICE_ID_LEN);
or even better:
base16_encode(hex, 2*CONDITIONAL_CONSENSUS_FPR_LEN+1, ds->v3_identity_digest, CONDITIONAL_CONSENSUS_FPR_LEN);
- Source length requirement issue. I think though most of them are fixed except base64 API with ticket legacy/trac#17868 (moved).
I do see this in the
base16_decode which could either be a hard requirement assert or assume that if it gets 9 bytes in srclen, well only 8 are decoded. We have callsites that do NOT check the returned value so...
if ((srclen % 2) != 0) return -1;
- Every baseXX_encode/decode should return the amount of bytes that has been encoded or decoded. They also should return
ssize_t;but that's another ticket I feel like because it's a large refactoring. Here are the missing pieces:
base16_encode()returns void so it should return
intas the encoded length.
base32_decode()returns 0 on success.
- We should also have macros for the encoded length computation else we ended up with stuff like this (taken from the prop250 branch).
#define SR_SRV_VALUE_BASE64_LEN \ (((DIGEST256_LEN - 1) / 3) * 4 + 4)
or flat values
#define REND_DESC_COOKIE_LEN_BASE64 22
This is a static calculation so most of the times that macro would be more useful than the function since it could be used for stack allocation which is a BIG use case of ours.