Skip to content

fix(client): keep array index 0 in ReadProperty and WriteProperty requests - #191

Open
chowchin wants to merge 1 commit into
fh1ch:masterfrom
chowchin:fix/read-property-array-index-zero
Open

fix(client): keep array index 0 in ReadProperty and WriteProperty requests#191
chowchin wants to merge 1 commit into
fh1ch:masterfrom
chowchin:fix/read-property-array-index-zero

Conversation

@chowchin

Copy link
Copy Markdown

Problem

Index 0 of a BACnet array property holds the number of elements. Reading it is how a client finds out how large a property such as object-list is, before reading the entries one by one — the standard fallback when the whole array does not fit in a single APDU.

Two places pass the index through arrayIndex || ASN1_ARRAY_ALL, so the valid index 0 becomes ASN1_ARRAY_ALL and the request asks for the entire array instead:

  • lib/client.jsreadProperty and writeProperty option handling
  • lib/services/read-property.js — the request encoder

Round trip on master:

const buffer = {buffer: Buffer.alloc(1482), offset: 0};
baServices.readProperty.encode(buffer, 4, 630, 85, 0);
baServices.readProperty.decode(buffer.buffer, 0, buffer.offset).property.index;
// => 4294967295   (0xFFFFFFFF, the whole array)

The outer guard if (arrayIndex !== baEnum.ASN1_ARRAY_ALL) already establishes that an index was requested, so the || fallback inside it can only misfire — it is unreachable for every value except 0, which it corrupts.

Practical effect: against a device with a few thousand objects, reading object-list[0] returns Abort(segmentation-not-supported) (or overflows the APDU buffer) rather than the object count, so discovery cannot proceed.

Change

Pass the index through unchanged in the encoder, and in client.js fall back to ASN1_ARRAY_ALL only when arrayIndex was not supplied. The explicit !== undefined check is used rather than ?? to stay within the declared node >= 12 engine range.

lib/services/write-property.js already encoded the index correctly; only the client-side option handling needed the same treatment.

Tests

Added an index-0 round trip to test/unit/service-read-property.spec.js. It fails on master:

- "index": 0,
+ "index": 4294967295,

and passes with this change. Full unit suite: 33 suites, 118 tests, all passing.

…uests

Index 0 of a BACnet array property holds the number of elements, which is how a
client discovers the size of a property such as object-list before reading its
entries individually.

Both the client option handling and the ReadProperty encoder passed the index
through 'arrayIndex || ASN1_ARRAY_ALL', which turns the valid index 0 into
ASN1_ARRAY_ALL and therefore requests the entire array instead.

On a device with a large object-list that whole-array response does not fit into
a single APDU, so the request is aborted or times out and the client never
learns the object count.
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.

1 participant