`readUInt32` issues

[PeterTrotter]

Whilst querying a range of devices on my network I started to see issues with Uint32 parsing and I think we may be handling this a little too strictly.

Before this commit https://github.com/markabrahams/node-net-snmp/commit/6fee368f0bb4cf5064afcd199cc6ceaeb810ba2c readUint32 was broken but did not error (-3 was being parsed as 253 instead of 4294967293).

I saw the discussion in https://github.com/markabrahams/node-net-snmp/pull/206 around writeUint but I think we need to be accepting of real world use where others might be sending us bad data (In this case from a Cambium PMP450 radio).

As suggested by mvduin we could just add:

function readUint32 (buffer) {
    var parsedInt = buffer.readInt ();
    if ( ! Number.isInteger(parsedInt) ) {
        throw new TypeError('Value read as integer ' + parsedInt + ' is not an integer');
    }
    parsedInt = (parsedInt>>>0);
    if ( parsedInt < MIN_UNSIGNED_INT32 || parsedInt > MAX_UNSIGNED_INT32 ) {
        throw new RangeError('Read integer ' + parsedInt + ' is outside the unsigned 32-bit range');
    }
    return parsedInt;
}

At which point the slightly updated tests pass:

describe('parseUint()', function() {
    describe('given a positive integer', function() {
        var writer = new ber.Writer();
        writer.writeInt(3242425);
        var reader = new ber.Reader(writer.buffer);
        it('returns a positive integer', function() {
            assert.equal(3242425, snmp.ObjectParser.readUint32(reader));
        });
    }),
    describe('given a negative integer', function() {
        var writer = new ber.Writer();
        writer.writeInt(-3);
        var reader = new ber.Reader(writer.buffer);
        it('returns a positive integer', function() {
            assert.equal(4294967293, snmp.ObjectParser.readUint32(reader));
        });
    }),
    describe('given a large integer', function() {
        var writer = new ber.Writer();
        writer.writeInt(4294967293);
        var reader = new ber.Reader(writer.buffer);
        it('returns a positive integer', function() {
            assert.equal(4294967293, snmp.ObjectParser.readUint32(reader));
        });
    });
});

If you agree I'm happy to put a PR together.

[markabrahams]

Hi @PeterTrotter - yes, I'd be happy to accept a PR for this.

[PeterTrotter]

@markabrahams would it be appropriate to add a mocha test dependency and script?

{
...
  "devDependencies": {
    "eslint": "^5.16.0",
    "getopts": "*",
    "mocha": "^10.2.0"
  },
...
  "scripts": {
    "lint": "./node_modules/.bin/eslint . ./**/*.js",
    "test": "./node_modules/mocha/bin/mocha.js"
  }
}

[PeterTrotter]

Also, Hi and thanks for your work on this :smiley:

Think I must be in robot mode...

[markabrahams]

Hi @PeterTrotter - just re-read the mocha comment - and yes that would be appropriate! I'll add that into the next release.