Fix: Base64 encoding/decoding #2452
Conversation
mookerji
force-pushed
the
mookerji-fix-base64
branch
from
294ae87
to
2f5db62
Compare
| size_t pad_chars = std::count(padded.begin(), padded.end(), PADDING_ENCODED); | ||
| std::replace(padded.begin(), padded.end(), PADDING_ENCODED, ZERO_ENCODED); |
There was a problem hiding this comment.
it seems strange that we would iterate over the whole of the input when we know we can have at max 2 pad chars on the end of the string only. should we not just check the last 2 chars of the string? if any other chars in the string are set to the padding char then we know the input is bogus anyway.
There was a problem hiding this comment.
i suppose the other option is that someone sent multiple messages in the same string but i fail to see how we would handle that practically after decoding. in the end we just get back a bucket of bytes and lose an info about the message boundaries
test/predictedspeeds.cc
Outdated
| // HACK(mookerji): kDecodedSpeedSize+1 is the expected size (as opposed to kDecodedSpeedSize) | ||
| // because we start reading from a 1-byte offset below at the little endian conversion. This is | ||
| // actually a broken test fixture because we start reading from 0 in the actual CLI decoding in | ||
| // src/mjolnir/valhalla_add_predicted_traffic.cc |
There was a problem hiding this comment.
to fix this should we do a full round trip? we could loop over the values in the block of data below converting them to big endian, pushing back 2 bytes at a time into a string, encode the whole string, and then run the test as is but start the index at 0 instead of 1. i cant understand why this is written this way otherwise?
|
@mookerji you should add a changelog entry |
- Moves base64 decoding into proper header file, adds unit test coverage. - Fix bug in base64 decoder that failed to remove padding bytes. - Updates test fixtures impacted by improper base64 decoding. Split out from: - #2424 /cc @kevinkreiser
mookerji
force-pushed
the
mookerji-fix-base64
branch
from
69bfefe
to
fab9e25
Compare
Split out from:
/cc @kevinkreiser