Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Citrix CTX1 Encoding/Decoding #428

Merged
merged 3 commits into from
Nov 21, 2018
Merged

Citrix CTX1 Encoding/Decoding #428

merged 3 commits into from
Nov 21, 2018

Conversation

bwhitn
Copy link
Contributor

@bwhitn bwhitn commented Nov 20, 2018

This should add the requested operation for Citrix CTX1 encoded passwords in issue #413.

@n1474335 n1474335 merged commit c378bcb into gchq:master Nov 21, 2018
@n1474335
Copy link
Member

Great work, thanks very much!

Just a few points to bear in mind in future:

  • We prefer using Array.from to Buffer.from. Buffer is a Node type that doesn't exist in browsers. Babel will include a polyfill so that it works, but we'd rather use native JS code where possible. Array.from is ES6, so Babel will probably include a polyfill for it anyway, but we'd still rather use this syntax so that a polyfill is not required in future when support is broader.
  • Make sure the operation class is named correctly. You had the operation named 'Citrix CTX1 Encode' and the class named EncodeCitrixCTX1. This seems like a small thing to worry about, but we do sometimes rely on the operation object having a predictable name so it's important we get it right. If you use the npm run newop helper script to create new operations, the names will all be generated for you correctly.
  • When carrying out a validity check on the data, make sure you throw an OperationError when the check fails, rather than just returning a blank output. This makes it more clear to the user what has gone wrong and allows CyberChef to fail gracefully and predictably.

Nice work on the algorithm for this, I'm sure people will find it useful.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants