Skip to content

Caesar cipher produces non-alphabetic garbage output for negative shifts #7506

Description

@herley-shaori

Description

Caesar.encode and Caesar.decode in src/main/java/com/thealgorithms/ciphers/Caesar.java produce incorrect, non-alphabetic output when called with a negative shift value. The encode(decode(x)) == x round-trip is also broken for negative shifts in some cases.

Root cause

normalizeShift computes the shift as:

private static char normalizeShift(final int shift) {
    return (char) (shift % 26);
}

In Java, % preserves the sign of the dividend, so for a negative shift that is not an exact multiple of 26 (e.g. -1, -2, -5), shift % 26 is itself negative (e.g. -1 % 26 == -1). Casting that negative int directly to char does not produce a small negative number — char is an unsigned 16-bit type in Java, so (char) -1 wraps around to 65535 (0xFFFF).

That huge value is then added to the letter's char code (current += shiftChar), overflows the 16-bit char range, and truncates back into a seemingly "normal" but wrong character. The subsequent bounds check (current > 'Z' / current < 'A') does not catch this because the truncated value can land anywhere in the char range, not just outside the alphabet.

Steps to reproduce

Caesar caesar = new Caesar();
System.out.println(caesar.encode("A", -1));

Expected: "Z" (a Caesar shift of -1 should wrap 'A' back to 'Z', just like a shift of +25 would).

Actual: "@" (ASCII 64) — a non-alphabetic character is silently emitted.

Verified locally by extracting the exact normalizeShift/encode/decode logic and running it:

shift=-1   encode("A") = "@"   (char code 64, not a letter)
shift=-2   encode("A") = "?"   (char code 63)
shift=-5   encode("A") = "<"   (char code 60)
shift=-25  encode("A") = "("   (char code 40)

The round-trip is also broken for edge cases:

String encoded = caesar.encode("A", -1); // "@"
String decoded = caesar.decode(encoded, -1); // "@" (not "A")

Why this matters

  • This isn't just an edge case — any negative shift that isn't an exact multiple of 26 triggers it (e.g. -1, -2, ..., -25, -27, etc.), and negative shifts are a normal, documented way to use a Caesar cipher (shifting backward).
  • The bug is completely uncovered by the existing tests: CaesarTest.java only exercises shift = 5 and the bruteforce() method (which internally only iterates shifts 0..26), so no negative shift is ever tested.
  • The failure mode is silent — it doesn't throw an exception, it just returns wrong/corrupted data, which is worse for a class users might trust for correctness.

Suggested fix

Normalize the shift as a plain int using a formula that is always non-negative, instead of casting a possibly-negative int to char:

private static int normalizeShift(final int shift) {
    return ((shift % 26) + 26) % 26;
}

and use int shiftChar in encode/decode instead of char shiftChar. Happy to submit a PR with this fix plus a test case covering negative shifts if that's welcome.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions