From 16913627de9497a5d2f71edb710cec6e2eb9f890 Mon Sep 17 00:00:00 2001 From: Eli Bosley Date: Wed, 17 Sep 2025 13:59:57 -0400 Subject: [PATCH] fix: cleanup ini parser logic with better fallbacks (#1713) Added a new parser for INI boolean values, including functions to convert INI boolean strings to JavaScript booleans and handle malformed inputs. Introduced unit tests to validate the functionality of both `iniBooleanToJsBoolean` and `iniBooleanOrAutoToJsBoolean`, covering various valid, malformed, and edge case scenarios. Updated state parsers to utilize the new functions for improved reliability. --- .../utils/parsers/ini-boolean-parser.test.ts | 178 ++++++++++++++++++ .../core/utils/parsers/ini-boolean-parser.ts | 86 +++++++++ api/src/store/state-parsers/var.ts | 68 +++---- 3 files changed, 285 insertions(+), 47 deletions(-) create mode 100644 api/src/__test__/core/utils/parsers/ini-boolean-parser.test.ts create mode 100644 api/src/core/utils/parsers/ini-boolean-parser.ts diff --git a/api/src/__test__/core/utils/parsers/ini-boolean-parser.test.ts b/api/src/__test__/core/utils/parsers/ini-boolean-parser.test.ts new file mode 100644 index 000000000..4dcbfb7f1 --- /dev/null +++ b/api/src/__test__/core/utils/parsers/ini-boolean-parser.test.ts @@ -0,0 +1,178 @@ +import { describe, expect, test } from 'vitest'; + +import { + iniBooleanOrAutoToJsBoolean, + iniBooleanToJsBoolean, +} from '@app/core/utils/parsers/ini-boolean-parser.js'; + +describe('iniBooleanToJsBoolean', () => { + describe('valid boolean values', () => { + test('returns false for "no"', () => { + expect(iniBooleanToJsBoolean('no')).toBe(false); + }); + + test('returns false for "false"', () => { + expect(iniBooleanToJsBoolean('false')).toBe(false); + }); + + test('returns true for "yes"', () => { + expect(iniBooleanToJsBoolean('yes')).toBe(true); + }); + + test('returns true for "true"', () => { + expect(iniBooleanToJsBoolean('true')).toBe(true); + }); + }); + + describe('malformed values', () => { + test('handles "no*" as false', () => { + expect(iniBooleanToJsBoolean('no*')).toBe(false); + }); + + test('handles "yes*" as true', () => { + expect(iniBooleanToJsBoolean('yes*')).toBe(true); + }); + + test('handles "true*" as true', () => { + expect(iniBooleanToJsBoolean('true*')).toBe(true); + }); + + test('handles "false*" as false', () => { + expect(iniBooleanToJsBoolean('false*')).toBe(false); + }); + + test('returns undefined for "n0!" (cleans to "n" which is invalid)', () => { + expect(iniBooleanToJsBoolean('n0!')).toBe(undefined); + }); + + test('returns undefined for "y3s!" (cleans to "ys" which is invalid)', () => { + expect(iniBooleanToJsBoolean('y3s!')).toBe(undefined); + }); + + test('handles mixed case with extra chars "YES*" as true', () => { + expect(iniBooleanToJsBoolean('YES*')).toBe(true); + }); + + test('handles mixed case with extra chars "NO*" as false', () => { + expect(iniBooleanToJsBoolean('NO*')).toBe(false); + }); + }); + + describe('default values', () => { + test('returns default value for invalid input when provided', () => { + expect(iniBooleanToJsBoolean('invalid', true)).toBe(true); + expect(iniBooleanToJsBoolean('invalid', false)).toBe(false); + }); + + test('returns default value for empty string when provided', () => { + expect(iniBooleanToJsBoolean('', true)).toBe(true); + expect(iniBooleanToJsBoolean('', false)).toBe(false); + }); + }); + + describe('undefined fallback cases', () => { + test('returns undefined for invalid input without default', () => { + expect(iniBooleanToJsBoolean('invalid')).toBe(undefined); + }); + + test('returns undefined for empty string without default', () => { + expect(iniBooleanToJsBoolean('')).toBe(undefined); + }); + + test('returns undefined for numeric string without default', () => { + expect(iniBooleanToJsBoolean('123')).toBe(undefined); + }); + }); +}); + +describe('iniBooleanOrAutoToJsBoolean', () => { + describe('valid boolean values', () => { + test('returns false for "no"', () => { + expect(iniBooleanOrAutoToJsBoolean('no')).toBe(false); + }); + + test('returns false for "false"', () => { + expect(iniBooleanOrAutoToJsBoolean('false')).toBe(false); + }); + + test('returns true for "yes"', () => { + expect(iniBooleanOrAutoToJsBoolean('yes')).toBe(true); + }); + + test('returns true for "true"', () => { + expect(iniBooleanOrAutoToJsBoolean('true')).toBe(true); + }); + }); + + describe('auto value', () => { + test('returns null for "auto"', () => { + expect(iniBooleanOrAutoToJsBoolean('auto')).toBe(null); + }); + }); + + describe('malformed values', () => { + test('handles "no*" as false', () => { + expect(iniBooleanOrAutoToJsBoolean('no*')).toBe(false); + }); + + test('handles "yes*" as true', () => { + expect(iniBooleanOrAutoToJsBoolean('yes*')).toBe(true); + }); + + test('handles "auto*" as null', () => { + expect(iniBooleanOrAutoToJsBoolean('auto*')).toBe(null); + }); + + test('handles "true*" as true', () => { + expect(iniBooleanOrAutoToJsBoolean('true*')).toBe(true); + }); + + test('handles "false*" as false', () => { + expect(iniBooleanOrAutoToJsBoolean('false*')).toBe(false); + }); + + test('handles "n0!" as undefined fallback (cleans to "n" which is invalid)', () => { + expect(iniBooleanOrAutoToJsBoolean('n0!')).toBe(undefined); + }); + + test('handles "a1ut2o!" as null (removes non-alphabetic chars)', () => { + expect(iniBooleanOrAutoToJsBoolean('a1ut2o!')).toBe(null); + }); + + test('handles mixed case "AUTO*" as null', () => { + expect(iniBooleanOrAutoToJsBoolean('AUTO*')).toBe(null); + }); + }); + + describe('fallback behavior', () => { + test('returns undefined for completely invalid input', () => { + expect(iniBooleanOrAutoToJsBoolean('invalid123')).toBe(undefined); + }); + + test('returns undefined for empty string', () => { + expect(iniBooleanOrAutoToJsBoolean('')).toBe(undefined); + }); + + test('returns undefined for numeric string', () => { + expect(iniBooleanOrAutoToJsBoolean('123')).toBe(undefined); + }); + + test('returns undefined for special characters only', () => { + expect(iniBooleanOrAutoToJsBoolean('!@#$')).toBe(undefined); + }); + }); + + describe('edge cases', () => { + test('handles undefined gracefully', () => { + expect(iniBooleanOrAutoToJsBoolean(undefined as any)).toBe(undefined); + }); + + test('handles null gracefully', () => { + expect(iniBooleanOrAutoToJsBoolean(null as any)).toBe(undefined); + }); + + test('handles non-string input gracefully', () => { + expect(iniBooleanOrAutoToJsBoolean(123 as any)).toBe(undefined); + }); + }); +}); diff --git a/api/src/core/utils/parsers/ini-boolean-parser.ts b/api/src/core/utils/parsers/ini-boolean-parser.ts new file mode 100644 index 000000000..30b8d9857 --- /dev/null +++ b/api/src/core/utils/parsers/ini-boolean-parser.ts @@ -0,0 +1,86 @@ +import { type IniStringBoolean, type IniStringBooleanOrAuto } from '@app/core/types/ini.js'; + +/** + * Converts INI boolean string values to JavaScript boolean values. + * Handles malformed values by cleaning them of non-alphabetic characters. + * + * @param value - The string value to parse ("yes", "no", "true", "false", etc.) + * @returns boolean value or undefined if parsing fails + */ +export function iniBooleanToJsBoolean(value: string): boolean | undefined; +/** + * Converts INI boolean string values to JavaScript boolean values. + * Handles malformed values by cleaning them of non-alphabetic characters. + * + * @param value - The string value to parse ("yes", "no", "true", "false", etc.) + * @param defaultValue - Default value to return if parsing fails + * @returns boolean value or defaultValue if parsing fails (never undefined when defaultValue is provided) + */ +export function iniBooleanToJsBoolean(value: string, defaultValue: boolean): boolean; +export function iniBooleanToJsBoolean(value: string, defaultValue?: boolean): boolean | undefined { + if (value === 'no' || value === 'false') { + return false; + } + + if (value === 'yes' || value === 'true') { + return true; + } + + // Handle malformed values by cleaning them first + if (typeof value === 'string') { + const cleanValue = value.replace(/[^a-zA-Z]/g, '').toLowerCase(); + if (cleanValue === 'no' || cleanValue === 'false') { + return false; + } + if (cleanValue === 'yes' || cleanValue === 'true') { + return true; + } + } + + // Always return defaultValue when provided (even if undefined) + if (arguments.length >= 2) { + return defaultValue; + } + + // Return undefined only when no default was provided + return undefined; +} + +/** + * Converts INI boolean or auto string values to JavaScript boolean or null values. + * Handles malformed values by cleaning them of non-alphabetic characters. + * + * @param value - The string value to parse ("yes", "no", "auto", "true", "false", etc.) + * @returns boolean value for yes/no/true/false, null for auto, or undefined as fallback + */ +export const iniBooleanOrAutoToJsBoolean = ( + value: IniStringBooleanOrAuto | string +): boolean | null | undefined => { + // Handle auto first + if (value === 'auto') { + return null; + } + + // Try to parse as boolean + const boolResult = iniBooleanToJsBoolean(value as IniStringBoolean); + if (boolResult !== undefined) { + return boolResult; + } + + // Handle malformed values like "auto*" by extracting the base value + if (typeof value === 'string') { + const cleanValue = value.replace(/[^a-zA-Z]/g, '').toLowerCase(); + if (cleanValue === 'auto') { + return null; + } + if (cleanValue === 'no' || cleanValue === 'false') { + return false; + } + if (cleanValue === 'yes' || cleanValue === 'true') { + return true; + } + } + + // Return undefined as fallback instead of throwing to prevent API crash + return undefined; +}; diff --git a/api/src/store/state-parsers/var.ts b/api/src/store/state-parsers/var.ts index a0813d09b..d3b9b888f 100644 --- a/api/src/store/state-parsers/var.ts +++ b/api/src/store/state-parsers/var.ts @@ -1,6 +1,10 @@ import type { StateFileToIniParserMap } from '@app/store/types.js'; import { type IniStringBoolean, type IniStringBooleanOrAuto } from '@app/core/types/ini.js'; import { toNumber } from '@app/core/utils/index.js'; +import { + iniBooleanOrAutoToJsBoolean, + iniBooleanToJsBoolean, +} from '@app/core/utils/parsers/ini-boolean-parser.js'; import { ArrayState } from '@app/unraid-api/graph/resolvers/array/array.model.js'; import { DiskFsType } from '@app/unraid-api/graph/resolvers/disks/disks.model.js'; import { @@ -157,36 +161,6 @@ export type VarIni = { useUpnp: IniStringBoolean; }; -const iniBooleanToJsBoolean = (value: string, defaultValue?: boolean) => { - if (value === 'no' || value === 'false') { - return false; - } - - if (value === 'yes' || value === 'true') { - return true; - } - - if (defaultValue !== undefined) { - return defaultValue; - } - - throw new Error(`Value "${value}" is not false/true or no/yes.`); -}; - -const iniBooleanOrAutoToJsBoolean = (value: IniStringBooleanOrAuto) => { - try { - // Either it'll return true/false or throw - return iniBooleanToJsBoolean(value as IniStringBoolean); - } catch { - // Auto or null - if (value === 'auto') { - return null; - } - } - - throw new Error(`Value "${value as string}" is not auto/no/yes.`); -}; - const safeParseMdState = (mdState: string | undefined): ArrayState => { if (!mdState || typeof mdState !== 'string') { return ArrayState.STOPPED; @@ -210,7 +184,7 @@ export const parse: StateFileToIniParserMap['var'] = (iniFile) => { ...iniFile, defaultFsType: DiskFsType[iniFile.defaultFsType] || DiskFsType.XFS, mdState: safeParseMdState(iniFile.mdState), - bindMgt: iniBooleanOrAutoToJsBoolean(iniFile.bindMgt), + bindMgt: iniBooleanOrAutoToJsBoolean(iniFile.bindMgt) ?? null, cacheNumDevices: toNumber(iniFile.cacheNumDevices), cacheSbNumDisks: toNumber(iniFile.cacheSbNumDisks), configValid: iniBooleanToJsBoolean(iniFile.configValid, false), @@ -221,8 +195,8 @@ export const parse: StateFileToIniParserMap['var'] = (iniFile) => { fsCopyPrcnt: toNumber(iniFile.fsCopyPrcnt), fsNumMounted: toNumber(iniFile.fsNumMounted), fsNumUnmountable: toNumber(iniFile.fsNumUnmountable), - hideDotFiles: iniBooleanToJsBoolean(iniFile.hideDotFiles), - localMaster: iniBooleanToJsBoolean(iniFile.localMaster), + hideDotFiles: iniBooleanToJsBoolean(iniFile.hideDotFiles, false), + localMaster: iniBooleanToJsBoolean(iniFile.localMaster, false), maxArraysz: toNumber(iniFile.maxArraysz), maxCachesz: toNumber(iniFile.maxCachesz), mdNumDisabled: toNumber(iniFile.mdNumDisabled), @@ -254,34 +228,34 @@ export const parse: StateFileToIniParserMap['var'] = (iniFile) => { regState: RegistrationState[(iniFile.regCheck || iniFile.regTy || '').toUpperCase()] ?? RegistrationState.EGUID, - safeMode: iniBooleanToJsBoolean(iniFile.safeMode), - sbClean: iniBooleanToJsBoolean(iniFile.sbClean), + safeMode: iniBooleanToJsBoolean(iniFile.safeMode, false), + sbClean: iniBooleanToJsBoolean(iniFile.sbClean, false), sbEvents: toNumber(iniFile.sbEvents), sbNumDisks: toNumber(iniFile.sbNumDisks), sbSynced: toNumber(iniFile.sbSynced), sbSynced2: toNumber(iniFile.sbSynced2), sbSyncErrs: toNumber(iniFile.sbSyncErrs), - shareAvahiEnabled: iniBooleanToJsBoolean(iniFile.shareAvahiEnabled), - shareCacheEnabled: iniBooleanToJsBoolean(iniFile.shareCacheEnabled), + shareAvahiEnabled: iniBooleanToJsBoolean(iniFile.shareAvahiEnabled, false), + shareCacheEnabled: iniBooleanToJsBoolean(iniFile.shareCacheEnabled, false), shareCount: toNumber(iniFile.shareCount), - shareMoverActive: iniBooleanToJsBoolean(iniFile.shareMoverActive), - shareMoverLogging: iniBooleanToJsBoolean(iniFile.shareMoverLogging), + shareMoverActive: iniBooleanToJsBoolean(iniFile.shareMoverActive, false), + shareMoverLogging: iniBooleanToJsBoolean(iniFile.shareMoverLogging, false), shareNfsCount: toNumber(iniFile.shareNfsCount), - shareNfsEnabled: iniBooleanToJsBoolean(iniFile.shareNfsEnabled), + shareNfsEnabled: iniBooleanToJsBoolean(iniFile.shareNfsEnabled, false), shareSmbCount: toNumber(iniFile.shareSmbCount), shareSmbEnabled: ['yes', 'ads'].includes(iniFile.shareSmbEnabled), shareSmbMode: iniFile.shareSmbEnabled === 'ads' ? 'active-directory' : 'workgroup', shutdownTimeout: toNumber(iniFile.shutdownTimeout), spindownDelay: toNumber(iniFile.spindownDelay), - spinupGroups: iniBooleanToJsBoolean(iniFile.spinupGroups), - startArray: iniBooleanToJsBoolean(iniFile.startArray), + spinupGroups: iniBooleanToJsBoolean(iniFile.spinupGroups, false), + startArray: iniBooleanToJsBoolean(iniFile.startArray, false), sysArraySlots: toNumber(iniFile.sysArraySlots), sysCacheSlots: toNumber(iniFile.sysCacheSlots), sysFlashSlots: toNumber(iniFile.sysFlashSlots), - useNtp: iniBooleanToJsBoolean(iniFile.useNtp), - useSsh: iniBooleanToJsBoolean(iniFile.useSsh), - useSsl: iniBooleanOrAutoToJsBoolean(iniFile.useSsl), - useTelnet: iniBooleanToJsBoolean(iniFile.useTelnet), - useUpnp: iniBooleanToJsBoolean(iniFile.useUpnp), + useNtp: iniBooleanToJsBoolean(iniFile.useNtp, false), + useSsh: iniBooleanToJsBoolean(iniFile.useSsh, false), + useSsl: iniBooleanOrAutoToJsBoolean(iniFile.useSsl) ?? null, + useTelnet: iniBooleanToJsBoolean(iniFile.useTelnet, false), + useUpnp: iniBooleanToJsBoolean(iniFile.useUpnp, false), }; };