Issue in hexdecode_string()

[michal-josef-spacek]

From https://bugzilla.redhat.com/show_bug.cgi?id=2045195

There is failing test:

make[5]: Entering directory '/builddir/build/BUILD/amanda-3.5.1/common-src'
PASS: amflock-test
PASS: event-test
PASS: amsemaphore-test
PASS: crc32-test
PASS: quoting-test
PASS: ipc-binary-test
FAIL: hexencode-test
PASS: fileheader-test
PASS: match-test

Error output is this:

 PASS test_encode (total: 0.000056)

(process:73137): GLib-ERROR **: 14:58:00.084: adding 18446744073709551615 to string would overflow
 PASS test_roundtrip (total: 0.000070)
 PASS test_roundtrip_rand (total: 0.001036)

Same error in debian build log: http://qa-logs.debian.net/2022/03/26/amanda_3.5.1-8_unstable.log

Exact error is in common-src/hexencode-test.c on line 78

 ...
 typedef struct {char *in; char *out; gboolean expect_err; } dec_vec;
 static gboolean
 test_decode(void)
 {
     static const dec_vec test_strs[] = {
         {"hi", "hi", FALSE},
         {"hi%21", "hi!", FALSE},
         {"%25", "%", FALSE},
         {"%2a", "*", FALSE},
         {"%2A", "*", FALSE},
         {"%0a", "\n", FALSE},
         {"%0A", "\n", FALSE},
         {"%0ahi%0a", "\nhi\n", FALSE},
         {"%", "", TRUE},
      // ^^^^^^^^^^^^^^^^
         {"%2", "", TRUE},
         {"h%", "", TRUE},
         {"%0h", "", TRUE},
         {"%h0", "", TRUE},
         {"%00", "", TRUE}
     };
 ...

From Petr Písař:
That's an exception from glib2 and glib2-devel was upgraded from 2.70.2-4.fc36 to 2.71.0-1.fc36. So the failure can be triggered by glib2.

However, there is also a bug in the testing function:

    ret = TRUE;
    for (i = 0; i < num; i++) {
        tmp = hexdecode_string(test_strs[i].in, &err);
->      if (!tmp || !g_str_equal(test_strs[i].out, tmp) ||
            (!!err != test_strs[i].expect_err)) {
            ret = FALSE;
            tu_dbg("decode failure:\n")
            tu_dbg("input:     \"%s\"\n", test_strs[i].in);
            tu_dbg("output:    \"%s\"\n", tmp? tmp : "(null)");
            tu_dbg("expected:  \"%s\"\n", test_strs[i].out);
            tu_dbg("error msg: %s\n", err? err->message : "(none)");
        }
        g_clear_error(&err);
        g_free(tmp);

    }
    return ret;

hexdecode_string() is documented to return NULL in case of an error. The test "%" is expected to fail, hence it should return NULL. If it does, then a condition (!tmp) in the testing function will falsely report a test failure.

But the most importat bug is in hexdecode_string():

    size_t orig_len, new_len, i;
    [...]
    new_len = orig_len = strlen(str);
    for (i = 0; i < orig_len; i++) {
        if (str[i] == '%') {
->          new_len -= 2;
        }
    }

In case of "%", strlen("%") sets new_len=1. (str[0] == '%') is true, new_len underflows to (size_t)-1.

[chassell]

common-src/fileheader-test -----------------------
 PASS test_roundtrip (total: 0.013622)
common-src/hexencode-test -----------------------
 PASS test_encode (total: 0.000078)
 PASS test_decode (total: 0.000084)
 PASS test_roundtrip (total: 0.000066)
 PASS test_roundtrip_rand (total: 0.001008)

This seems to work in 3_6.beta currently.

[michal-josef-spacek]

@chassell Ok, I am looking to code and there is no change in any code. Sic :-)
This is not fine from my perspective.
Still, we need to fix Fedora build of Amanda and I don't see how to fix it.

[chassell]

Phooey!! I have a big big change set up for 3_6.beta.

What's the Fedora problem? Try again if you need to. The build has been shifting a lot.

[michal-josef-spacek]

@chassell heh.. I understand, that you are doing changes. :-) Thanks for it.

Ad Fedora) We have amanda 3.5.1 and build is failing for long time. Failing on this test. See issue: https://bugzilla.redhat.com/show_bug.cgi?id=2045195 .

[michal-josef-spacek]

I reviewed this patch and it's working.