vigenere sometimes prints extra characters at the end of ciphertext in an unpredictable pattern

Question

Check50 yields 2 error messages (the rest is correct):

:( encrypts "a" as "a" using "a" as keyword output not valid ASCII text

:( encrypts "world!$?" as "xoqmd!$?" using "baz" as keyword output not valid ASCII text

when I execute the program, output looks always different without having changed anything in my code:

~/workspace/pset2/vigenere/ $ ./vigenere a

plaintext: a

ciphertext: a

~/workspace/pset2/vigenere/ $ ./vigenere a

plaintext: a

ciphertext: aa

~/workspace/pset2/vigenere/ $ ./vigenere a

plaintext: a ciphertext: aq%N

~/workspace/pset2/vigenere/ $ ./vigenere a

plaintext: a

ciphertext: a

~/workspace/pset2/vigenere/ $ ./vigenere a

plaintext: a

ciphertext: a?

Also, if i just hit enter when asked for plaintext, this happens:

~/workspace/pset2/vigenere/ $ ./vigenere a

plaintext:

ciphertext: 0m

~/workspace/pset2/vigenere/ $ ./vigenere a

plaintext:

ciphertext: 0S

Can someone please take a look at my code and help me find where this mistake is coming from?

So here is my code:

#include <cs50.h>
#include <ctype.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main(int argc, string argv[])
{
    // Check if command line argument is valid input
    if (argc != 2)
    {
        printf("Usage: ./vigenere key\n");
        return 1;
    }
    // Declare string key (keyword) as useres second command line argument
    string key = argv[1];
    // Check if all characters are alphabetical
    for (int a = 0, b = strlen(key); a < b; a++)
    {
        if (!isalpha(key[a]))
        {
            printf("Usage: ./vigenere key\n");
            return 1;
        }
    }

    // Get users Plaintext Input
    string plain = get_string("plaintext: ");

    // Define an array to store the characters of ciphertext
    char cipher[strlen(plain)];

    // Convert plaintext to ciphertext using alphabetical index A and a = 0, B    and b = 1... Z and Z = 25
    int non_alpha = 0; // Define an integer non_alpha to count non_alphabetical input
    int j = 0;
    for (int i = 0, n = strlen(plain); i < n; i++)
    {
        // Check if ith character is alphabetic
        if (isalpha(plain[i]))
        {
            // Determin which position of the keyword must be used
            j = (i - non_alpha) % strlen(key);
            if (isupper(key[j]))
            {
                // Check if ith character is uppercase then convert
                if (isupper(plain[i]))
                {
                    cipher[i] = (((plain[i] - 'A') + (key[j] - 'A')) % 26) + 'A';
                }
                // Check if ith character is lowercase then convert
                if (islower(plain[i]))
                {
                    cipher[i] = (((plain[i] - 'a') + (key[j] - 'A')) % 26) + 'a';
                }
            }
            else if (islower(key[j]))
            {
                // Check if ith character is uppercase then convert
                if (isupper(plain[i]))
                {
                    cipher[i] = (((plain[i] - 'A') + (key[j] - 'a')) % 26) + 'A';
                }
                // Check if ith character is lowercase then convert
                if (islower(plain[i]))
                {
                    cipher[i] = (((plain[i] - 'a') + (key[j] - 'a')) % 26) + 'a';
                }
            }
            else
            {
                return 1;
            }
        }
        // non alphabetical input = output
        else
        {
            cipher[i] = plain[i];
            non_alpha += 1;
        }
    }
    // Print ciphertext
    printf("ciphertext: %s\n", cipher);
    return 0;
}

Answer 1

The problem comes because cipher is declared a char array here char cipher[strlen(plain)];, but printed out using a string (aka char*) format here printf("ciphertext: %s\n", cipher);.

Remember, the thing that makes a string a string is the terminating null byte \0. And a string occupies one more byte than strlen to accomodate the terminator.

cipher is not null-terminated, therefore the printf result is printing everything in memory until it encounters a \0'.