Caesar Cipher not running as expected

Question

I'm playing around with Caesar Cipher and it doesn't seem to work.

This is my code:

class CaesarCipher {
  const CHARS = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789';
  protected $encrypt_sequence = array();
  protected $decrypt_sequence = array();

  public function __construct($seed = 1) {
      $total_chars = strlen(self::CHARS);
      $seed = $seed % $total_chars;
      for ($i = 0; $i < $total_chars; $i++) {
          $src = substr(self::CHARS, $i, 1);
          $dst = substr(self::CHARS, ($i + $seed) % $total_chars, 1);
          $this->encrypt_sequence[$src] = $dst;
          $this->decrypt_sequence[$dst] = $src;
      }
      $this->encrypt_sequence[' '] = ' ';
      $this->decrypt_sequence[' '] = ' ';
  }

  public function encrypt($value) {
      $value = strtoupper($value);
      return str_replace($this->encrypt_sequence, $this->decrypt_sequence, $value);
  }

  public function decrypt($value) {
      $value = strtoupper($value);
      return str_replace($this->decrypt_sequence, $this->encrypt_sequence, $value);
  }

  public function getEncryptSequence() {
      return $this->encrypt_sequence;
  }

  public function getDecryptSequence() {
      return $this->decrypt_sequence;
  }
}

Run it:

$seed = mt_rand(1, 35);
$cipher = new CaesarCipher($seed);
$source = 'THIS IS JUST A TEST WITH 123 NUMBERS';
$encrypted = $cipher->encrypt($source);
$decrypted = $cipher->decrypt($encrypted);

And this my output:

CAESAR CIPHER (seed=16)

Source:    THIS IS JUST A TEST WITH 123 NUMBERS
Encrypted: X12W 2W 3YWX U XYWX 02X1 567 7Y6VYVW   ENCRYPTED :)
Decrypted: DHIC IC JECD A DECD GIDH LMN NEMBEBC   DOES NOT MATCH SOURCE :(


Encryption: A>Q B>R C>S D>T E>U F>V G>W H>X I>Y J>Z K>0 L>1 M>2 N>3 O>4 P>5 Q>6 R>7 S>8 T>9 U>A V>B W>C X>D Y>E Z>F 0>G 1>H 2>I 3>J 4>K 5>L 6>M 7>N 8>O 9>P
Decryption: Q>A R>B S>C T>D U>E V>F W>G X>H Y>I Z>J 0>K 1>L 2>M 3>N 4>O 5>P 6>Q 7>R 8>S 9>T A>U B>V C>W D>X E>Y F>Z G>0 H>1 I>2 J>3 K>4 L>5 M>6 N>7 O>8 P>9

Can anyone give me a hint why it's not working?

I am not in PHP, but I am not sure if you are calling `encrypt` — ivan.mylyanyk, Apr 17 '15 at 14:34

jeroen · Answer 1 · 2015-04-17T14:38:19.033

3

The most obvious problem would be this section from the manual on str_replace:

Caution

Replacement order gotcha

Because str_replace() replaces left to right, it might replace a previously inserted value when doing multiple replacements. See also the examples in this document.

You are replacing some characters multiple times and that causes the result you get.

That applies to both the encrypting and the decrypting so you can't use str_replace() with arrays to do what you want to do.

A possible solution would be to encrypt (en decrypt...) your string character by character so that you only pass each character once.

edited Apr 17 '15 at 14:38

answered Apr 17 '15 at 14:34

jeroen

91,079
21
114
132

@Cojones Just added the last paragraph. You can access an individual string character as if your string is an array, so looping over it should be easy. – jeroen Apr 17 '15 at 14:39

score 2 · Accepted Answer · answered Apr 17 '15 at 14:43

Got it!

public function encrypt($value) {
    $value = strtoupper($value);
    return strtr($value, $this->encrypt_sequence);
}

public function decrypt($value) {
    $value = strtoupper($value);
    return strtr($value, $this->decrypt_sequence);
}

Will correctly en- and decrypt the value. Thanks to jeroen and GoogleHireMe :)

score 0 · Answer 3 · answered Apr 17 '15 at 14:47

That's a pretty inefficient way of doing it, each str_replace scans the whole string for each pair. Rather, why not scan the input string and calculate the replacement at encrypt/decrypt time. You asked for hints rather than solutions... but here's an example encrypt func (wouldn't need construct, although could store the construct-seed instead of passing it in as a parameter):

public function encrypt($value,$seed) {
    $n=strlen($value);
    $nchars=strlen(self::CHARS);
    for($i=0;$i<$n;$i++) {
        //This saves you doing the space-space replace, and allows 
        //  any chars not in your ciper to remain unmolested
        $pos=strpos(self::CHARS,$value[$i]);
        if ($pos>=0) $value[$i]=self::CHARS[($pos+$seed)%$nchars];
    }
}

That does look more efficient, however is it also more effiecient than `strtr`? — Cojones, Apr 20 '15 at 08:02

Caesar Cipher not running as expected

3 Answers3