[tor-commits] [tor/master] base32_decode(): Return number of bytes written on success.

[asn at torproject.org]

commit a517daa56f5848d25ba79617a1a7b82ed2b0a7c0
Author: Nick Mathewson <nickm at torproject.org>
Date:   Thu Dec 20 08:36:25 2018 -0500

    base32_decode(): Return number of bytes written on success.
    
    This makes it consistent with base64_decode().
    
    Closes ticket 28913.
---
 changes/ticket28913           | 4 ++++
 src/feature/control/control.c | 3 ++-
 src/feature/rend/rendcache.c  | 3 +--
 src/lib/encoding/binascii.c   | 4 ++--
 src/test/test_crypto.c        | 8 ++++----
 src/test/test_util_format.c   | 4 ++--
 6 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/changes/ticket28913 b/changes/ticket28913
new file mode 100644
index 000000000..e09847464
--- /dev/null
+++ b/changes/ticket28913
@@ -0,0 +1,4 @@
+  o Code simplification and refactoring:
+    - Make the base32_decode() API return the number of bytes written,
+      for consistency with base64_decode().
+      Closes ticket 28913.
diff --git a/src/feature/control/control.c b/src/feature/control/control.c
index 7fae3b7a1..1e3db6337 100644
--- a/src/feature/control/control.c
+++ b/src/feature/control/control.c
@@ -4428,7 +4428,8 @@ handle_control_hsfetch(control_connection_t *conn, uint32_t len,
   } else if (strcmpstart(arg1, v2_str) == 0 &&
              rend_valid_descriptor_id(arg1 + v2_str_len) &&
              base32_decode(digest, sizeof(digest), arg1 + v2_str_len,
-                           REND_DESC_ID_V2_LEN_BASE32) == 0) {
+                           REND_DESC_ID_V2_LEN_BASE32) ==
+                REND_DESC_ID_V2_LEN_BASE32) {
     /* We have a well formed version 2 descriptor ID. Keep the decoded value
      * of the id. */
     desc_id = digest;
diff --git a/src/feature/rend/rendcache.c b/src/feature/rend/rendcache.c
index b851e7195..ecd85e4a5 100644
--- a/src/feature/rend/rendcache.c
+++ b/src/feature/rend/rendcache.c
@@ -854,7 +854,7 @@ rend_cache_store_v2_desc_as_client(const char *desc,
     *entry = NULL;
   }
   if (base32_decode(want_desc_id, sizeof(want_desc_id),
-                    desc_id_base32, strlen(desc_id_base32)) != 0) {
+                    desc_id_base32, strlen(desc_id_base32)) < 0) {
     log_warn(LD_BUG, "Couldn't decode base32 %s for descriptor id.",
              escaped_safe_str_client(desc_id_base32));
     goto err;
@@ -1005,4 +1005,3 @@ rend_cache_store_v2_desc_as_client(const char *desc,
   tor_free(intro_content);
   return retval;
 }
-
diff --git a/src/lib/encoding/binascii.c b/src/lib/encoding/binascii.c
index 067db075a..a7662658f 100644
--- a/src/lib/encoding/binascii.c
+++ b/src/lib/encoding/binascii.c
@@ -84,7 +84,7 @@ base32_encode(char *dest, size_t destlen, const char *src, size_t srclen)
 }
 
 /** Implements base32 decoding as in RFC 4648.
- * Returns 0 if successful, -1 otherwise.
+ * Return the number of bytes decoded if successful; -1 otherwise.
  */
 int
 base32_decode(char *dest, size_t destlen, const char *src, size_t srclen)
@@ -147,7 +147,7 @@ base32_decode(char *dest, size_t destlen, const char *src, size_t srclen)
   memset(tmp, 0, srclen); /* on the heap, this should be safe */
   tor_free(tmp);
   tmp = NULL;
-  return 0;
+  return i;
 }
 
 #define BASE64_OPENSSL_LINELEN 64
diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c
index 81d2fa6f3..e924cea42 100644
--- a/src/test/test_crypto.c
+++ b/src/test/test_crypto.c
@@ -1865,13 +1865,13 @@ test_crypto_base32_decode(void *arg)
   /* Encode and decode a random string. */
   base32_encode(encoded, 96 + 1, plain, 60);
   res = base32_decode(decoded, 60, encoded, 96);
-  tt_int_op(res,OP_EQ, 0);
+  tt_int_op(res, OP_EQ, 60);
   tt_mem_op(plain,OP_EQ, decoded, 60);
   /* Encode, uppercase, and decode a random string. */
   base32_encode(encoded, 96 + 1, plain, 60);
   tor_strupper(encoded);
   res = base32_decode(decoded, 60, encoded, 96);
-  tt_int_op(res,OP_EQ, 0);
+  tt_int_op(res, OP_EQ, 60);
   tt_mem_op(plain,OP_EQ, decoded, 60);
   /* Change encoded string and decode. */
   if (encoded[0] == 'A' || encoded[0] == 'a')
@@ -1879,12 +1879,12 @@ test_crypto_base32_decode(void *arg)
   else
     encoded[0] = 'A';
   res = base32_decode(decoded, 60, encoded, 96);
-  tt_int_op(res,OP_EQ, 0);
+  tt_int_op(res, OP_EQ, 60);
   tt_mem_op(plain,OP_NE, decoded, 60);
   /* Bad encodings. */
   encoded[0] = '!';
   res = base32_decode(decoded, 60, encoded, 96);
-  tt_int_op(0, OP_GT, res);
+  tt_int_op(res, OP_LT, 0);
 
  done:
   ;
diff --git a/src/test/test_util_format.c b/src/test/test_util_format.c
index fd57125b8..f91171dc2 100644
--- a/src/test/test_util_format.c
+++ b/src/test/test_util_format.c
@@ -346,7 +346,7 @@ test_util_format_base32_decode(void *arg)
     const char *src = "mjwgc2dcnrswqmjs";
 
     ret = base32_decode(dst, strlen(expected), src, strlen(src));
-    tt_int_op(ret, OP_EQ, 0);
+    tt_int_op(ret, OP_EQ, 10);
     tt_str_op(expected, OP_EQ, dst);
   }
 
@@ -357,7 +357,7 @@ test_util_format_base32_decode(void *arg)
     const char *src = "mjwgc2dcnrswq";
 
     ret = base32_decode(dst, strlen(expected), src, strlen(src));
-    tt_int_op(ret, OP_EQ, 0);
+    tt_int_op(ret, OP_EQ, 8);
     tt_mem_op(expected, OP_EQ, dst, strlen(expected));
   }