Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error feedback for base64 decoding #253

Open
prince-chrismc opened this issue Oct 2, 2022 · 1 comment
Open

Improve error feedback for base64 decoding #253

prince-chrismc opened this issue Oct 2, 2022 · 1 comment

Comments

@prince-chrismc
Copy link
Collaborator

This problem was very difficult to detect. Because prints were the same. But strings weren't been the same. My string has a lot of white characters inside (Not only on end and begin).

Originally posted by @hanusek in #248 (comment)


Ideally the error messages would include a position and element that is invalid. Currently the messages are tok generic

@ItsAMeMarcel
Copy link
Contributor

Hi all,

I encountered this issue as well and came up with three potential improvements:

  1. Add char to the runtime error
inline uint32_t index(const std::array<int8_t, 256>& rdata, char symbol) {
	auto index = rdata[static_cast<unsigned char>(symbol)];
	if (index <= -1) {
		std::stringstream ss;
		ss << "Invalid input: '" << symbol << "' (0x" << std::hex << std::setw(2) << std::setfill('0')
		   << static_cast<int>(symbol) << ") not within alphabet";
		throw std::runtime_error(ss.str());
	}
	return static_cast<uint32_t>(index);
}
  1. Add char and offset to runtime error
inline uint32_t index(const std::array<int8_t, 256>& rdata, char symbol, size_t offset) {
	auto index = rdata[static_cast<unsigned char>(symbol)];
	if (index <= -1) {
		std::stringstream ss;
		ss << "Invalid input: '" << symbol << "' (0x" << std::hex << std::setw(2) << std::setfill('0')
		   << static_cast<int>(symbol) << ") at position " << std::dec << offset << " not within alphabet";
		throw std::runtime_error(ss.str());
	}
	return static_cast<uint32_t>(index);
}
  1. Same as 2 but handover base and offset
inline uint32_t index(const std::array<int8_t, 256>& rdata, const std::string& base, size_t offset) {
	auto symbol = base[offset];
	auto index = rdata[static_cast<unsigned char>(symbol)];
	if (index <= -1) {
		std::stringstream ss;
		ss << "Invalid input: '" << symbol << "' (0x" << std::hex << std::setw(2) << std::setfill('0')
		   << static_cast<int>(symbol) << ") at position " << std::dec << offset << " not within alphabet";
		throw std::runtime_error(ss.str());
	}
	return static_cast<uint32_t>(index);
}

Personally, I prefer option 3, as it provides the most comprehensive information, which can be extremely helpful for debugging.

I am less inclined towards option 2 because it introduces a function argument solely for error handling, which might feel unnecessary unless it's strictly needed.

What are your thoughts on these approaches?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants