Added Operations: Octal #92

mattnotmitt · 2017-03-14T21:30:24Z

To Octal
From Octal
Also, if anyone can figure out how to do highlighting for this, that'd be amazing. I was totally stumped by that on this one due to octal coming out at different lengths.

- To Octal - From Octal

m2951413 · 2017-03-19T19:37:47Z

I know its only a small change, unfortunately we can't merge this just yet as we don't want to loose support for non ES6 browsers yet.

That said... I know @n1474335 is currently working on #10 / #5 which will mean that we can use babel to convert back. Once that is sorted we can merge this!

Whilst waiting for that, I don't know if you saw, but tlwr has introduced a lightweight test suite / #84. Adding a test or so around this could be useful to ensure we don't accidentally break compatibility with anything you've got planned with this in the future

mattnotmitt · 2017-03-19T20:26:19Z

@m2951413 Sorry, don't know much at all about different ECMAScript versions - is the issue the arrow functions? If so I can just replace them, they're not necessary and just because it's how I would normally do it.

m2951413 · 2017-03-19T23:51:52Z

Yes that is correct, the arrow function/fat arrow was my concern.

n1474335

Thanks very much for another great contribution. As @m2951413 has said, we can't merge this quite yet due to the ES6 support, however I wouldn't change it - let's use it as the first test case for Babel once I've committed my build process changes.

Regarding the highlighting, I don't think there's any way of getting it to work with the current highlighting system of offsets and lengths. This is something that we should work on to improve in future.

You've included good descriptions for these operations, perhaps you could use the examples in them as test cases?

n1474335 · 2017-03-19T23:57:04Z

src/js/operations/ByteRepr.js

