search

bmwinstead / enigma-project

Other
1 stars 1 forks source link

Bug: GUI: Inconsistent Decryption with the Special Characters Test Case #39

Closed ikleyjl closed 10 years ago

ikleyjl commented 10 years ago
  1. Run the GUI
  2. Open the prepared test file, TestCasePlaintextWithCasesSpacesSpecialPunctuation.txt
  3. Cut and paste the contents of the text file into the bulk encryption field.
  4. Set the Rotor positions to "AAB"
  5. Set the output space option to "4"
  6. Click "Encrypt"
  7. Observe output: " ZCNX MCFW JBEG NDNF TPFV KQGF MGKH IW"
  8. Click "Default Configuration"
  9. Set the rotor positions to "AAB"
  10. Set the output space option to "4"
  11. Click "Browse"
  12. Navigate to "TestCasePlaintextWithCasesSpacesSpecialPunctuation.txt" and select.
  13. Click "Encrypt"
  14. Observe output: "UJHW BRAX KCCX NXEB TYDB OFOO OXUG XMYK R"

Expected result: Output should be the same regardless of whether the text is cut and pasted or uploaded.

EOhlmacher commented 10 years ago

Interesting thing about this bug.. both examples appear to be wrong. :) The correct encryption should be "ZCNX MCFW JBEG NDNF TPFV KQGF MGKH I". I fixed the case in line 7 where there is an extra W, but can't easily fix the extra character in the file input case without causing other errors. The problem is, something about moving the text from the file to the list and then to the string is converting the final special character to "e&". I can strip special characters, but that leaves the e, which outputs an incorrect result. I put in a hack for now and noted it as such.

ikleyjl commented 10 years ago

Stripping characters on input is perfectly valid! The only problem with that method is that we can't be positive what all special characters will get themselves translated into, but to be completely honest, I'm not all that worried about how accurately we encrypt heart-smilie face-spade, anyway?

EOhlmacher commented 10 years ago

The issue I see is - the only way I could strip "heart" after the file->list->string conversion changed it to "e&" is to strip "e&", which might strip a valid e in a case such as "fame&fortune". It would then be encrypted as if it were "famfortune." So there is still a bug. If I could figure out how to strip it before it got through all that conversion, or not convert it to "e&" at all, I could remove the hacky fix.

wadolph commented 10 years ago

I put together a decent character rejection method (in my opinion) in TextParser. Look under addWord() and see if that helps.

bmwinstead commented 10 years ago

By "heart" do you mean an ASCII heart character? Why would anyone input a text file that had one of those... In any case... this should be relatively easy to fix by changing the file reading method. Absolute worst case we can parse the text file character by character and disallow all non-alphanumeric characters.

bmwinstead commented 10 years ago

Also, Ellen did you reach any conclusions about why the two methods provide different encryption results in the first place? I'm going to look at this tomorrow.

EOhlmacher commented 10 years ago

They were different because the swing text box and file import handled the heart differently, but neither handled it well.

The text box converted the heart to something that included a single letter somewhere after the plaintext, which resulted in the extra W Jessica found. I was able to eliminate that character immediately after the text is pulled in.

The file import converts it to literally e& and it is like that immediately after import. It also added special characters to the beginning of the text because (I believe) the heart character is UTF8, not ASCII. We could possibly just reject UTF8 txt docs and update our User's guide.

EOhlmacher commented 10 years ago

Question: Do we want to close this one as "Known Shipable" with the hack fix, or do we want to spend more time trying to resolve this?

ikleyjl commented 10 years ago

I notice the heart fix was removed, which is a good idea - that has the potential to break more than it fixes. Otherwise, I agree with marking this as "Known Shippable" and closing it out.

bmwinstead commented 10 years ago

Close it On Dec 13, 2013 3:24 PM, "ikleyjl" notifications@github.com wrote:

I notice the heart fix was removed, which is a good idea - that has the potential to break more than it fixes. Otherwise, I agree with marking this as "Known Shippable" and closing it out.

— Reply to this email directly or view it on GitHubhttps://github.com/bmwinstead/enigma-project/issues/39#issuecomment-30540774 .