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

Add TLV / LV / KLV Decoding Operation #351

Merged
merged 5 commits into from
Oct 10, 2018
Merged

Add TLV / LV / KLV Decoding Operation #351

merged 5 commits into from
Oct 10, 2018

Conversation

GCHQ77703
Copy link
Member

Currently with a single, dysfunctional test.

@n1474335 n1474335 changed the title Add TLV / LV / KLV Decoding Operation WIP: Add TLV / LV / KLV Decoding Operation Aug 29, 2018
@GCHQ77703
Copy link
Member Author

GCHQ77703 commented Aug 29, 2018

Hey @n1474335, could I bother you for a second to take a look at why the tests are failing? The TLV / KLV / LV operation seems to be working just fine but no matter what I enter into the input and expectedOutput I appear to get the following error:

>> (node:2926) ExperimentalWarning: The ESM module loader is experimental.
>> { SyntaxError: Unexpected end of JSON input
>>     at JSON.parse (<anonymous>)
>>     at Dish._translate (<redacted>/CyberChef/src/core/Dish.mjs:213:35)
>>     at Dish.get (<redacted>/CyberChef/src/core/Dish.mjs:137:24)
>>     at Recipe.execute (<redacted>/CyberChef/src/core/Recipe.mjs:155:36)
>>     at Chef.bake (<redacted>/CyberChef/src/core/Chef.mjs:83:37)
>>     at <redacted>/CyberChef/test/TestRegister.mjs:43:29
>>     at Array.map (<anonymous>)
>>     at TestRegister.runTests (<redacted>/CyberChef/test/TestRegister.mjs:40:24)
>>     at <redacted>/CyberChef/test/index.mjs:127:14
>>     at ModuleJob.run (internal/loader/ModuleJob.js:97:14)
>>   progress: 0,
>>   displayStr: 'To MessagePack - Unexpected end of JSON input' }
>> (node:2926) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'module' of undefined
>>     at Recipe._parseConfig (<redacted>/CyberChef/src/core/Recipe.mjs:42:44)
>>     at new Recipe (<redacted>/CyberChef/src/core/Recipe.mjs:27:18)
>>     at Chef.bake (<redacted>/CyberChef/src/core/Chef.mjs:44:27)
>>     at <redacted>/CyberChef/test/TestRegister.mjs:43:29
>>     at Array.map (<anonymous>)
>>     at TestRegister.runTests (<redacted>/CyberChef/test/TestRegister.mjs:40:24)
>>     at <redacted>/CyberChef/test/index.mjs:127:14
>>     at ModuleJob.run (internal/loader/ModuleJob.js:97:14)
>>     at <anonymous>
>> (node:2926) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 3)
>> (node:2926) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
Tests took longer than 10 seconds to run, returning.

Even if I alter the output of the LV decoder to be string instead of JSON, it still prints the same JSON error message.

Copy link
Member

@n1474335 n1474335 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, really looking forward to merging these ops. Just a few things to sort out, but thanks very much for taking them on, they'll be really useful.

@@ -53,7 +53,8 @@
"To MessagePack",
"From MessagePack",
"To Braille",
"From Braille"
"From Braille",
"From Length Value"
Copy link
Member

@n1474335 n1474335 Aug 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rename this to something like 'LV Decode'. Then we can have an 'LV Encode' operation that does the opposite.

this.name = "From Length Value";
this.module = "Default";
this.description = "Converts a Length-Value (LV) encoded string into a JSON object. Can optionally include a <code>Key</code> / <code>Type</code> entry.";
this.infoURL = "";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a link to the appropriate wiki page

this.args = [
{
name: "Bytes in Key Value",
type: "populateOption",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the wrong argument type to use. Take a look here for a description of each of the ingredient types: https://github.com/gchq/CyberChef/wiki/Adding-a-new-operation#data-types

TestRegister.addTests([
{
name: "KeyValue",
input: [5, 72, 111, 117, 115, 101, 4, 114, 111, 111, 109, 4, 100, 111, 111, 114],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see your logic here and it should probably be possible to do, but as it stands, tests expect all input and output to be a string, in the same way that the web app takes all input as a string and returns all output as a string.

The data will be converted to a byteArray automatically by CyberChef before it is passed to the operation.

In this case, you probably want to pass in "\x05\x48\x6f\x75\x73\x65\x04\x72\x6f\x6f\x6d\x04\x64\x6f\x6f\x72" and expect JSON.stringify([{"key": [25]..., null, 4).

@GCHQ77703 GCHQ77703 changed the title WIP: Add TLV / LV / KLV Decoding Operation Add TLV / LV / KLV Decoding Operation Sep 10, 2018
@n1474335 n1474335 merged commit 3833c5f into gchq:master Oct 10, 2018
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