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

Types greater than 512 fail. Falls short of ASN1_MAX_OBJECT #122

Closed
1 task done
NateZimmer opened this issue Sep 25, 2018 · 1 comment
Closed
1 task done

Types greater than 512 fail. Falls short of ASN1_MAX_OBJECT #122

NateZimmer opened this issue Sep 25, 2018 · 1 comment
Assignees
Labels
Milestone

Comments

@NateZimmer
Copy link

Node Version: 9.1

Node BACstack Version: 0.0.1-beta.10 (though I believe this to be in the present as well)

  • Bug Report

Current Behavior (Bug Report)

Bacnet types that are greater than 512 cause an exception. ASN1_MAX_OBJECT is defined as 0x3FF / 1023.
baAsn1.encodeContextObjectId(buffer, 0, objectType, objectInstance); returns an exception for these values when it shouldn't be since they are less than 0x3FF.

The issue is the following lines of code:
asn1.js

const encodeBacnetObjectId = module.exports.encodeBacnetObjectId = (buffer, objectType, instance) => {
  const value = ((objectType & baEnum.ASN1_MAX_OBJECT) << baEnum.ASN1_INSTANCE_BITS) | (instance & baEnum.ASN1_MAX_INSTANCE);
  encodeUnsigned(buffer, value, 4);
};

value isn't intrinsically an unsigned int. It is an int. Hence it propagates a negative value which eventually throws an error when put into an unsigned buffer. A solution is as follows:

const encodeBacnetObjectId = module.exports.encodeBacnetObjectId = (buffer, objectType, instance) => {
  const value = (((objectType & baEnum.ASN1_MAX_OBJECT) << baEnum.ASN1_INSTANCE_BITS) | (instance & baEnum.ASN1_MAX_INSTANCE))>>>0;
  encodeUnsigned(buffer, value, 4);
};

Another example:
(512 & 0x3FF) << 22 | (1 & 0x3FFFFF ) = -2147483647
((512 & 0x3FF) << 22 | (1 & 0x3FFFFF )) >>> 0 = 2147483649

NateZimmer pushed a commit to NateZimmer/node-bacstack that referenced this issue Jan 14, 2019
@fh1ch fh1ch self-assigned this Feb 10, 2019
@fh1ch fh1ch added the bug label Feb 10, 2019
@fh1ch fh1ch added this to the 0.0.1-beta.14 milestone Feb 10, 2019
@fh1ch
Copy link
Owner

fh1ch commented Feb 10, 2019

@NateZimmer sorry for the delay on this one. The whole project was more or less on ice for a while, since I was busy with other stuff...

However, a bight thanks for reporting this issue and already providing a solution 🎉 I've added it in #133 and covered it with some tests. It should be available with the next release.

@fh1ch fh1ch closed this as completed in 3103ad5 Feb 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants