Missing punctuation from C++ hex2bin

Question

While trying to duplicate PHP's bin2hex($s) and pack('H*',$s) (aka hex2bin($s) in PHP 5.4.3+) in GCC/Linux C++, I seem to have it figured out except that it's dropping punctuation for some strange reason. Can you figure out what I might be doing wrong in the hex2bin() function? I compared PHP's bin2hex() with mine and it appears to be working there properly, so the problem is in hex2bin().

#include <strings.h>
#include <string>
#include <stdio.h>
#include <stdlib.h>
using namespace std;

string bin2hex(string s) {
  int nLen = s.length();
  string sOut;
  char cBuff[2];
  for (int i = 0; i < nLen; i++) {
    sprintf(cBuff,"%.2x",s[i]);
    sOut.append(cBuff);
    cBuff[0] = '\0';
  }
  return sOut;
}

string hex2bin(string s) {
  int nLen = s.length();
  string sOut;
  char cBuff1[2];
  char cBuff2[2];
  char cBuff[1];
  int n,n1,n2;
  for (int i = 0; i <= nLen; i+=2) {
    sprintf(cBuff1,"%c",s[i]);
    sprintf(cBuff2,"%c",s[i+1]);
    n1 = atoi(cBuff1);
    n2 = atoi(cBuff2);
    n = (n1 * 16) + n2;
    sprintf(cBuff,"%c",n);
    sOut.append(cBuff);
    cBuff[0] = '\0';
    cBuff1[0] = '\0';
    cBuff2[0] = '\0';
  }
  return sOut;
}

int main() {
  string s;
  string sResult;  
  s = "This is a 123 test.";
  sResult = bin2hex(s);
  printf("ENCODED: %s\n",sResult.c_str());
  sResult = hex2bin(sResult);
  printf("UNENCODED: %s\n",sResult.c_str());
  return 1;
}

This emits:

ENCODED: 5468697320697320612031323320746573742e
UNENCODED: This is a 123 test

Answer 1

Okay, sleeves rolled up: let's look at C++ version:

Live on Coliru

• Don't use C strings unless you need to (sprintf to build a two-char string is not... very efficient)
• Use iostreams to encode/decode the hex digits (std::hex)
• The hex2bin could optimized, but I went for "simpler"
• I added a modicum of input sanitizing on hex2bin

#include <string>
#include <sstream>
#include <iomanip>

std::string bin2hex(std::string const &s) {
    std::ostringstream oss;

    for (unsigned char ch : s)
        oss << std::hex << std::setw(2) << std::setfill('0') << (int) ch;

    return oss.str();
}

#include <cassert>
std::string hex2bin(std::string const& s) {
    assert(s.length() % 2 == 0);

    std::string sOut;
    sOut.reserve(s.length()/2);

    std::string extract;
    for (std::string::const_iterator pos = s.begin(); pos<s.end(); pos += 2)
    {
        extract.assign(pos, pos+2);
        sOut.push_back(std::stoi(extract, nullptr, 16));
    }
    return sOut;
}

#include <iostream>
int main() {
    std::cout << "ENCODED: " << bin2hex("This is a 123 test.")          << "\n";
    std::cout << "DECODED: " << hex2bin(bin2hex("This is a 123 test.")) << "\n";
}

Output:

ENCODED: 5468697320697320612031323320746573742e
DECODED: This is a 123 test.

Answer 2

With all but the period '.' you just went lucky: the hex digits didn't use an actual hexadecimal value. However, for the period you got 2e but you tried to decode the e using atoi("e"), roughly: that won't work as atoi() requires a decimal value. You could use strtol(str, 0, 16) instead to decode the hexadecimal value.

Note that you have a few buffer overruns when you are using sprintf(): this function writes a terminating null character. In general, you are much better off to snprintf() to avoid buffer overruns. Also, in your decoding routine you access values beyond the end of your string (you use i <= nLen with nLen = s.length() and then access s[i] and s[i+1]). Of course, the code is far too complex:

#include <string>
#include <sstream>
#include <iostream>
#include <iomanip>

std::string bin2hex(std::string s) {
    std::ostringstream out;
    out << std::hex << std::setfill('0');
    for (char c: s) {
        out << std::setw(2) << int(c);
    }
    return out.str();
}

std::string hex2bin(std::string s) {
    std::string rc;
    int nLen = s.length();
    int tmp;
    for (int i(0); i + 1 < nLen; i += 2) {
        if (std::istringstream(s.substr(i, 2)) >> std::hex >> tmp) {
            rc.push_back(tmp);
        }
    }
    return rc;
}

int main() {
  std::string s;
  std::string sResult;  
  s = "This is a 123 test.";
  sResult = bin2hex(s);
  std::cout << "ENCRYPTED: " << sResult << '\n';
  sResult = hex2bin(sResult);
  std::cout << "UNENCRYPTED: " << sResult << '\n';
  return 1;
}

Answer 3

Your code does not convert hexadecimal digits correctly because atoi can only handle decimal digits. Try this

sprintf(cBuff1,"%c",s[i]);
sprintf(cBuff2,"%c",s[i+1]);
n1 = strtoul(cBuff1, 0, 16);
n2 = strtoul(cBuff2, 0, 16);

Also your for loop should be

for (int i = 0; i < nLen; i+=2) {

Answer 4

n1 = atoi(cBuff1);
n2 = atoi(cBuff2);
n = (n1 * 16) + n2;

if cbuff1 is, say, "a", then this won't work, since a is not a digit. It works fine for digits that are '0-9', but not 'a-f'.

You will need to translate non-digits to numeric values.

There are quite a few ways to convert a hex value string to a byte. I think this is pretty decent:

int hexchar(char c)
{
   if (c >= '0' && c <= '9') return c - '0';
   // if you need to support upper-case hex:
   // c = tolower(c); 
   if (c >= 'a' && c <= 'f') return c - 'a' + 10; 
   // If we get here, panic
   cout << "Error, invalid hex digit:" << c << endl;
   return -1;
}

int hexbyte(string s)
{
    for(i = 0; i < s.length(); i+=2)
    {
       char c = hexbyte(s[i]);
       c <<= 4;
       c += hexbyte(s[i+1];
       cout << c;
    }
}

Answer 5

Try these trivial routines, good for C and C ++

/*------------------------------------------+
|       bin2hex     bin2hex     bin2hex     |
+------------------------------------------*/
static  char *bin2hex(unsigned char *s, long L)
{
    static  char hex[2048];
    long i,l=0;
    for (i=0; i<L; i++) l+=sprintf(&hex[l], "%02x", 0xFF & (*(s+i)));
    hex[l]=0;
    return hex;
}
/*------------------------------------------+
|       hex2bin     hex2bin     hex2bin     |
+------------------------------------------*/
static  char *hex2bin( char *s)
{
    static  char bin[2048];
    unsigned int i,e,l=0,L=strlen(s);
    for (i=0; i<L; i+=2) { sscanf(s+i, "%02x",&e); bin[l++]=(char)e; }
    bin[l]=0;
    return bin;
}