ronantakizawa/codereview-bench huggingface dataset:
Repository: gchq/CyberChef
File: src/core/operations/DateTime.js (JavaScript)

Code before:
import Decimal from "../lib/decimal.min.js";

/**
 * Date and time operations.
 *
 * @author n1474335 [n1474335@gmail.com]
 * @copyright Crown Copyright 2016
 * @license Apache-2.0
 *
 * @namespace
 */
const DateTime = {

    /**
     * @constant
     * @default
     */
    UNITS: ["Seconds (s)", "Milliseconds (ms)", "Microseconds (μs)", "Nanoseconds (ns)"],

    /**
     * From UNIX Timestamp operation.
     *
     * @param {number} input
     * @param {Object[]} args
     * @returns {string}
     */

Review comment (suggestion):
Libraries should be imported through npm if at all possible (`npm install --save decimal.js`), rather than being included statically. However in this case, I would recommend using [`jsbn`](https://www.npmjs.com/package/jsbn) instead as it is already included in this project (see [`IP.js`](https://github.com/gchq/CyberChef/blob/master/src/core/operations/IP.js#L3)) and provides similar functionality. The [Design Principals](https://github.com/gchq/CyberChef/wiki/Contributing#design-principals) on the project wiki go into this reasoning in a bit more detail.

Code after:
import {BigInteger} from "jsbn";

/**
 * Date and time operations.
 *
 * @author n1474335 [n1474335@gmail.com]
 * @copyright Crown Copyright 2016
 * @license Apache-2.0
 *
 * @namespace
 */
const DateTime = {

    /**
     * @constant
     * @default
     */
    UNITS: ["Seconds (s)", "Milliseconds (ms)", "Microseconds (μs)", "Nanoseconds (ns)"],

    /**
     * From UNIX Timestamp operation.
     *
     * @param {number} input
     * @param {Object[]} args
     * @returns {string}
     */
Repository: gchq/CyberChef
File: src/core/operations/MS.js (JavaScript)

Code before:
        0,
        0,
        1,
        2,
        2,
        0,
        0,
        1,
        2,
        0,
        2,
        1
    ],

    /**
     * @param {string} data
     * @returns {string}
     */
    decode: function (data) {
        let result = [];
        let index = -1;
        data = data.replace(/@&/g, String.fromCharCode(10));
        data = data.replace(/@#/g, String.fromCharCode(13));
        data = data.replace(/@\*/g, ">");
        data = data.replace(/@!/g, "<");
        data = data.replace(/@\$/g, "@");
        for (let i = 0; i < data.length; i++) {
            let byte = data.charCodeAt(i);
            let char = data.charAt(i);
            if (byte < 128) {
                index++;
            }
            if ((byte === 9 || byte > 31 && byte < 128) && byte !== 60 && byte !== 62 && byte !== 64) {
                char = MS.D_DECODE[byte].charAt(MS.D_COMBINATION[index % 64]);
            }
            result.push(char);
        }
        return result.join("");
    },

    /**
     * @param {string} input
     * @param {Object[]} args
     * @returns {string}
     */
    runDecodeScript: function (input, args) {
        let matcher = /#@~\^......==(.+)......==\^#~@/;
        let encodedData = matcher.exec(input);
        if (encodedData){
            return MS.decode(encodedData[1]);
        } else {

Review comment (suggestion):
These `.replace` calls should be chained i.e.
```
data = data
    .replace(...)
    .replace(...)
    .replace(...);
```

Code after:
        0,
        0,
        1,
        2,
        2,
        0,
        0,
        1,
        2,
        0,
        2,
        1
    ],

    /**
     * @private
     * @param {string} data
     * @returns {string}
     */
    _decode: function (data) {
        let result = [];
        let index = -1;
        data = data.replace(/@&/g, String.fromCharCode(10))
            .replace(/@#/g, String.fromCharCode(13))
            .replace(/@\*/g, ">")
            .replace(/@!/g, "<")
            .replace(/@\$/g, "@");
        for (let i = 0; i < data.length; i++) {
            let byte = data.charCodeAt(i);
            let char = data.charAt(i);
            if (byte < 128) {
                index++;
            }
            if ((byte === 9 || byte > 31 && byte < 128) && byte !== 60 && byte !== 62 && byte !== 64) {
                char = MS.D_DECODE[byte].charAt(MS.D_COMBINATION[index % 64]);
            }
            result.push(char);
        }
        return result.join("");
    },

    /**
     * Microsoft Script Decoder operation
     *
     * @param {string} input
     * @param {Object[]} args
     * @returns {string}
     */
    runDecodeScript: function (input, args) {
        let matcher = /#@~\^.{6}==(.+).{6}==\^#~@/;
        let encodedData = matcher.exec(input);
Repository: gchq/CyberChef
File: src/core/operations/MS.js (JavaScript)

Code before:
        data = data.replace(/@&/g, String.fromCharCode(10));
        data = data.replace(/@#/g, String.fromCharCode(13));
        data = data.replace(/@\*/g, ">");
        data = data.replace(/@!/g, "<");
        data = data.replace(/@\$/g, "@");
        for (let i = 0; i < data.length; i++) {
            let byte = data.charCodeAt(i);
            let char = data.charAt(i);
            if (byte < 128) {
                index++;
            }
            if ((byte === 9 || byte > 31 && byte < 128) && byte !== 60 && byte !== 62 && byte !== 64) {
                char = MS.D_DECODE[byte].charAt(MS.D_COMBINATION[index % 64]);
            }
            result.push(char);
        }
        return result.join("");
    },

    /**
     * @param {string} input
     * @param {Object[]} args
     * @returns {string}
     */
    runDecodeScript: function (input, args) {
        let matcher = /#@~\^......==(.+)......==\^#~@/;
        let encodedData = matcher.exec(input);
        if (encodedData){
            return MS.decode(encodedData[1]);
        } else {
            return "";
        }
    },
};

export default MS;


Review comment (suggestion):
The dots in this regex can be collapsed using range notation e.g. `#@~\^.{6}==(.+).{6}==\^#~@`. This seems a little easier to read to me.

Code after:
        let index = -1;
        data = data.replace(/@&/g, String.fromCharCode(10))
            .replace(/@#/g, String.fromCharCode(13))
            .replace(/@\*/g, ">")
            .replace(/@!/g, "<")
            .replace(/@\$/g, "@");
        for (let i = 0; i < data.length; i++) {
            let byte = data.charCodeAt(i);
            let char = data.charAt(i);
            if (byte < 128) {
                index++;
            }
            if ((byte === 9 || byte > 31 && byte < 128) && byte !== 60 && byte !== 62 && byte !== 64) {
                char = MS.D_DECODE[byte].charAt(MS.D_COMBINATION[index % 64]);
            }
            result.push(char);
        }
        return result.join("");
    },

    /**
     * Microsoft Script Decoder operation
     *
     * @param {string} input
     * @param {Object[]} args
     * @returns {string}
     */
    runDecodeScript: function (input, args) {
        let matcher = /#@~\^.{6}==(.+).{6}==\^#~@/;
        let encodedData = matcher.exec(input);
        if (encodedData){
            return MS._decode(encodedData[1]);
        } else {
            return "";
        }
    },
};

export default MS;
Repository: gchq/CyberChef
File: src/core/operations/Sort.mjs (JavaScript)

Code before:
                if (ret !== 0) return ret;
            }
            if (!isNaN(a_[i]) && !isNaN(a_[i])) { // Compare numbers
                if (a_[i] - b_[i] !== 0) return a_[i] - b_[i];
            }
        }

        return a.localeCompare(b);
    }

    /**
     * Comparison operation for sorting of hexadecimal values.
     *
     * @author Chris van Marle
     * @private
     * @param {string} a
     * @param {string} b
     * @returns {number}
     */
    static _hexadecimalSort(a, b) {
        const a_ = a.split(/([^\da-f]+)/i),
            b_ = b.split(/([^\da-f]+)/i);

        for (let i = 0; i < a_.length; ++i) {
            const t = parseInt(a_[i], 16);
            if (!isNaN(t)) {
                a_[i] = t;
            }
        }

        for (let i = 0; i < b_.length; ++i) {
            const t = parseInt(b_[i], 16);
            if (!isNaN(t)) {
                b_[i] = t;
            }
        }

        for (let i = 0; i < a_.length && i < b.length; ++i) {
            if (isNaN(a_[i]) && !isNaN(b_[i])) return 1; // Numbers after non-numbers
            if (!isNaN(a_[i]) && isNaN(b_[i])) return -1;
            if (isNaN(a_[i]) && isNaN(b_[i])) {
                const ret = a_[i].localeCompare(b_[i]); // Compare strings
                if (ret !== 0) return ret;
            }
            if (!isNaN(a_[i]) && !isNaN(a_[i])) { // Compare numbers
                if (a_[i] - b_[i] !== 0) return a_[i] - b_[i];
            }
        }

        return a.localeCompare(b);
    }

Review comment (suggestion):
could using a `typeof` test be more clear than the double-negative `!isNaN`? e.g. `if(typeof t === "string")`

Code after:
                if (ret !== 0) return ret;
            }
            if (!isNaN(a_[i]) && !isNaN(a_[i])) { // Compare numbers
                if (a_[i] - b_[i] !== 0) return a_[i] - b_[i];
            }
        }

        return a.localeCompare(b);
    }

    /**
     * Comparison operation for sorting of hexadecimal values.
     *
     * @author Chris van Marle
     * @private
     * @param {string} a
     * @param {string} b
     * @returns {number}
     */
    static _hexadecimalSort(a, b) {
        let a_ = a.split(/([^\da-f]+)/i),
            b_ = b.split(/([^\da-f]+)/i);

        a_ = a_.map(v => {
            const t = parseInt(v, 16);
            return isNaN(t) ? v : t;
        });

        b_ = b_.map(v => {
            const t = parseInt(v, 16);
            return isNaN(t) ? v : t;
        });

        for (let i = 0; i < a_.length && i < b.length; ++i) {
            if (isNaN(a_[i]) && !isNaN(b_[i])) return 1; // Numbers after non-numbers
            if (!isNaN(a_[i]) && isNaN(b_[i])) return -1;
            if (isNaN(a_[i]) && isNaN(b_[i])) {
                const ret = a_[i].localeCompare(b_[i]); // Compare strings
                if (ret !== 0) return ret;
            }
            if (!isNaN(a_[i]) && !isNaN(a_[i])) { // Compare numbers
                if (a_[i] - b_[i] !== 0) return a_[i] - b_[i];
            }
        }

        return a.localeCompare(b);
    }

}

