#6841 closed enhancement (fixed)
[with patch, positive review] the shift cryptosystem
Reported by: | mvngu | Owned by: | somebody |
---|---|---|---|
Priority: | major | Milestone: | sage-4.1.2 |
Component: | cryptography | Keywords: | |
Cc: | rbeezer | Merged in: | Sage 4.1.2.alpha4 |
Authors: | Minh Van Nguyen | Reviewers: | Rob Beezer |
Report Upstream: | Work issues: | ||
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
Implement the shift cryptosystem for educational purposes. This adds to the classical cryptosystems already implemented.
Attachments (1)
Change History (12)
comment:1 Changed 13 years ago by
comment:2 Changed 13 years ago by
comment:3 Changed 13 years ago by
- Summary changed from the shift cryptosystem to [with patch, needs review] the shift cryptosystem
comment:4 Changed 13 years ago by
- Cc rbeezer added
comment:5 Changed 13 years ago by
- Summary changed from [with patch, needs review] the shift cryptosystem to [with patch, needs work] the shift cryptosystem
Here are some of rbeezer's comments on sanity checking the key value:
23:24 < mvngu-6971> rbeezer: Going back to shift cipher: The callable of ShiftCryptosystem (i.e. __call__) takes care of converting any value of key to 0 <= key < alphabet_size. 23:24 < mvngu-6971> rbeezer: Perhaps I should make that clear in the documentation. 23:27 < rbeezer> mvngu-6971: yes, I see it being Mod'ed, but I think you should disallow bad keys on input 23:27 < rbeezer> S(29) for "regular" alphabet just churns along, I'd prefer an error 23:27 < mvngu-6971> rbeezer: New patch coming up... in a hour or so. I'm still doing some release manage work... 23:28 < mvngu-6971> rbeezer: Thank you *very* much for the valuable feedback! 23:28 < rbeezer> For example with binary strings, my S(5) could have thrown an error, and I would have had to think about it right then, instead of looking for a positional shift of 5. ;-) 23:29 < rbeezer> mvngu-6971: no rush, probably be tomorrow night before I get to it 23:29 < mvngu-6971> rbeezer: Cool.
comment:6 Changed 13 years ago by
- Summary changed from [with patch, needs work] the shift cryptosystem to [with patch, needs review] the shift cryptosystem
The updated patch addresses rbeezer's comments on sanity checking the value of the encryption/decryption key. The sanity checking is done in the callable __call__()
of the class ShiftCryptosystem
. I have added doctests to that callable. Currently, methods beginning with an underscore "_
" don't show up in the reference manual, so I have also added the doctests for checking key value to the docstring for the class ShiftCryptosystem
. In this way, reading the reference manual for the shift cryptosystem should make it clear (hopefully) that the value of an encryption/decryption key must lie within the range of acceptable values, i.e. must be within the key space.
comment:7 follow-up: ↓ 9 Changed 13 years ago by
- Reviewers set to Rob Beezer
Builds and runs on 4.1.2.alpha2. Passes tests, docs build without warnings. Great documentation.
Would you want to put similar input protections on inverse_key()
? Your call.
Positive review, either way.
comment:8 Changed 13 years ago by
- Summary changed from [with patch, needs review] the shift cryptosystem to [with patch, positive review] the shift cryptosystem
comment:9 in reply to: ↑ 7 Changed 13 years ago by
comment:10 Changed 13 years ago by
- Merged in set to Sage 4.1.2.alpha3
- Resolution set to fixed
- Status changed from new to closed
comment:11 Changed 13 years ago by
- Merged in changed from Sage 4.1.2.alpha3 to Sage 4.1.2.alpha4
There is no 4.1.2.alpha3. Sage 4.1.2.alpha3 was William Stein's release for working on making the notebook a standalone package.
The patch
trac_6841-shift-cipher.patch
implements the shift cryptosystem. It also adds some doctests and documentation to existing crypto modules.