diff --git a/src/datatypes/compiler-utils.js b/src/datatypes/compiler-utils.js index d60eda5..6625a51 100644 --- a/src/datatypes/compiler-utils.js +++ b/src/datatypes/compiler-utils.js @@ -154,6 +154,7 @@ let val = value._value ${big ? '|| 0n' : ''} for (const key in flags) { if (value[key]) val |= flags[key] } +${(!big && /^l?u/.test(type)) ? 'val = val >>> 0 // unsigned underlying type: keep bit 31 from making val negative and rejected' : ''} return (ctx.${type})(val, buffer, offset) `.trim()) }], @@ -208,6 +209,7 @@ let val = value._value ${big ? '|| 0n' : ''} for (const key in flags) { if (value[key]) val |= flags[key] } +${(!big && /^l?u/.test(type)) ? 'val = val >>> 0 // unsigned underlying type (see above)' : ''} return (ctx.${type})(val) `.trim()) }], diff --git a/src/datatypes/utils.js b/src/datatypes/utils.js index 2f1722b..40adce8 100644 --- a/src/datatypes/utils.js +++ b/src/datatypes/utils.js @@ -257,6 +257,10 @@ function writeBitflags (value, buffer, offset, { type, flags, shift, big }, root for (const key in f) { if (value[key]) val |= f[key] } + // Coerce to unsigned only for unsigned underlying types: |= is signed in JS, so bit 31 makes val negative and the + // unsigned writer rejects it. Signed types (i8/i16/i32/li32) keep the |= result, which is already the correct value; + // forcing them unsigned would push a valid negative out of the signed writer's range. + if (!big && /^l?u/.test(type)) val = val >>> 0 return this.write(val, buffer, offset, type, rootNode) } diff --git a/test/misc.js b/test/misc.js index fc726e9..6877ba4 100644 --- a/test/misc.js +++ b/test/misc.js @@ -26,6 +26,51 @@ describe('mapper', () => { } }) +describe('bitflags', () => { + // A 32-bit bitflags whose top flag is bit 31. `|=` is signed in JS, so building the value makes it negative; + // the writer must treat it as unsigned or writeUInt32LE rejects it (regression for a bit-31 write crash). + const flags = Array.from({ length: 32 }, (_, i) => (i === 31 ? 'topbit' : 'f' + i)) + const type = ['bitflags', { type: 'lu32', flags }] + const proto = new ProtoDef() + proto.addType('flags32', type) + const compiler = new ProtoDefCompiler() + compiler.addTypesToCompile({ flags32: type }) + const compiled = compiler.compileProtoDefSync() + + for (const [label, p] of [['interpreted', proto], ['compiled', compiled]]) { + it(`round-trips a value with bit 31 set (${label})`, () => { + const buf = p.createPacketBuffer('flags32', { topbit: true, f0: true }) + assert.deepStrictEqual(buf, Buffer.from([0x01, 0x00, 0x00, 0x80])) + const back = p.parsePacketBuffer('flags32', buf).data + assert.strictEqual(back.topbit, true) + assert.strictEqual(back.f0, true) + assert.strictEqual(back.f1, false) + }) + } +}) + +describe('bitflags with a signed underlying type', () => { + // Signed underlying type with bit 31 set: reading 0xffffffff as i32 yields -1. The unsigned coercion must NOT apply + // here, or writing the decoded value pushes -1 to 4294967295 and the signed writer rejects it (a regression the + // unsigned bit-31 fix introduced). The |= result is already the correct signed value. + const type = ['bitflags', { type: 'i32', flags: { top: 31 }, shift: true }] + const proto = new ProtoDef() + proto.addType('sflags', type) + const compiler = new ProtoDefCompiler() + compiler.addTypesToCompile({ sflags: type }) + const compiled = compiler.compileProtoDefSync() + + for (const [label, p] of [['interpreted', proto], ['compiled', compiled]]) { + it(`round-trips a signed value with bit 31 set (${label})`, () => { + const buf = Buffer.from([0xff, 0xff, 0xff, 0xff]) // i32 -1, top bit set + const obj = p.parsePacketBuffer('sflags', buf).data + assert.strictEqual(obj.top, true) + const back = p.createPacketBuffer('sflags', obj) // must not throw and must reproduce the original bytes + assert.deepStrictEqual(back, buf) + }) + } +}) + describe('FullPacketParser', () => { const packet = ['container', [{ name: 'a', type: 'i32' }]] const proto = new ProtoDef()