ENGINE-109: dumb bugs squished! fpr comparison should work correctly and wordlists appear with correct spacing.
authorKrista Grothoff <krista@pep-project.org>
Tue, 18 Oct 2016 20:57:54 +0200
changeset 1315d42e1163f3f3
parent 1314 d886cca6586a
child 1316 942c9943ae9d
ENGINE-109: dumb bugs squished! fpr comparison should work correctly and wordlists appear with correct spacing.
src/message_api.c
test/trustwords_test.cc
     1.1 --- a/src/message_api.c	Tue Oct 18 19:33:56 2016 +0200
     1.2 +++ b/src/message_api.c	Tue Oct 18 20:57:54 2016 +0200
     1.3 @@ -1907,6 +1907,25 @@
     1.4      assert(false);
     1.5  }
     1.6  
     1.7 +static bool _is_valid_hex(const char* hexstr) {
     1.8 +    if (!hexstr)
     1.9 +        return false;
    1.10 +    
    1.11 +    const char* curr = hexstr;
    1.12 +    char currchar;
    1.13 +    
    1.14 +    for (currchar = *curr; currchar != '\0'; currchar = *(++curr)) {
    1.15 +        if ((currchar >= '0' && currchar <= '9') ||
    1.16 +            (currchar >= 'a' && currchar <= 'f') ||
    1.17 +            (currchar >= 'A' && currchar <= 'F')) 
    1.18 +        {
    1.19 +            continue;
    1.20 +        }
    1.21 +        return false;
    1.22 +    }
    1.23 +    return true;        
    1.24 +}
    1.25 +
    1.26  // Returns, in comparison: 1 if fpr1 > fpr2, 0 if equal, -1 if fpr1 < fpr2
    1.27  static PEP_STATUS _compare_fprs(const char* fpr1, const char* fpr2, int* comparison) {
    1.28      
    1.29 @@ -1919,11 +1938,15 @@
    1.30      if (fpr1_len != _FULL_FINGERPRINT_LENGTH || fpr2_len != _FULL_FINGERPRINT_LENGTH)
    1.31          return PEP_TRUSTWORDS_FPR_WRONG_LENGTH;
    1.32      
    1.33 +    if (!_is_valid_hex(fpr1) || !_is_valid_hex(fpr2))
    1.34 +        return PEP_ILLEGAL_VALUE;
    1.35 +    
    1.36      const char* fpr1_curr = fpr1;
    1.37      const char* fpr2_curr = fpr2;
    1.38      
    1.39      char current;
    1.40  
    1.41 +    // Advance past leading zeros.
    1.42      for (current = *fpr1_curr; current != '0' && current != '\0'; current = *(++fpr1_curr), fpr1_len--);
    1.43      for (current = *fpr2_curr; current != '0' && current != '\0'; current = *(++fpr2_curr), fpr2_len--);
    1.44      
    1.45 @@ -1931,7 +1954,7 @@
    1.46          char digit1;
    1.47          char digit2;
    1.48  
    1.49 -        while (fpr1_curr) {
    1.50 +        while (fpr1_curr && *fpr1_curr != '\0') {
    1.51              digit1 = *fpr1_curr++;
    1.52              digit2 = *fpr2_curr++;
    1.53  
    1.54 @@ -1940,16 +1963,7 @@
    1.55                  digit1 -= _ASCII_LOWERCASE_OFFSET;
    1.56              if (digit2 >= 'a' && digit2 <= 'f')
    1.57                  digit2 -= _ASCII_LOWERCASE_OFFSET;
    1.58 -            
    1.59 -            if (!((digit1 >= '0' && digit1 <= '9') ||
    1.60 -                  (digit1 >= 'a' && digit1 <= 'f'))
    1.61 -                || 
    1.62 -                !((digit2 >= '0' && digit2 <= '9') ||
    1.63 -                     (digit2 >= 'a' && digit2 <= 'f'))) {
    1.64 -                return PEP_ILLEGAL_VALUE;
    1.65 -            }
    1.66 -            
    1.67 -            // Otherwise...
    1.68 +                        
    1.69              // We take advantage of the fact that 'a'-'f' are larger
    1.70              // integer values in the ASCII table than '0'-'9'.
    1.71              // This allows us to compare digits directly.
    1.72 @@ -2033,12 +2047,18 @@
    1.73              status = trustwords(session, source2, lang, &second_set, &second_wsize, max_words_per_id); 
    1.74              if (status != PEP_STATUS_OK)
    1.75                  goto error_release;
    1.76 +            break;
    1.77          default:
    1.78              return PEP_UNKNOWN_ERROR; // shouldn't be possible
    1.79      }
    1.80      
    1.81      size_t _wsize = first_wsize + second_wsize;
    1.82      
    1.83 +    bool needs_space = (first_set[first_wsize - 1] != ' ');
    1.84 +    
    1.85 +    if (needs_space)
    1.86 +        _wsize++;
    1.87 +    
    1.88      _retstr = calloc(1, _wsize + 1);
    1.89      
    1.90      size_t len = strlcpy(_retstr, first_set, _wsize);
    1.91 @@ -2046,6 +2066,13 @@
    1.92          status = PEP_UNKNOWN_ERROR;
    1.93          goto error_release;
    1.94      }
    1.95 +    if (needs_space) {
    1.96 +        strlcat(_retstr, " ", _wsize);
    1.97 +        if (len >= _wsize) {
    1.98 +            status = PEP_UNKNOWN_ERROR;
    1.99 +            goto error_release;
   1.100 +        }
   1.101 +    }
   1.102      strlcat(_retstr, second_set, _wsize);
   1.103      if (len >= _wsize){
   1.104          status = PEP_UNKNOWN_ERROR;
     2.1 --- a/test/trustwords_test.cc	Tue Oct 18 19:33:56 2016 +0200
     2.2 +++ b/test/trustwords_test.cc	Tue Oct 18 20:57:54 2016 +0200
     2.3 @@ -38,6 +38,8 @@
     2.4      size_t wsize2;
     2.5      size_t wsize_full;
     2.6      
     2.7 +    cout << "\nTest 1: fpr1 > fpr2, short" << endl;
     2.8 +    
     2.9      cout << "\nfinding German trustwords for " << fingerprint1 << "...\n";
    2.10      trustwords(session, fingerprint1.c_str(), "de", &words1, &wsize1, 5);
    2.11      assert(words1);
    2.12 @@ -57,10 +59,143 @@
    2.13      pEp_free(words1);
    2.14      pEp_free(words2);
    2.15      pEp_free(full_wordlist);
    2.16 +
    2.17 +    cout << "\nTest 2: fpr1 == fpr1, short" << endl;
    2.18 +    
    2.19 +    cout << "\nfinding French trustwords for " << fingerprint2 << "...\n";
    2.20 +    trustwords(session, fingerprint1.c_str(), "fr", &words1, &wsize1, 5);
    2.21 +    assert(words1);
    2.22 +    cout << words1 << "\n";
    2.23 +        
    2.24 +    cout << "\nfinding French trustwords for " << identity2->address << " and " << identity2->address << "...\n";
    2.25 +    get_trustwords(session, identity2, identity2, "fr", &full_wordlist, &wsize_full, false);
    2.26 +    assert(full_wordlist);
    2.27 +    cout << full_wordlist << "\n";
    2.28 +
    2.29 +    pEp_free(words1);
    2.30 +    pEp_free(full_wordlist);
    2.31 +
    2.32 +    cout << "\nTest 3: fpr1 < fpr2, long" << endl;
    2.33 +    
    2.34 +    cout << "\nfinding English trustwords for " << fingerprint2 << "...\n";
    2.35 +    trustwords(session, fingerprint2.c_str(), "en", &words1, &wsize1, 0);
    2.36 +    assert(words1);
    2.37 +    cout << words1 << "\n";
    2.38 +    
    2.39 +    cout << "\nfinding English trustwords for " << fingerprint1 << "...\n";
    2.40 +    trustwords(session, fingerprint1.c_str(), "en", &words2, &wsize2, 0);
    2.41 +    assert(words2);
    2.42 +    cout << words2 << "\n";
    2.43 +    
    2.44 +    cout << "\nfinding English trustwords for " << identity2->address << " and " << identity2->address << "...\n";
    2.45 +    get_trustwords(session, identity2, identity1, "en", &full_wordlist, &wsize_full, true);
    2.46 +    assert(full_wordlist);
    2.47 +    cout << full_wordlist << "\n";
    2.48 +    
    2.49 +    pEp_free(words1);
    2.50 +    pEp_free(words2);
    2.51 +    pEp_free(full_wordlist);
    2.52 +    
    2.53 +    
    2.54 +    cout << "\nTest 4: fpr1 < fpr2, leading zeros (fpr1 has more), long" << endl;
    2.55 +    
    2.56 +    pEp_identity* identity3 = new_identity(
    2.57 +        "nobody@kgrothoff.org",
    2.58 +        "000F932086185C15917B72D30571AFBCA5493553",
    2.59 +        "blargh",
    2.60 +        "Krista Grothoff");
    2.61 +    
    2.62 +    pEp_identity* identity4 = new_identity(
    2.63 +        "nobody2@kgrothoff.org",
    2.64 +        "001F932086185C15917B72D30571AFBCA5493553",
    2.65 +        "blargh",
    2.66 +        "Krista Grothoff");
    2.67 +    
    2.68 +    pEp_identity* identity5 = new_identity(
    2.69 +        "nobody3@kgrothoff.org",
    2.70 +        "001F732086185C15917B72D30571AFBCA5493553",
    2.71 +        "blargh",
    2.72 +        "Krista Grothoff");
    2.73 +
    2.74 +    string fingerprint3 = identity3->fpr;
    2.75 +    string fingerprint4 = identity4->fpr;
    2.76 +    string fingerprint5 = identity5->fpr; 
    2.77 +        
    2.78 +    cout << "\nfinding Catalan trustwords for " << fingerprint3 << "...\n";
    2.79 +    trustwords(session, fingerprint3.c_str(), "ca", &words1, &wsize1, 0);
    2.80 +    assert(words1);
    2.81 +    cout << words1 << "\n";
    2.82 +    
    2.83 +    cout << "\nfinding Catalan trustwords for " << fingerprint4 << "...\n";
    2.84 +    trustwords(session, fingerprint4.c_str(), "ca", &words2, &wsize2, 0);
    2.85 +    assert(words2);
    2.86 +    cout << words2 << "\n";
    2.87 +    
    2.88 +    cout << "\nfinding Catalan trustwords for " << identity3->address << " and " << identity4->address << "...\n";
    2.89 +    get_trustwords(session, identity3, identity4, "ca", &full_wordlist, &wsize_full, true);
    2.90 +    assert(full_wordlist);
    2.91 +    cout << full_wordlist << "\n";
    2.92 +
    2.93 +    pEp_free(words1);
    2.94 +    pEp_free(words2);
    2.95 +    pEp_free(full_wordlist);
    2.96 +
    2.97 +    cout << "\nTest 5: fpr1 > fpr2, leading zeros (same number), interior digit difference, short" << endl;
    2.98 +    
    2.99 +    cout << "\nfinding Turkish trustwords for " << fingerprint4 << "...\n";
   2.100 +    trustwords(session, fingerprint4.c_str(), "tr", &words1, &wsize1, 5);
   2.101 +    assert(words1);
   2.102 +    cout << words1 << "\n";
   2.103 +    
   2.104 +    cout << "\nfinding Turkish trustwords for " << fingerprint5 << "...\n";
   2.105 +    trustwords(session, fingerprint5.c_str(), "tr", &words2, &wsize2, 5);
   2.106 +    assert(words2);
   2.107 +    cout << words2 << "\n";
   2.108 +    
   2.109 +    cout << "\nfinding Turkish trustwords for " << identity4->address << " and " << identity5->address << "...\n";
   2.110 +    get_trustwords(session, identity4, identity5, "tr", &full_wordlist, &wsize_full, false);
   2.111 +    assert(full_wordlist);
   2.112 +    cout << full_wordlist << "\n";
   2.113 +    
   2.114 +    pEp_free(words1);
   2.115 +    pEp_free(words2);
   2.116 +    pEp_free(full_wordlist);
   2.117 +
   2.118 +    cout << "\nTest 6: fpr2 is too short" << endl;
   2.119 +    
   2.120 +    pEp_identity* identity6 = new_identity(
   2.121 +        "nobody4@kgrothoff.org",
   2.122 +        "01F932086185C15917B72D30571AFBCA5493553",
   2.123 +        "blargh",
   2.124 +        "Krista Grothoff");
   2.125 +    
   2.126 +    cout << "\nfinding Turkish trustwords for " << identity5->address << " and " << identity6->address << "...\n";
   2.127 +    PEP_STATUS status6 = get_trustwords(session, identity5, identity6, "tr", &full_wordlist, &wsize_full, false);
   2.128 +    assert(status6 == PEP_TRUSTWORDS_FPR_WRONG_LENGTH);
   2.129 +    cout << "Bad fpr length correctly recognised." << "\n";
   2.130 +    
   2.131 +    pEp_identity* identity7 = new_identity(
   2.132 +        "nobody5@kgrothoff.org",
   2.133 +        "F01X932086185C15917B72D30571AFBCA5493553",
   2.134 +        "blargh",
   2.135 +        "Krista Grothoff");
   2.136 +
   2.137 +    cout << "\nTest 7: fpr2 has a non-hex character" << endl;
   2.138 +    
   2.139 +    cout << "\nfinding Turkish trustwords for " << identity1->address << " and " << identity7->address << "...\n";
   2.140 +    PEP_STATUS status7 = get_trustwords(session, identity1, identity7, "tr", &full_wordlist, &wsize_full, true);
   2.141 +    assert(status7 == PEP_ILLEGAL_VALUE);
   2.142 +    cout << "Illegal digit value correctly recognised." << "\n";
   2.143      
   2.144      
   2.145      free(identity1);
   2.146      free(identity2);
   2.147 +    free(identity3);
   2.148 +    free(identity4);
   2.149 +    free(identity5);
   2.150 +    free(identity6);
   2.151 +    free(identity7);
   2.152 +    
   2.153      cout << "calling release()\n";
   2.154      release(session);
   2.155      return 0;