@@ -25,6 +25,11 @@ var ByteRepr = {
     * @constant
     * @default
     */
+    OCT_DELIM_OPTIONS: ["Space", "Comma", "Semi-colon", "Colon", "Line feed", "CRLF"],


This is the same as DELIM_OPTIONS so I'd just use that instead of duplicating code.

n1474335 · 2017-03-20T00:00:22Z

src/js/core/Utils.js

@@ -829,6 +829,38 @@ var Utils = {


    /**
+     * Convert an byte array into a octal string


I can see why you've put this in Utils because that's what I've done with the bin and hex operations, however I think in this particular case it would make more sense to just put this code directly into the ByteRepr.runToOct and ByteRepr.runFromOct functions. It's probably not something that will be used elsewhere in the app, and if it is we can always break it out into Utils again.

n1474335 · 2017-03-20T00:02:42Z

src/js/config/OperationConfig.js

@@ -392,7 +392,7 @@ var OperationConfig = {
        ]
    },
    "From Hex": {
-        description: "Converts a hexadecimal byte string back into a its raw value.<br><br>e.g. <code>ce 93 ce b5 ce b9 ce ac 20 cf 83 ce bf cf 85 0a</code> becomes the UTF-8 encoded string <code>Γειά σου</code>",
+        description: "Converts a hexadecimal byte string back into its raw value.<br><br>e.g. <code>ce 93 ce b5 ce b9 ce ac 20 cf 83 ce bf cf 85 0a</code> becomes the UTF-8 encoded string <code>Γειά σου</code>",


mattnotmitt · 2017-03-27T19:39:06Z

I've taken a stab at test cases, as well as fixing the requested changes. However (even with changing the arrow functions to normal functions so it actually runs), very few of the tests will ever pass.

✔️️ To Octal: nothing
🔥 From Octal: nothing
	From Octal - Data is not a valid byteArray: [null]
✔️️ To Octal: hello world
🔥 From Octal: hello world
	From Octal - Data is not a valid byteArray: [null,null...
🔥 To Octal: Γειά σου
	To Octal - undefined is not an object (evaluating 'window.app.options')
🔥 From Octal: Γειά σου
	From Octal - Data is not a valid byteArray: [null,null...

I'd be very appreciative if someone could point out what I'm doing wrong!

n1474335 · 2017-03-28T14:43:26Z

I think the problem with your tests is that expectedOutput should always be a string. A recipe always converts the output from the last operation to a string so that it is printable for the user. The way to think of it is to simulate what you would expect to be written in the output box in the web app, rather than exactly what your operation would return.

If you think this is unintuitive behaviour, perhaps we should consider modifying the test runner in future to allow testing individual operations instead of entire recipes.

mattnotmitt · 2017-03-28T15:23:58Z

I've rectified some of those problems with it now (forgot how normal JS functions work :P) - however I'm still getting these two test fails with this config.

🔥 To Octal: Γειά σου
	To Octal - undefined is not an object (evaluating 'window.app.options')
❌ From Octal: Γειά σου
	Expected
		Γειά σου
	Received
		Î�ÎµÎ¹Î¬ Ï�Î¿Ï

I'm particularly interested in the first one, since test doesn't give a full trace I can't tell where it's going wrong.

tlwr · 2017-03-28T16:25:55Z

@n1474335

If you think this is unintuitive behaviour, perhaps we should consider modifying the test runner in future to allow testing individual operations instead of entire recipes.

Agree, it is a big blind spot that doesn't allow us to properly test output types.

I can implement it such that tests have a recipe:

recipeConfig: [],

or just an operation and arguments:

operation: "To Octal",
operationArguments: [
],

It may also require operationInputType and operationOutputType if parsing JSDoc is not possible.

@artemisbot right now the test runner is a bit janky (we can improve it now with #95), though once you do grunt test you can navigate to $project-home/build/test/index.html and use the debugger to find out more. This is an ugly, hacky solution that I want to improve.

Your test To Octal: Γειά σου is erroring because of

        if (str.length !== wordArray.sigBytes)
            window.app.options.attemptHighlight = false;

in Utils.strToUtf8ByteArray, this may actually be indicative of a problem in the function rather than your use of it.

n1474335 · 2017-03-28T16:34:00Z

@tlwr, those modifications to the test runner sound good. It's probably best that you wait until #95 is merged before making changes otherwise there will be conflicts.

You should be able to look up the input and output types in OperationConfig.js, without having to resort to parsing JSDoc comments. Take a look at how the recipeConfig is parsed here to see how it works.

The problem you've found in Utils.strToUtf8ByteArray should get sorted out in #95.

n1474335 · 2017-03-30T18:40:46Z

@artemisbot
Now that #95 has been merged, this should be ready to go. Check that your tests are passing, commit them, and I'll pull this in.

mattnotmitt · 2017-03-30T19:31:04Z

Right, I'll sort them tomorrow :)

mattnotmitt · 2017-03-31T18:11:15Z

Worryingly, I'm still getting failures on the last two of my tests. Additionally, after running grunt test the webpage is not being generated, but I can still see that it's the same errors as normal.

mattnotmitt · 2017-04-05T22:10:14Z

I'm still interested as to why the tests are failing - perhaps someone else can try running them because they never pass at all for me. Hopefully, then we can push them to master.

n1474335 · 2017-04-05T22:13:34Z

I fixed the tests - it was entirely my fault. There were calls to window in Utils.js that weren't correctly defeated in non-browser environments. Once these were sorted out, all your tests passed.

The only thing I changed regarding your operations was the unnecessary cast to binary and back that you had in 'From Octal'. The input is already a byteArray so you can just call val.toString(8) which makes things a bit simpler.

It looks like Babel is correctly translating your arrow notation so things are working as expected! Thanks for another great contribution.

[DOCS] Adds docs for Render.com deployment

Adds Octal functions

b8ce10a

- To Octal - From Octal

n1474335 requested changes Mar 20, 2017

View reviewed changes

Merge branch 'master' into feature-octal

497824f

Made requested changes.

96e40a6

n1474335 approved these changes Mar 28, 2017

View reviewed changes

Made fixes so that tests might work

7ab6c88

Merge remote-tracking branch 'upstream/master' into feature-octal

a36c9ca

n1474335 merged commit a36c9ca into gchq:master Apr 5, 2017

mattnotmitt deleted the feature-octal branch April 5, 2017 22:15

BRAVO68WEB pushed a commit to BRAVO68WEB/CyberChef that referenced this pull request May 29, 2022

🔀 Merge pull request gchq#92 from Lissy93/DOCS/more-deploy-methods

a6ed52d

[DOCS] Adds docs for Render.com deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Operations: Octal #92

Added Operations: Octal #92

mattnotmitt commented Mar 14, 2017

m2951413 commented Mar 19, 2017

mattnotmitt commented Mar 19, 2017

m2951413 commented Mar 19, 2017 •

edited

Loading

n1474335 left a comment

n1474335 Mar 19, 2017

n1474335 Mar 20, 2017 •

edited

Loading

n1474335 Mar 20, 2017

mattnotmitt commented Mar 27, 2017 •

edited

Loading

n1474335 commented Mar 28, 2017

mattnotmitt commented Mar 28, 2017

tlwr commented Mar 28, 2017

n1474335 commented Mar 28, 2017

n1474335 commented Mar 30, 2017 •

edited

Loading

mattnotmitt commented Mar 30, 2017

mattnotmitt commented Mar 31, 2017

mattnotmitt commented Apr 5, 2017

n1474335 commented Apr 5, 2017

		@@ -829,6 +829,38 @@ var Utils = {


		/**
		* Convert an byte array into a octal string

Added Operations: Octal #92

Added Operations: Octal #92

Conversation

mattnotmitt commented Mar 14, 2017

m2951413 commented Mar 19, 2017

mattnotmitt commented Mar 19, 2017

m2951413 commented Mar 19, 2017 • edited Loading

n1474335 left a comment

Choose a reason for hiding this comment

n1474335 Mar 19, 2017

Choose a reason for hiding this comment

n1474335 Mar 20, 2017 • edited Loading

Choose a reason for hiding this comment

n1474335 Mar 20, 2017

Choose a reason for hiding this comment

mattnotmitt commented Mar 27, 2017 • edited Loading

n1474335 commented Mar 28, 2017

mattnotmitt commented Mar 28, 2017

tlwr commented Mar 28, 2017

n1474335 commented Mar 28, 2017

n1474335 commented Mar 30, 2017 • edited Loading

mattnotmitt commented Mar 30, 2017

mattnotmitt commented Mar 31, 2017

mattnotmitt commented Apr 5, 2017

n1474335 commented Apr 5, 2017

m2951413 commented Mar 19, 2017 •

edited

Loading

n1474335 Mar 20, 2017 •

edited

Loading

mattnotmitt commented Mar 27, 2017 •

edited

Loading

n1474335 commented Mar 30, 2017 •

edited

Loading