Vigenère cipher implementation

Question

I have to implement a variant of the Vigenère cipher. I got the encryption part without issues, but I have a bug in the decryption code and I don't understand what I'm doing wrong.

The requirements are:

• the key can only contain A - Z (uppercase)

• code values for the key characters are 0 for A, 1 for B, ..., and 25 for Z

• do not encode a character if the code is < 32 (preserve control characters)

• encrypted character code = original character code + key character code

• the final encrypted character must be between 32 and 126, exclusively so if the final encrypted character > 126 it must be brought back into the 32 - 126 range by adding 32 to the value and then subtracting 126

The encryption code:

// it works ok
// I have tested it with some provided strings and the results are as expected

public String encrypt(String plainText)
{
    StringBuilder sb = new StringBuilder();
    for (int i = 0; i < plainText.length(); i++) {
        char c = plainText.charAt(i);
        if (c >= 32) {
            int keyCharValue = theKey.charAt(i % theKey.length()) - 'A';
            c += keyCharValue;
            if (c > 126) {
                c = (char) (c + 32 - 126);
            }
        }
        sb.append(c);
    }
    return sb.toString();
}

The decryption code:

// there probably is an off-by-one error somewhere
// everything is decrypted ok, except '~' which gets decrypted to ' ' (space)

public String decrypt(String cipherText)
{
    StringBuilder sb = new StringBuilder();
    for (int i = 0; i < cipherText.length(); i++) {
        char c = cipherText.charAt(i);
        if (c >= 32) {
            int keyCharValue = theKey.charAt(i % theKey.length()) - 'A';
            c -= keyCharValue;
            if (c < 32) {
                c = (char) (c + 126 - 32);
            }
        }
        sb.append(c);
    }
    return sb.toString();
}

Example (with key ABCDEFGHIJKLMNOPQRSTUVWXYZ):

• original ~~~~~~~~~~~~~~~~~~~~~~~~~~

• encrypted ~!"#$%&'()*+,-./0123456789

• decrypted ~ ('~' followed by spaces)

EDIT:

Here is the code I use for testing (it tests every character from 0 to 126 repeated as a string):

public static void main(String[] args) {
    int passed = 0;
    int failed = 0;
    String key = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
    for (int c = 0; c <= 126; c++) {
        StringBuilder sbString = new StringBuilder();
        for (int i = 0; i <= 25; i++) {
            sbString.append((char) c);
        }
        String original = sbString.toString();
        Cipher cipher = Cipher(key);
        String encrypted = cipher.encrypt(original);
        String decrypted = cipher.decrypt(encrypted);
        if (!original.equals(decrypted)) {
            failed++;
            System.out.println("--FAILED--");
            System.out.println(original);
            System.out.println(encrypted);
            System.out.println(decrypted);
        } else {
            passed++;
        }
    }
    int tests = passed + failed;
    System.out.println(tests + " tests");
    System.out.println("passed: " + passed);
    System.out.println("failed: " + failed);
}

Answer 1

I believe the If(c < 32) in the decryption needs to be If (c <= 32).

Reasoning: if you take the case of Char(126) or '~' then add one to it in the encryption you get 127, which goes through the encryption transform and becomes 33.

When decrypting that you get 33 minus that same 1 which leaves you with 32, which won't trigger the special decryption case. By including 32 in that statement it will trigger the special decryption and change the 32 (" ") to 126 ("~")

You are correct it is an off by one error, but it is kinda subtle

EDIT: there is a collision error because char(32) and char(126) are hashing to the same value. In my previous example that value would be 33, the equation needs to be changed such that Char(126) will hash to 32.

Changing the c = (char) (c + 32 - 126); to c = (char) (c + 32 - 127); should free up the extra space to prevent the collision from happening. THe decrypt will also have to be changed from c = (char) (c + 126 - 32); to c = (char) (c + 127 - 32);

And someone posted that in my comments.