From 1ae6c3f05c1c5fa1177dfa2e2fe81284da773813 Mon Sep 17 00:00:00 2001 From: Tom Butcher Date: Sat, 7 Nov 2026 18:52:45 +0000 Subject: [PATCH] Enhance database object handling with ObjectId normalization and error handling - Introduced `refId`, `normalizeFilter`, and `coerceObjectIds` functions in `objectid.js` to standardize ObjectId handling across the database operations. - Updated `listObjects`, `getObject`, `editObject`, and `newObject` functions to utilize the new normalization methods, improving data integrity and error management. - Enhanced error handling in socket event handlers to ensure proper feedback on operation failures. - Refactored `expandObjectIds` and related utility functions to streamline ObjectId processing and improve code clarity. --- src/database/database.js | 32 ++-- src/database/filter.js | 6 +- src/database/nats.js | 7 +- src/database/objectid.js | 169 ++++++++++++++++++ .../schemas/inventory/stockevent.schema.js | 7 +- src/database/utils.js | 57 +++--- src/socket/sockethost.js | 92 ++++++---- src/socket/socketmanager.js | 7 + src/socket/socketuser.js | 11 +- src/socket/stringidparser.js | 30 ++++ 10 files changed, 337 insertions(+), 81 deletions(-) create mode 100644 src/database/objectid.js create mode 100644 src/socket/stringidparser.js diff --git a/src/database/database.js b/src/database/database.js index 07d5548..03992cc 100644 --- a/src/database/database.js +++ b/src/database/database.js @@ -14,6 +14,7 @@ import { loadConfig } from '../config.js'; import { formatTraceData, getQueryToCacheKey } from '../utils.js'; import { redisServer } from './redis.js'; import { auditLogModel } from './schemas/management/auditlog.schema.js'; +import { coerceObjectIds, normalizeFilter, refId } from './objectid.js'; const config = loadConfig(); @@ -153,17 +154,10 @@ export const listObjects = async ({ if (!sort || sort === '') { sort = 'createdAt'; } - // Translate parent._id to parent for Mongoose - if (filter['parent._id']) { - filter.parent = filter['parent._id']; - delete filter['parent._id']; - } - // Translate owner._id to owner for Mongoose - if (filter['owner._id']) { - filter.owner = filter['owner._id']; - delete filter['owner._id']; - } + // `{ job: { _id } }` and socket.io's `{ buffer: ... }` ObjectId form both + // need to be the raw id before Mongoose casts the query. + filter = normalizeFilter(filter); // Use find with population and filter let query = model.find(filter).sort({ [sort]: sortOrder }); @@ -217,6 +211,12 @@ export const getObject = async ({ cached = false }) => { try { + id = refId(id); + if (typeof id !== 'string' || id.length === 0) { + logger.error('Invalid object id:', formatTraceData(id)); + return undefined; + } + logger.trace( `Getting object: ${formatTraceData({ model, @@ -275,7 +275,6 @@ export const getObject = async ({ return finalResult; } catch (error) { logger.error('An error retreiving object:', error.message); - throw error; return undefined; } }; @@ -521,6 +520,12 @@ export const editObject = async ({ recalculate = true }) => { try { + id = refId(id); + if (typeof id !== 'string' || id.length === 0) { + return { error: 'Invalid object id.', code: 400 }; + } + updateData = coerceObjectIds(model, updateData); + // Determine parentType from model name const parentType = model.modelName ? model.modelName : 'unknown'; // Fetch the and update object @@ -620,6 +625,7 @@ export const newObject = async ({ }) => { try { const parentType = model.modelName ? model.modelName : 'unknown'; + newData = coerceObjectIds(model, newData); const result = await model.create(newData); if (!result || result.length === 0) { @@ -663,6 +669,10 @@ export const deleteObject = async ({ ownerType = undefined }) => { try { + id = refId(id); + if (typeof id !== 'string' || id.length === 0) { + return { error: 'Invalid object id.', code: 400 }; + } const parentType = model.modelName ? model.modelName : 'unknown'; // Delete the object const result = await model.findByIdAndDelete(id); diff --git a/src/database/filter.js b/src/database/filter.js index 80a30c6..1556c8a 100644 --- a/src/database/filter.js +++ b/src/database/filter.js @@ -1,4 +1,5 @@ import mongoose from 'mongoose'; +import { refId } from './objectid.js'; import log4js from 'log4js'; import { loadConfig } from '../config.js'; @@ -810,7 +811,10 @@ async function parseFilter(property, value, model = null) { const isBoolField = isBooleanField(property, model); if (value?._id !== undefined && value?._id !== null) { - return { [filterProperty]: new mongoose.Types.ObjectId(value._id) }; + const id = refId(value._id); + if (typeof id === 'string' && /^[a-fA-F\d]{24}$/.test(id)) { + return { [filterProperty]: new mongoose.Types.ObjectId(id) }; + } } // Non-string values pass through; boolean fields only accept actual booleans. diff --git a/src/database/nats.js b/src/database/nats.js index 8c4d3ee..bd5e63d 100644 --- a/src/database/nats.js +++ b/src/database/nats.js @@ -2,6 +2,7 @@ import { connect } from '@nats-io/transport-node'; import log4js from 'log4js'; import { loadConfig } from '../config.js'; import { formatTraceData } from '../utils.js'; +import { stringifyIds } from './objectid.js'; const config = loadConfig(); const logger = log4js.getLogger('Nats'); @@ -69,7 +70,7 @@ class NatsServer { async publish(subject, data) { const client = await this.getClient(); - const payload = typeof data === 'string' ? data : JSON.stringify(data); + const payload = typeof data === 'string' ? data : JSON.stringify(stringifyIds(data)); try { client.publish(subject, this.textEncoder.encode(payload)); @@ -83,7 +84,7 @@ class NatsServer { async request(subject, data, timeout = 30000) { const client = await this.getClient(); - const payload = typeof data === 'string' ? data : JSON.stringify(data); + const payload = typeof data === 'string' ? data : JSON.stringify(stringifyIds(data)); try { const response = await client.request( @@ -197,7 +198,7 @@ class NatsServer { const responsePayload = typeof response === 'string' ? response - : JSON.stringify(response); + : JSON.stringify(stringifyIds(response)); msg.respond(this.textEncoder.encode(responsePayload)); } catch (err) { logger.error( diff --git a/src/database/objectid.js b/src/database/objectid.js new file mode 100644 index 0000000..d31e6cd --- /dev/null +++ b/src/database/objectid.js @@ -0,0 +1,169 @@ +const PURE_REF_KEYS = new Set(['_id', 'id', 'buffer', '$oid']); + +function bytesOf(value) { + if (value == null) return null; + if (typeof Buffer !== 'undefined' && Buffer.isBuffer(value)) { + return value.length === 12 ? value : null; + } + if (value instanceof Uint8Array) { + return value.byteLength === 12 ? Buffer.from(value) : null; + } + if ( + value && + value.type === 'Buffer' && + Array.isArray(value.data) && + value.data.length === 12 + ) { + return Buffer.from(value.data); + } + if (typeof value !== 'object' || Array.isArray(value) || value instanceof Date) { + return null; + } + + const keys = Object.keys(value); + if (keys.length !== 12) return null; + + const bytes = Buffer.alloc(12); + for (let index = 0; index < 12; index += 1) { + if (!Object.prototype.hasOwnProperty.call(value, index)) return null; + const byte = Number(value[index]); + if (!Number.isInteger(byte) || byte < 0 || byte > 255) return null; + bytes[index] = byte; + } + return bytes; +} + +// Socket.io's binary encoder copies BSON ObjectIds into `{ buffer: <12 bytes> }` +// whenever the same packet also contains a Buffer. Accept that shape, hex +// strings, `{ _id }`, and `{ $oid }` as the same id. +export function refId(value) { + if (value == null || value === '') return null; + if (typeof value === 'string') return value; + if (typeof value === 'object' && typeof value.toHexString === 'function') { + return value.toHexString(); + } + if (typeof value !== 'object' || Array.isArray(value) || value instanceof Date) { + return null; + } + if (typeof value.$oid === 'string') return refId(value.$oid); + + const keys = Object.keys(value); + if ( + keys.length > 0 && + keys.every(key => key === 'buffer' || key === 'id' || key === 'type' || key === 'data') + ) { + const bytes = bytesOf(value.buffer) || bytesOf(value.id) || bytesOf(value); + if (bytes) return bytes.toString('hex'); + } + + if (value._id != null) return refId(value._id); + return null; +} + +const BINARY_TYPES = new Set(['Buffer', 'Uint8Array', 'ArrayBuffer', 'Blob', 'File']); + +function isBinaryPayload(value) { + if (value == null || typeof value !== 'object') return false; + if (typeof Buffer !== 'undefined' && Buffer.isBuffer(value)) return true; + if (value instanceof Uint8Array || value instanceof ArrayBuffer) return true; + const name = value.constructor?.name; + return BINARY_TYPES.has(name); +} + +// Object ids go out as 24-character hex strings. Binary file payloads stay binary. +export function stringifyIds(value) { + if (value == null) return value; + if (typeof value === 'string' || typeof value === 'number' || typeof value === 'boolean') { + return value; + } + if (typeof value.toHexString === 'function') return value.toHexString(); + if (value instanceof Date || typeof value === 'function') return value; + if (isBinaryPayload(value)) return value; + if (Array.isArray(value)) return value.map(item => stringifyIds(item)); + if (typeof value === 'object') { + if (isPureRef(value)) { + const id = refId(value); + if (typeof id === 'string') return id; + } + const copy = {}; + for (const [key, child] of Object.entries(value)) { + copy[key] = stringifyIds(child); + } + return copy; + } + return value; +} + +export function isPureRef(value) { + if (value == null || typeof value !== 'object') return false; + if (typeof value.toHexString === 'function') return true; + if (Array.isArray(value) || value instanceof Date) return false; + const keys = Object.keys(value); + return keys.length > 0 && keys.every(key => PURE_REF_KEYS.has(key)) && refId(value) != null; +} + +function normalizeOperatorValue(value) { + const next = { ...value }; + for (const operator of ['$in', '$nin']) { + if (Array.isArray(next[operator])) { + next[operator] = next[operator].map(item => refId(item) ?? item); + } + } + if (next.$eq != null) next.$eq = refId(next.$eq) ?? next.$eq; + if (next.$ne != null) next.$ne = refId(next.$ne) ?? next.$ne; + return next; +} + +function normalizeFilterValue(value) { + if (Array.isArray(value)) return value.map(item => refId(item) ?? item); + if (isPureRef(value)) return refId(value); + if ( + value && + typeof value === 'object' && + !(value instanceof Date) && + ('$in' in value || '$nin' in value || '$eq' in value || '$ne' in value) + ) { + return normalizeOperatorValue(value); + } + return value; +} + +export function normalizeFilter(filter) { + if (!filter || typeof filter !== 'object' || Array.isArray(filter)) { + return {}; + } + + const normalized = {}; + for (const [key, value] of Object.entries(filter)) { + const targetKey = key.endsWith('._id') ? key.slice(0, -4) : key; + normalized[targetKey] = normalizeFilterValue(value); + } + return normalized; +} + +function isObjectIdSchemaPath(schemaType) { + if (!schemaType) return false; + if (schemaType.instance === 'ObjectId' || schemaType.instance === 'ObjectID') { + return true; + } + const caster = + schemaType.caster ?? schemaType.$embeddedSchemaType ?? schemaType.embeddedSchemaType; + return caster?.instance === 'ObjectId' || caster?.instance === 'ObjectID'; +} + +export function coerceObjectIds(model, data) { + if (!data || typeof data !== 'object' || Array.isArray(data) || !model?.schema) { + return data; + } + + const coerced = { ...data }; + for (const [key, value] of Object.entries(data)) { + if (!isObjectIdSchemaPath(model.schema.path(key)) || value == null) continue; + if (Array.isArray(value)) { + coerced[key] = value.map(item => refId(item) ?? item); + } else { + coerced[key] = refId(value) ?? value; + } + } + return coerced; +} diff --git a/src/database/schemas/inventory/stockevent.schema.js b/src/database/schemas/inventory/stockevent.schema.js index 4d886fb..36cc569 100644 --- a/src/database/schemas/inventory/stockevent.schema.js +++ b/src/database/schemas/inventory/stockevent.schema.js @@ -6,6 +6,7 @@ import { aggregateRollups, aggregateRollupsHistory, } from '../../database.js'; +import { refId } from '../../objectid.js'; const { Schema } = mongoose; const parentStockModelNames = { @@ -60,10 +61,10 @@ const buildParentState = (parentType, parentStock, currentAmount, startingAmount }; const getStockEventTotal = async (parentId, parentType) => { - if (!parentId) return null; + const id = refId(parentId); + if (typeof id !== 'string' || !/^[a-fA-F\d]{24}$/.test(id)) return null; - const objectId = - parentId instanceof mongoose.Types.ObjectId ? parentId : new mongoose.Types.ObjectId(parentId); + const objectId = new mongoose.Types.ObjectId(id); const [result] = await mongoose .model('stockEvent') diff --git a/src/database/utils.js b/src/database/utils.js index 0026f95..162c746 100644 --- a/src/database/utils.js +++ b/src/database/utils.js @@ -1,4 +1,5 @@ import { ObjectId } from 'mongodb'; +import { refId } from './objectid.js'; import { auditLogModel } from './schemas/management/auditlog.schema.js'; import { notificationModel } from './schemas/misc/notification.schema.js'; import { userNotifierModel } from './schemas/misc/usernotifier.schema.js'; @@ -701,48 +702,54 @@ function flatternObjectIds(object) { return result; } -function expandObjectIds(input) { - // Helper to check if a value is an ObjectId or a 24-char hex string - function isObjectId(val) { - // Check for MongoDB ObjectId instance - if (val instanceof ObjectId) return true; - // Check for exactly 24 hex characters (no special characters) - if (typeof val === 'string' && /^[a-fA-F\d]{24}$/.test(val)) return true; - return false; - } +function isIdValue(val) { + if (val instanceof ObjectId) return true; + if (val && typeof val === 'object' && typeof val.toHexString === 'function') return true; + if (typeof val === 'string' && /^[a-fA-F\d]{24}$/.test(val)) return true; + return false; +} - // Recursive function +function idString(val) { + if (typeof val === 'string') return val; + if (val && typeof val.toHexString === 'function') return val.toHexString(); + return refId(val); +} + +function expandObjectIds(input) { + // Recursive function. Ids are always hex strings, never ObjectId buffers. function expand(value) { + if (isIdValue(value)) { + return { _id: idString(value) }; + } if (Array.isArray(value)) { return value.map(expand); - } else if ( - value && - typeof value === 'object' && - !(value instanceof ObjectId) - ) { - var result = {}; + } + if (value && typeof value === 'object' && !(value instanceof Date)) { + const result = {}; for (const [key, val] of Object.entries(value)) { if (key === '_id') { - // Do not expand keys that are already named _id - result[key] = val; - } else if (isObjectId(val)) { - result[key] = { _id: val }; + result[key] = isIdValue(val) ? idString(val) : val; + } else if (isIdValue(val)) { + result[key] = { _id: idString(val) }; } else if (Array.isArray(val)) { result[key] = val.map(expand); } else if (val instanceof Date) { result[key] = val; } else if (val && typeof val === 'object') { - result[key] = expand(val); + const nestedId = refId(val); + const keys = Object.keys(val); + const bufferOnly = + nestedId && + keys.length > 0 && + keys.every(item => item === 'buffer' || item === 'id' || item === '$oid'); + result[key] = bufferOnly ? { _id: nestedId } : expand(val); } else { result[key] = val; } } return result; - } else if (isObjectId(value)) { - return { _id: value }; - } else { - return value; } + return value; } return expand(input); diff --git a/src/socket/sockethost.js b/src/socket/sockethost.js index 51fb095..7985c76 100644 --- a/src/socket/sockethost.js +++ b/src/socket/sockethost.js @@ -163,51 +163,71 @@ export class SocketHost { } async handleNewObject(data, callback) { - const object = await newObject({ - model: getModelByName(data.objectType), - newData: data.newData, - owner: this.host, - ownerType: 'host' - }); - callback(object); + try { + const object = await newObject({ + model: getModelByName(data.objectType), + newData: data.newData, + owner: this.host, + ownerType: 'host' + }); + callback(object); + } catch (error) { + logger.error('newObject failed:', error); + callback({ error: error.message }); + } } async handleEditObject(data, callback) { - const object = await editObject({ - model: getModelByName(data.objectType), - id: data._id, - updateData: data.updateData, - populate: data.populate, - owner: this.host, - ownerType: 'host', - auditLog: data?.auditLog, - notify: data?.notify - }); - callback(object); + try { + const object = await editObject({ + model: getModelByName(data.objectType), + id: data._id, + updateData: data.updateData, + populate: data.populate, + owner: this.host, + ownerType: 'host', + auditLog: data?.auditLog, + notify: data?.notify + }); + callback(object); + } catch (error) { + logger.error('editObject failed:', error); + callback({ error: error.message }); + } } async handleGetObject(data, callback) { - const object = await getObject({ - model: getModelByName(data.objectType), - id: data._id, - cached: true, - populate: data.populate - }); - callback(object); + try { + const object = await getObject({ + model: getModelByName(data.objectType), + id: data._id, + cached: true, + populate: data.populate + }); + callback(object); + } catch (error) { + logger.error('getObject failed:', error); + callback({ error: error.message }); + } } async handleListObjects(data, callback) { - const object = await listObjects({ - model: getModelByName(data.objectType), - id: data._id, - cached: data?.cached, - populate: data?.populate, - filter: data?.filter, - project: data?.project, - sort: data?.sort, - order: data?.order - }); - callback(object); + try { + const object = await listObjects({ + model: getModelByName(data.objectType), + id: data._id, + cached: data?.cached, + populate: data?.populate, + filter: data?.filter, + project: data?.project, + sort: data?.sort, + order: data?.order + }); + callback(object); + } catch (error) { + logger.error('listObjects failed:', error); + callback({ error: error.message }); + } } async handleObjectEventEvent(data) { diff --git a/src/socket/socketmanager.js b/src/socket/socketmanager.js index 608d072..ec6c06e 100644 --- a/src/socket/socketmanager.js +++ b/src/socket/socketmanager.js @@ -6,6 +6,7 @@ import { loadConfig } from '../config.js'; import { SocketUser } from './socketuser.js'; import { UpdateManager } from '../updates/updatemanager.js'; import { SocketHost } from './sockethost.js'; +import { stringIdParser, useStringIds } from './stringidparser.js'; const config = loadConfig(); @@ -21,6 +22,7 @@ export class SocketManager { // Create Socket.IO server - CORS applies to HTTP long-polling transport const allowedOrigins = config.server.corsOrigins || ['*']; const io = new Server(server, { + parser: stringIdParser, maxHttpBufferSize: 1e8, cors: { origin: (origin, callback) => { @@ -37,6 +39,11 @@ export class SocketManager { }); // Handle user connections + io.use((socket, next) => { + useStringIds(socket); + next(); + }); + io.on('connection', async socket => { const authType = socket.handshake?.auth?.type; if (authType == 'user') { diff --git a/src/socket/socketuser.js b/src/socket/socketuser.js index 977062b..b5e8b4d 100644 --- a/src/socket/socketuser.js +++ b/src/socket/socketuser.js @@ -128,7 +128,11 @@ export class SocketUser { } else { logger.warn('User is not authenticated.'); } - callback(result); + if (typeof callback === 'function') { + callback(result); + } else { + this.socket.emit('authenticate_result', result); + } } } @@ -384,7 +388,10 @@ export class SocketUser { return; } - logger.info('Cancelling scan sessions on user disconnect:', sessions.length); + logger.info( + 'Cancelling scan sessions on user disconnect:', + sessions.length + ); await Promise.all( sessions.map(([sessionId, hostId]) => diff --git a/src/socket/stringidparser.js b/src/socket/stringidparser.js new file mode 100644 index 0000000..c60dd83 --- /dev/null +++ b/src/socket/stringidparser.js @@ -0,0 +1,30 @@ +import { createRequire } from 'node:module'; +import { stringifyIds } from '../database/objectid.js'; + +const require = createRequire(import.meta.url); +const { Encoder, Decoder } = createRequire(require.resolve('socket.io'))( + 'socket.io-parser' +); + +class StringIdEncoder extends Encoder { + encode(packet) { + if (packet?.data == null) return super.encode(packet); + return super.encode({ ...packet, data: stringifyIds(packet.data) }); + } +} + +export const stringIdParser = { + Encoder: StringIdEncoder, + Decoder +}; + +export function useStringIds(socket) { + socket.use((packet, next) => { + for (let index = 1; index < packet.length; index += 1) { + if (typeof packet[index] !== 'function') { + packet[index] = stringifyIds(packet[index]); + } + } + next(); + }); +}