I am trying to get a working and good version of base64 conversions using OpenSSL. My alternatives were an implementation from an answer on SO and Boost (which I didn't chose because I read that is pretty broken).
I will need OpenSSL in my project and I thought it would be a good idea to use it for base64 too so I can get use with the API.
For start, let's just assume:
using ByteBuffer = std::vector<std::uint8_t>;
This is the header (redi
is the main namespace of my project):
#ifndef REDI_BASE64_HPP
#define REDI_BASE64_HPP
#include "../bytebuffer.hpp"
namespace redi
{
namespace util
{
namespace base64
{
ByteBuffer encode(const ByteBuffer& data);
ByteBuffer encode(const std::string& str);
std::string encodeToString(const std::string& str);
ByteBuffer decode(const ByteBuffer& data);
ByteBuffer decode(const std::string& str);
std::string decodeToString(const ByteBuffer& data);
std::string decodeToString(const std::string& str);
} // namespace base64
} // namespace util
} // namespace redi
#endif // REDI_BASE64_HPP
and here is the source file:
#include <cassert>
#include <string>
#include <vector>
#include <openssl/bio.h>
#include <openssl/buffer.h>
#include <openssl/evp.h>
#include "base64.hpp"
namespace redi
{
namespace util
{
namespace base64
{
namespace
{
void rawBase64Encode(const std::uint8_t* input, std::size_t inputLen, char*& output, std::size_t& outputLen)
{
BIO* bio;
BIO* b64;
BUF_MEM* bufferPtr;
b64 = BIO_new(BIO_f_base64());
bio = BIO_new(BIO_s_mem());
bio = BIO_push(b64, bio);
BIO_set_flags(bio, BIO_FLAGS_BASE64_NO_NL);
BIO_write(bio, input, static_cast<int>(inputLen));
BIO_flush(bio);
BIO_get_mem_ptr(bio, std::addressof(bufferPtr));
BIO_set_close(bio, BIO_NOCLOSE);
output = bufferPtr->data;
outputLen = bufferPtr->length;
free(bufferPtr);
BIO_free_all(bio);
}
std::size_t calcDecodeLength(const std::uint8_t* input, std::size_t len)
{
std::size_t padding = 0;
if (input[len - 1] == '=')
{
++padding;
if (input[len - 2] == '=') ++padding;
}
return len * 3 / 4 - padding;
}
template <typename T>
void rawBase64Decode(const std::uint8_t* input, std::size_t inputLen, T& container)
{
BIO* bio;
BIO* b64;
container.resize(calcDecodeLength(input, inputLen));
bio = BIO_new_mem_buf(input, -1);
b64 = BIO_new(BIO_f_base64());
bio = BIO_push(b64, bio);
BIO_set_flags(bio, BIO_FLAGS_BASE64_NO_NL);
int l = BIO_read(bio, std::addressof(container[0]), static_cast<int>(inputLen));
assert(static_cast<std::size_t>(l) == container.size());
BIO_free_all(bio);
}
template <typename T, typename K>
T decodeInternal(const K& data)
{
T result;
rawBase64Decode(reinterpret_cast<const std::uint8_t*>(data.data()), data.size(), result);
return result;
}
template <typename T, typename K>
T encodeInternal(const K& data)
{
char* ptr = nullptr;
std::size_t len = 0;
rawBase64Encode(reinterpret_cast<const std::uint8_t*>(data.data()), data.size(), ptr, len);
T result(ptr, len);
free(ptr);
return result;
}
}
ByteBuffer encode(const ByteBuffer& data)
{
return encodeInternal<ByteBuffer, ByteBuffer>(data);
}
ByteBuffer encode(const std::string& str)
{
return encodeInternal<ByteBuffer, std::string>(str);
}
std::string encodeToString(const std::string& str)
{
return encodeInternal<std::string, std::string>(str);
}
ByteBuffer decode(const ByteBuffer& data)
{
return decodeInternal<ByteBuffer, ByteBuffer>(data);
}
ByteBuffer decode(const std::string& str)
{
return decodeInternal<ByteBuffer, std::string>(str);
}
std::string decodeToString(const ByteBuffer& data)
{
return decodeInternal<std::string, ByteBuffer>(data);
}
std::string decodeToString(const std::string& str)
{
return decodeInternal<std::string, std::string>(str);
}
} // namespace base64
} // namespace util
} // namespace redi
All of this has been "combined" from answers on SO and example on gist.
I would like to know if is a good design, if I can make any optimization (example: avoid copying the char*
into the ByteBuffer
, for instance) in encode, and, last but not least, if I have any memory leaks.
1 Answer 1
I would like to know if is a good design
A few points about your design from a c++ perspective:
1. Usage of free functions instead of classes
From the point of OOP view, you should rather use classes than free functions to wrap / encapsulate the C API:
namespace redi
{
namespace util
{
namespace base64
{
class Encoder {
public:
Encoder(const ByteBuffer& data);
Encoder(const std::string& str);
ByteBuffer operator(); // Do the encoding
std::string to_string();
};
class Decoder {
public:
Decoder(const ByteBuffer& data);
Decoder(const std::string& str);
ByteBuffer operator(); // Do the decoding
std::string to_string();
};
} // namespace base64
} // namespace util
} // namespace redi
2. Prefer class specializations over introducing a namespace
Using classes as mentioned above you should prefer specializing classes (may be interface implementations) rather introducing a new namespace
for a specific data category:
namespace redi
{
namespace util
{
class Base64Encoder;
// ^^^^^^
class Base64Decoder;
// ^^^^^^
} // namespace util
} // namespace redi
Same for the function naming:
void rawBase64Encode(const std::uint8_t* input, std::size_t inputLen, char*& output, std::size_t& outputLen);
and
void rawBase64Decode(const std::uint8_t* input, std::size_t inputLen, T& container);
will become
void Base64Encoder::rawEncode();
and
void Base64Decoder::rawDecode();
as (probably private
) class member functions.
3. Don't use using
or typedef
unnecessarily in published API's
The
using ByteBuffer = std::vector<std::uint8_t>;
obfuscates more, and leaves research efforts to API user, than it helps to save typing more code for the API provider.
Also I wouldn't provide that separately from the encoding / decoding features, but may be as a public
typedef
provided with classes.
4. In c++ encapsulate c-API handles
Your code all depends on a BIO* bio;
API handle, at least internally
As mentioned above rather encapsulate that into a class. May be even into a common base class used for both, encoding and decoding:
class Base64EncoderDecoderBase {
protected:
Bio* bio;
};
class Base64Encode : public Base64EncoderDecoderBase {
// ...
};
class Base64Decode : public Base64EncoderDecoderBase {
// ...
};
5. Are you sure you need to use OpenSSL?
As suggested in this answer are you sure you really need the OpenSS Bio
structure, or that could be left to a simpler solution?
6. You probably don't need to copy
As mentioned in my comment you probably don't need to have copies to interact with the OppenSSL library.
Using the data()
members of these container classes directly, may be helpful to avoid that (not sure for the Bio*
context though).
-
\$\begingroup\$ Thank you for your suggestions. I am going to apply your changes. \$\endgroup\$Andrei Damian– Andrei Damian2017年01月06日 12:05:22 +00:00Commented Jan 6, 2017 at 12:05
std::string
andByteBuffer
? \$\endgroup\$std::string
andstd::vector
both support thedata()
member function (with the current standard). So I don't believe any copying of the buffer is necessary. \$\endgroup\$