export default Sort;
Repository: gchq/CyberChef
File: src/core/lib/FileSignatures.mjs (JavaScript)

Code before:
 * @param {number} offset
 * @returns {Uint8Array}
 */
export function extractELF(bytes, offset) {
    const stream = new Stream(bytes.slice(offset));

    // Skip over magic number
    stream.moveForwardsBy(4);

    // Read architecture (x86 == 1, x64 == 2)
    const x86 = stream.readInt(1) === 1;

    // Read endianness (1 == little, 2 == big)
    const endian = stream.readInt(1) === 1 ? "le" : "be";

    // Skip over header values
    stream.moveForwardsBy(x86 ? 26 : 34);

    // Read section header table offset
    const shoff = x86 ? stream.readInt(4, endian) : stream.readInt(8, endian);

    // Skip over flags, header size and program header size and entries
    stream.moveForwardsBy(10);

    // Read section header table entry size
    const shentsize = stream.readInt(2, endian);

    // Read number of entries in the section header table
    const shnum = stream.readInt(2, endian);

    // Jump to section header table
    stream.moveTo(shoff);

    // Move past each section header
    stream.moveForwardsBy(shentsize * shnum);

    return stream.carve();
}


// Construct required Huffman Tables
const fixedLiteralTableLengths = new Array(288);
for (let i = 0; i < fixedLiteralTableLengths.length; i++) {
    fixedLiteralTableLengths[i] =
        (i <= 143) ? 8 :
            (i <= 255) ? 9 :
                (i <= 279) ? 7 :
                    8;
}
const fixedLiteralTable = buildHuffmanTable(fixedLiteralTableLengths);
const fixedDistanceTableLengths = new Array(30).fill(5);

