From 41c0089d48ed938b2eba8263b979d213fd7ddfe1 Mon Sep 17 00:00:00 2001 From: Benjamin Date: Fri, 8 Mar 2024 13:49:36 +0100 Subject: [PATCH] Fix integer attrib convertion with signed integer type (#1246) (#1258) --- src-electron/rest/user-data.js | 3 +- src-electron/validation/validation.js | 211 +++++++++++++++++++-- test/validation.test.js | 260 ++++++++++++++++++++++---- 3 files changed, 426 insertions(+), 48 deletions(-) diff --git a/src-electron/rest/user-data.js b/src-electron/rest/user-data.js index ba66be8c28..154a40f7f8 100644 --- a/src-electron/rest/user-data.js +++ b/src-electron/rest/user-data.js @@ -333,7 +333,8 @@ function httpPostAttributeUpdate(db) { db, endpointTypeIdList[0], id, - clusterRef + clusterRef, + request.zapSessionId ) response.status(StatusCodes.OK).json({ diff --git a/src-electron/validation/validation.js b/src-electron/validation/validation.js index e40065b57c..85b19779fb 100644 --- a/src-electron/validation/validation.js +++ b/src-electron/validation/validation.js @@ -25,8 +25,26 @@ const queryZcl = require('../db/query-zcl.js') const queryConfig = require('../db/query-config.js') const queryEndpoint = require('../db/query-endpoint.js') const types = require('../util/types.js') +const queryPackage = require('../db/query-package.js') -async function validateAttribute(db, endpointTypeId, attributeRef, clusterRef) { +/** + * Main attribute validation function. + * Returns a promise of an object which stores a list of validational issues. + * Such issues as "Invalid type" or "Out of Range". + * @param {*} db db reference + * @param {*} endpointTypeId endpoint reference + * @param {*} attributeRef attribute reference + * @param {*} clusterRef cluster reference + * @param {*} zapSessionId session reference + * @returns Promise of the list of issues + */ +async function validateAttribute( + db, + endpointTypeId, + attributeRef, + clusterRef, + zapSessionId +) { let endpointAttribute = await queryZcl.selectEndpointTypeAttribute( db, endpointTypeId, @@ -34,7 +52,13 @@ async function validateAttribute(db, endpointTypeId, attributeRef, clusterRef) { clusterRef ) let attribute = await queryZcl.selectAttributeById(db, attributeRef) - return validateSpecificAttribute(endpointAttribute, attribute) + return validateSpecificAttribute( + endpointAttribute, + attribute, + db, + zapSessionId, + clusterRef + ) } async function validateEndpoint(db, endpointId) { @@ -65,7 +89,22 @@ async function validateNoDuplicateEndpoints( return count.length <= 1 } -function validateSpecificAttribute(endpointAttribute, attribute) { +/** + * Checks the attributes type then validates the incoming input string. + * @param {*} endpointAttribute + * @param {*} attribute + * @param {*} db + * @param {*} zapSessionId + * @param {*} clusterRef + * @returns List of issues wrapped in an object + */ +async function validateSpecificAttribute( + endpointAttribute, + attribute, + db, + zapSessionId, + clusterRef +) { let defaultAttributeIssues = [] if (attribute.isNullable && endpointAttribute.defaultValue == null) { return { defaultValue: defaultAttributeIssues } @@ -77,15 +116,35 @@ function validateSpecificAttribute(endpointAttribute, attribute) { if (!checkAttributeBoundsFloat(attribute, endpointAttribute)) defaultAttributeIssues.push('Out of range') } else if (types.isSignedInteger(attribute.type)) { - if (!isValidSignedNumberString(endpointAttribute.defaultValue)) + if (!isValidSignedNumberString(endpointAttribute.defaultValue)) { defaultAttributeIssues.push('Invalid Integer') - if (!checkAttributeBoundsInteger(attribute, endpointAttribute)) + } else if ( + //we shouldn't check boundaries for an invalid number string + !(await checkAttributeBoundsInteger( + attribute, + endpointAttribute, + db, + zapSessionId, + clusterRef + )) + ) { defaultAttributeIssues.push('Out of range') + } } else { - if (!isValidNumberString(endpointAttribute.defaultValue)) + if (!isValidNumberString(endpointAttribute.defaultValue)) { defaultAttributeIssues.push('Invalid Integer') - if (!checkAttributeBoundsInteger(attribute, endpointAttribute)) + } else if ( + // we shouldn't check boundaries for an invalid number string + !(await checkAttributeBoundsInteger( + attribute, + endpointAttribute, + db, + zapSessionId, + clusterRef + )) + ) { defaultAttributeIssues.push('Out of range') + } } } else if (types.isString(attribute.type)) { let maxLengthForString = @@ -127,11 +186,18 @@ function isValidNumberString(value) { //We test to see if the number is valid in hex. Decimals numbers also pass this test return /^(0x)?[\dA-F]+$/i.test(value) || Number.isInteger(Number(value)) } - function isValidSignedNumberString(value) { return /^(0x)?[\dA-F]+$/i.test(value) || Number.isInteger(Number(value)) } +function isValidHexString(value) { + return /^(0x)?[\dA-F]+$/i.test(value) +} + +function isValidDecimalString(value) { + return /^\d+$/.test(value) +} + function isValidFloat(value) { return !/^0x/i.test(value) && !isNaN(Number(value)) } @@ -140,6 +206,13 @@ function extractFloatValue(value) { return parseFloat(value) } +/** + * Expects a number string , parse it back on a default base 10 if its a decimal. + * If its a hexadecimal or anything else , parse it back on base 16. + * Loses precision after javascripts Number.MAX_SAFE_INTEGER range. + * @param {*} value + * @returns A decimal number + */ function extractIntegerValue(value) { if (/^-?\d+$/.test(value)) { return parseInt(value) @@ -150,16 +223,123 @@ function extractIntegerValue(value) { } } -function getBoundsInteger(attribute) { +function extractBigIntegerValue(value) { + if (/^-?\d+$/.test(value)) { + return BigInt(value) + } else if (/^[0-9A-F]+$/i.test(value)) { + return BigInt('0x' + value) + } else { + return BigInt(value) + } +} + +function isBigInteger(bits) { + return bits >= 32 +} + +async function getBoundsInteger(attribute, typeSize, isSigned) { return { - min: extractIntegerValue(attribute.min), - max: extractIntegerValue(attribute.max), + min: await getIntegerFromAttribute(attribute.min, typeSize, isSigned), + max: await getIntegerFromAttribute(attribute.max, typeSize, isSigned), } } -function checkAttributeBoundsInteger(attribute, endpointAttribute) { - let { min, max } = getBoundsInteger(attribute) - let defaultValue = extractIntegerValue(endpointAttribute.defaultValue) +/** + * Converts an unsigned integer to its signed value. Returns the same integer if its not a signed type. + * Works for both BigInts and regular numbers. + * @param {*} value - integer to convert + * @param {*} typeSize - bit representation + * @returns A decimal number + */ +function unsignedToSignedInteger(value, typeSize) { + const isSigned = value.toString(2).padStart(typeSize, '0').charAt(0) === '1' + if (isSigned) { + value = ~value + value += isBigInteger(typeSize) ? 1n : 1 + } + return value +} + +/** + * Converts an attribute (number string) into a decimal number without losing precision. + * Accepts both decimal and hexadecimal strings (former has priority) in any bit representation. + * Shifts signed hexadecimals to their correct value. + * @param {*} attribute - attribute to convert + * @param {*} typeSize - bit representation size + * @param {*} isSigned - is type is signed + * @returns A decimal number + */ +async function getIntegerFromAttribute(attribute, typeSize, isSigned) { + let value = isBigInteger(typeSize) + ? extractBigIntegerValue(attribute) + : extractIntegerValue(attribute) + if ( + !isValidDecimalString(attribute) && + isValidHexString(attribute) && + isSigned + ) { + value = unsignedToSignedInteger(value, typeSize) + } + return value +} + +/** + * Returns information about an integer type. + * @param {*} db + * @param {*} zapSessionId + * @param {*} clusterRef + * @param {*} attribType + * @returns {*} { size: bit representation , isSigned: is signed type } + */ +async function getIntegerAttributeSize( + db, + zapSessionId, + clusterRef, + attribType +) { + let packageIds = await queryPackage.getSessionZclPackageIds(db, zapSessionId) + let attribData = await queryZcl.selectNumberByNameAndClusterId( + db, + attribType, + clusterRef, + packageIds + ) + return attribData + ? { size: attribData.size * 8, isSigned: attribData.isSigned } + : { size: undefined, isSigned: undefined } +} + +/** + * Checks if the incoming integer is within it's attributes bound while handling signed and unsigned cases. + * @param {*} attribute + * @param {*} endpointAttribute + * @param {*} db + * @param {*} zapSessionId + * @param {*} clusterRef + * @returns boolean + */ +async function checkAttributeBoundsInteger( + attribute, + endpointAttribute, + db, + zapSessionId, + clusterRef +) { + const { size, isSigned } = await getIntegerAttributeSize( + db, + zapSessionId, + clusterRef, + attribute.type + ) + if (size === undefined || isSigned === undefined) { + return false + } + let { min, max } = await getBoundsInteger(attribute, size, isSigned) + let defaultValue = await getIntegerFromAttribute( + endpointAttribute.defaultValue, + size, + isSigned + ) return checkBoundsInteger(defaultValue, min, max) } @@ -202,3 +382,6 @@ exports.getBoundsInteger = getBoundsInteger exports.checkBoundsInteger = checkBoundsInteger exports.getBoundsFloat = getBoundsFloat exports.checkBoundsFloat = checkBoundsFloat +exports.unsignedToSignedInteger = unsignedToSignedInteger +exports.extractBigIntegerValue = extractBigIntegerValue +exports.getIntegerAttributeSize = getIntegerAttributeSize diff --git a/test/validation.test.js b/test/validation.test.js index f2aa576277..6d98cee8c2 100644 --- a/test/validation.test.js +++ b/test/validation.test.js @@ -54,6 +54,13 @@ test( async () => { let context = await zclLoader.loadZcl(db, env.builtinSilabsZclMetafile()) pkgId = context.packageId + sid = await testQuery.createSession( + db, + 'USER', + 'SESSION', + env.builtinSilabsZclMetafile(), + env.builtinTemplateMetafile() + ) }, timeout.medium() ) @@ -96,10 +103,86 @@ test( //float expect(validation.extractFloatValue('0.53') == 0.53).toBeTruthy() expect(validation.extractFloatValue('.53') == 0.53).toBeTruthy() + // big + expect( + validation.extractBigIntegerValue('0x7FFFFFFFFFFFFF') == + 36028797018963967n + ).toBeTruthy() }, timeout.medium() ) +test('getIntegerFromAttribute function', () => { + //Decimal over hex + //expect(validation.getIntegerFromAttribute('')) + //Convert hex +}) + +test('Test hex unsigned to signed conversion', () => { + // testing the available bit representation extreme values + + //8 bits + expect(validation.unsignedToSignedInteger(0x80, 8) == -128).toBeTruthy() + expect(validation.unsignedToSignedInteger(0x7f, 8) == 127).toBeTruthy() + + // 16 bits + expect(validation.unsignedToSignedInteger(0x8000, 16) == -32768).toBeTruthy() + expect(validation.unsignedToSignedInteger(0x7fff, 16) == 32767).toBeTruthy() + + // 24 bits + expect( + validation.unsignedToSignedInteger(0x800000, 24) == -8388608 + ).toBeTruthy() + expect( + validation.unsignedToSignedInteger(0x7fffff, 24) == 8388607 + ).toBeTruthy() + + // 32 bits + expect( + validation.unsignedToSignedInteger(0x80000000n, 32) == -2147483648n + ).toBeTruthy() + expect( + validation.unsignedToSignedInteger(0x7fffffffn, 32) == 2147483647n + ).toBeTruthy() + + // 40 bits + expect( + validation.unsignedToSignedInteger(0x8000000000n, 40) == -549755813888n + ).toBeTruthy() + expect( + validation.unsignedToSignedInteger(0x7fffffffffn, 40) == 549755813887n + ).toBeTruthy() + + // 48 bits + expect( + validation.unsignedToSignedInteger(0x800000000000n, 48) == -140737488355328n + ).toBeTruthy() + expect( + validation.unsignedToSignedInteger(0x7fffffffffffn, 48) == 140737488355327n + ).toBeTruthy() + + // 54 bits + expect( + validation.unsignedToSignedInteger(0x80000000000000n, 56) == + -36028797018963968n + ).toBeTruthy() + + expect( + validation.unsignedToSignedInteger(0x7fffffffffffffn, 56) == + 36028797018963967n + ).toBeTruthy() + + // 64 bits + expect( + validation.unsignedToSignedInteger(0x8000000000000000n, 64) == + -9223372036854775808n + ).toBeTruthy() + expect( + validation.unsignedToSignedInteger(0x7fffffffffffffffn, 64) == + 9223372036854775807n + ).toBeTruthy() +}) + test( 'Test int bounds', () => { @@ -110,6 +193,7 @@ test( expect(!validation.checkBoundsInteger(30, 'avaa', 2)).toBeTruthy() expect(!validation.checkBoundsInteger(30, 45, 50)).toBeTruthy() expect(validation.checkBoundsInteger('asdfa', 40, 50)).toBeFalsy() + expect(validation.checkBoundsInteger(-32, -128, 127)).toBeTruthy() //Float expect(validation.checkBoundsFloat(35.0, 25, 50.0)) @@ -140,56 +224,96 @@ test( test( 'Integer Test', - () => - queryZcl - .selectAttributesByClusterCodeAndManufacturerCode(db, pkgId, 3, null) - .then((attribute) => { - attribute = attribute.filter((e) => { - return e.code === 0 - })[0] - - //Test Constraints - let minMax = validation.getBoundsInteger(attribute) - expect(minMax.min == 0).toBeTruthy() - expect(minMax.max === 0xffff).toBeTruthy() - }), + async () => { + let attribute = + await queryZcl.selectAttributesByClusterCodeAndManufacturerCode( + db, + pkgId, + 3, + null + ) + attribute = attribute.filter((e) => { + return e.code === 0 + })[0] + + const { size, isSigned } = await validation.getIntegerAttributeSize( + db, + sid, + 3, + attribute.type + ) + //Test Constraints + let minMax = await validation.getBoundsInteger(attribute, size, isSigned) + expect(minMax.min == 0).toBeTruthy() + expect(minMax.max === 0xffff).toBeTruthy() + }, timeout.medium() ) test( 'validate Attribute Test', - () => { + async () => { let fakeEndpointAttribute = { defaultValue: '30', } let fakeAttribute = { - type: 'UINT16', + type: 'int16u', min: '0x0010', max: '50', } expect( - validation.validateSpecificAttribute(fakeEndpointAttribute, fakeAttribute) - .defaultValue.length == 0 + ( + await validation.validateSpecificAttribute( + fakeEndpointAttribute, + fakeAttribute, + db, + sid, + 3 + ) + ).defaultValue.length == 0 ).toBeTruthy() + // Check for if attribute is out of bounds. fakeEndpointAttribute.defaultValue = '60' expect( - validation.validateSpecificAttribute(fakeEndpointAttribute, fakeAttribute) - .defaultValue.length == 0 + ( + await validation.validateSpecificAttribute( + fakeEndpointAttribute, + fakeAttribute, + db, + sid, + 3 + ) + ).defaultValue.length == 0 ).toBeFalsy() + fakeEndpointAttribute.defaultValue = '5' expect( - validation.validateSpecificAttribute(fakeEndpointAttribute, fakeAttribute) - .defaultValue.length == 0 + ( + await validation.validateSpecificAttribute( + fakeEndpointAttribute, + fakeAttribute, + db, + sid, + 3 + ) + ).defaultValue.length == 0 ).toBeFalsy() //Check if attribute is actually a number fakeEndpointAttribute.defaultValue = 'xxxxxx' expect( - validation.validateSpecificAttribute(fakeEndpointAttribute, fakeAttribute) - .defaultValue.length == 0 + ( + await validation.validateSpecificAttribute( + fakeEndpointAttribute, + fakeAttribute, + db, + sid, + 3 + ) + ).defaultValue.length == 0 ).toBeFalsy() fakeAttribute = { @@ -202,30 +326,58 @@ test( defaultValue: '1.5', } expect( - validation.validateSpecificAttribute(fakeEndpointAttribute, fakeAttribute) - .defaultValue.length == 0 + ( + await validation.validateSpecificAttribute( + fakeEndpointAttribute, + fakeAttribute, + db, + sid, + 3 + ) + ).defaultValue.length == 0 ).toBeTruthy() //Check out of bounds. fakeEndpointAttribute = { defaultValue: '4.5', } expect( - validation.validateSpecificAttribute(fakeEndpointAttribute, fakeAttribute) - .defaultValue.length == 0 + ( + await validation.validateSpecificAttribute( + fakeEndpointAttribute, + fakeAttribute, + db, + sid, + 3 + ) + ).defaultValue.length == 0 ).toBeFalsy() fakeEndpointAttribute = { defaultValue: '.25', } expect( - validation.validateSpecificAttribute(fakeEndpointAttribute, fakeAttribute) - .defaultValue.length == 0 + ( + await validation.validateSpecificAttribute( + fakeEndpointAttribute, + fakeAttribute, + db, + sid, + 3 + ) + ).defaultValue.length == 0 ).toBeFalsy() //Check if attribute is actually a number fakeEndpointAttribute.defaultValue = 'xxxxxx' expect( - validation.validateSpecificAttribute(fakeEndpointAttribute, fakeAttribute) - .defaultValue.length == 0 + ( + await validation.validateSpecificAttribute( + fakeEndpointAttribute, + fakeAttribute, + db, + sid, + 3 + ) + ).defaultValue.length == 0 ).toBeFalsy() // Expect no issues with strings. @@ -236,9 +388,51 @@ test( defaultValue: '30adfadf', } expect( - validation.validateSpecificAttribute(fakeEndpointAttribute, fakeAttribute) - .defaultValue.length == 0 + ( + await validation.validateSpecificAttribute( + fakeEndpointAttribute, + fakeAttribute, + db, + sid, + 3 + ) + ).defaultValue.length == 0 + ).toBeTruthy() + + // check if handle signed numbers + let fakeEndpointAttributeInvalid = { + defaultValue: '549755813887', + } + let fakeEndpointAttributeValid = { + defaultValue: '549755813885', + } + let fakeSignedAttribute = { + type: 'int40s', + min: '0x8000000000', + max: '0x7FFFFFFFFE', + } + expect( + ( + await validation.validateSpecificAttribute( + fakeEndpointAttributeValid, + fakeSignedAttribute, + db, + sid, + 3 + ) + ).defaultValue.length == 0 ).toBeTruthy() + expect( + ( + await validation.validateSpecificAttribute( + fakeEndpointAttributeInvalid, + fakeSignedAttribute, + db, + sid, + 3 + ) + ).defaultValue.length == 0 + ).toBeFalsy() }, timeout.medium() )