Review comment (suggestion):
In the interests of defensive programming, I'd use `i < lookingfor.length` here. I realise we're unlikely to add more, but it's good practice.

Code after:

    return stream.carve();
}


/**
 * @param {Uint8Array} bytes
 * @param {Number} offset
 * @returns {Uint8Array}
 */
export function extractBZIP2(bytes, offset){
    const stream = new Stream(bytes.slice(offset));
    
    //The EOFs shifted between all possible combinations.
    var lookingfor = [
                        [0x77, 0x24, 0x53, 0x85, 0x09],
                        [0xee, 0x48, 0xa7, 0x0a, 0x12],
                        [0xdc, 0x91, 0x4e, 0x14, 0x24],
                        [0xb9, 0x22, 0x9c, 0x28, 0x48],
                        [0x72, 0x45, 0x38, 0x50, 0x90],
                        [0xbb, 0x92, 0x29, 0xc2, 0x84],
                        [0x5d, 0xc9, 0x14, 0xe1, 0x42],
                        [0x2e, 0xe4, 0x8a, 0x70, 0xa1],
                        [0x17, 0x72, 0x45, 0x38, 0x50]];
    
    for(let i = 0; i < 9; i++){

        //Continue until an EOF.
        stream.continueUntil(lookingfor[i]);
        if(stream.getBytes(5).join("") == lookingfor[i].join(""))
            break;

        //Jump back to the start if invalid EOF.
        stream.moveTo(0);
    }
    stream.moveForwardsBy(4);
    
    return stream.carve();
}


/**
 * Zlib extractor.
 *
 * @param {Uint8Array} bytes
 * @param {number} offset
 * @returns {Uint8Array}
 */
export function extractZlib(bytes, offset) {
    const stream = new Stream(bytes.slice(offset));
Repository: gchq/CyberChef
File: src/core/lib/Stream.mjs (JavaScript)

Code before:

    /**
     * Consume the stream until we reach the specified byte or sequence of bytes.
     *
     * @param {number|List<number>} val
     */
    continueUntil(val) {
        if (this.position > this.length) return;

        this.bitPos = 0;

        if (typeof val === "number") {
            while (++this.position < this.length && this.bytes[this.position] !== val) {
                continue;
            }
            return;
        }

        // val is an array


        /**
         * Build's the skip forward table from the value to be searched.
         *
         * @param val
         * @param len
         */
        function preprocess(val, len) {
            const skiptable = new Array();
            val.forEach(function(element, index) {
                skiptable[element] = len - index;
            });
            return skiptable;
        }

        const length = val.length;

        const initial = val[length-1];

        this.position = length;

        // Get the skip table.
        const skiptable = preprocess(val, length);
        let found = true;

        while (this.position < this.length) {

            // Until we hit the final element of val in the stream.
            while ((this.position < this.length) && (this.bytes[this.position++] !== initial));

            found = true;

Review comment (suggestion):
Add the data type for each of these. e.g.
``` javascript
@param {number} val
```
Also add a `@returns` field.

Code after:

    /**
     * Consume the stream until we reach the specified byte or sequence of bytes.
     *
     * @param {number|List<number>} val
     */
    continueUntil(val) {
        if (this.position > this.length) return;

        this.bitPos = 0;

        if (typeof val === "number") {
            while (++this.position < this.length && this.bytes[this.position] !== val) {
                continue;
            }
            return;
        }

        // val is an array


        /**
         * Builds the skip forward table from the value to be searched.
         *
         * @param {Uint8Array} val
         * @param {Number} len
         * @returns {Uint8Array}
         */
        function preprocess(val, len) {
            const skiptable = new Array();
            val.forEach((element, index) => {
                skiptable[element] = len - index;
            });
            return skiptable;
        }

        const length = val.length;

        const initial = val[length-1];

        this.position = length;

        // Get the skip table.
        const skiptable = preprocess(val, length);
        let found = true;

        while (this.position < this.length) {

            // Until we hit the final element of val in the stream.
            while ((this.position < this.length) && (this.bytes[this.position++] !== initial));
Repository: gchq/CyberChef
File: src/core/lib/Stream.mjs (JavaScript)

Code before:
     * @param {number|List<number>} val
     */
    continueUntil(val) {
        if (this.position > this.length) return;

        this.bitPos = 0;

        if (typeof val === "number") {
            while (++this.position < this.length && this.bytes[this.position] !== val) {
                continue;
            }
            return;
        }

        // val is an array


        /**
         * Build's the skip forward table from the value to be searched.
         *
         * @param val
         * @param len
         */
        function preprocess(val, len) {
            const skiptable = new Array();
            val.forEach(function(element, index) {
                skiptable[element] = len - index;
            });
            return skiptable;
        }

        const length = val.length;

        const initial = val[length-1];

        this.position = length;

        // Get the skip table.
        const skiptable = preprocess(val, length);
        let found = true;

        while (this.position < this.length) {

            // Until we hit the final element of val in the stream.
            while ((this.position < this.length) && (this.bytes[this.position++] !== initial));

            found = true;

            // Loop through the elements comparing them to val.
            for (let x = length-1; x+1; x--) {
                if (this.bytes[(this.position-length) + x] !== val[x]) {

Review comment (suggestion):
You can use the funky ES6 shorthand here for a slightly neater inline function:
``` javascript
val.forEach((element, index) => {
```

Code after:
     * @param {number|List<number>} val
     */
    continueUntil(val) {
        if (this.position > this.length) return;

        this.bitPos = 0;

        if (typeof val === "number") {
            while (++this.position < this.length && this.bytes[this.position] !== val) {
                continue;
            }
            return;
        }

        // val is an array


        /**
         * Builds the skip forward table from the value to be searched.
         *
         * @param {Uint8Array} val
         * @param {Number} len
         * @returns {Uint8Array}
         */
        function preprocess(val, len) {
            const skiptable = new Array();
            val.forEach((element, index) => {
                skiptable[element] = len - index;
            });
            return skiptable;
        }

        const length = val.length;

        const initial = val[length-1];

        this.position = length;

        // Get the skip table.
        const skiptable = preprocess(val, length);
        let found = true;

        while (this.position < this.length) {

            // Until we hit the final element of val in the stream.
            while ((this.position < this.length) && (this.bytes[this.position++] !== initial));

            found = true;

            // Loop through the elements comparing them to val.
            for (let x = length-1; x >= 0; x--) {
Repository: gchq/CyberChef
File: src/core/lib/Stream.mjs (JavaScript)

Code before:

            // Loop through the elements comparing them to val.
            for (let x = length-1; x+1; x--) {
                if (this.bytes[(this.position-length) + x] !== val[x]) {
                    found = false;

                    // If element is not equal to val's element then jump forward by the correct amount.
                    this.position += skiptable[val[x]];
                    break;
                }
            }
            if (found) {
                this.position = (this.position - length);
                break;
            }
        }
    }


    /**
     * Consume bytes if it matches the supplied value.
     *
     * @param val
     */
    consumeWhile(val) {
        while ((this.position < this.length) && (this.bytes[this.position++] === val));
    }

    /**
     * Consume the next byte if it matches the supplied value.
     *
     * @param {number} val
     */
    consumeIf(val) {
        if (this.bytes[this.position] === val) {
            this.position++;
            this.bitPos = 0;
        }
    }

    /**
     * Move forwards through the stream by the specified number of bytes.
     *
     * @param {number} numBytes
     */
    moveForwardsBy(numBytes) {
        const pos = this.position + numBytes;
        if (pos < 0 || pos > this.length)
            throw new Error("Cannot move to position " + pos + " in stream. Out of bounds.");
        this.position = pos;
        this.bitPos = 0;

Review comment (suggestion):
This will always increment `this.position` by at least 1. We don't want to do that if the next byte is not equal to `val`.

Code after:
            found = true;

            // Loop through the elements comparing them to val.
            for (let x = length-1; x >= 0; x--) {
                if (this.bytes[this.position-length + x] !== val[x]) {
                    found = false;

                    // If element is not equal to val's element then jump forward by the correct amount.
                    this.position += skiptable[val[x]];
                    break;
                }
            }
            if (found) {
                this.position -= length;
                break;
            }
        }
    }


    /**
     * Consume bytes if it matches the supplied value.
     *
     * @param {Number} val
     */
    consumeWhile(val) {
        while (this.position < this.length) {
            if (this.bytes[this.position] !== val) {
                break;
            }
            this.position++;
        }
    }

    /**
     * Consume the next byte if it matches the supplied value.
     *
     * @param {number} val
     */
    consumeIf(val) {
        if (this.bytes[this.position] === val) {
            this.position++;
            this.bitPos = 0;
        }
    }

    /**
     * Move forwards through the stream by the specified number of bytes.
     *
     * @param {number} numBytes
     */