fix: correctly parse periods in share names from ini file (#1629)

Share names live as section headers in `emhttp/state/shares.ini`.
However, periods in ini section headers typically denote nested
hierarchy. This behavior is disabled in unraid's php setup, but cannot
be disabled/configured in the api's current ini parser.

So, we perform post-processing to reconcile nested objects into dot
notation.

Known issue: trailing and leading periods will not be treated as
distinct from shares without them.

i.e. `.share.name.` will conflict with `share.name`, `.share.name`, or
`share.name.`. The last of the conflicting names will be used/exposed.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- New Features
- Support shares with periods and emoji in their names across parsing
and listings.

- Bug Fixes
- Fixed configuration parsing for section names containing periods to
ensure affected shares load and display correctly.
  - Standardized reporting of encryption status for all shares.

- Tests
- Expanded coverage to validate parsing and retrieval of shares with
special characters (periods and emoji), ensuring consistent behavior
across modules.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Pujit Mehrotra
2025-08-27 15:14:07 -04:00
committed by GitHub
parent 674323fd87
commit 7d67a40433
6 changed files with 452 additions and 1 deletions

View File

@@ -65,4 +65,38 @@ color="yellow-on"
size="0"
free="9091184"
used="32831348"
luksStatus="0"
["system.with.periods"]
name="system.with.periods"
nameOrig="system.with.periods"
comment="system data with periods"
allocator="highwater"
splitLevel="1"
floor="0"
include=""
exclude=""
useCache="prefer"
cachePool="cache"
cow="auto"
color="yellow-on"
size="0"
free="9091184"
used="32831348"
luksStatus="0"
["system.with.🚀"]
name="system.with.🚀"
nameOrig="system.with.🚀"
comment="system data with 🚀"
allocator="highwater"
splitLevel="1"
floor="0"
include=""
exclude=""
useCache="prefer"
cachePool="cache"
cow="auto"
color="yellow-on"
size="0"
free="9091184"
used="32831348"
luksStatus="0"

View File

@@ -95,6 +95,48 @@ test('Returns both disk and user shares', async () => {
"type": "user",
"used": 33619300,
},
{
"allocator": "highwater",
"cachePool": "cache",
"color": "yellow-on",
"comment": "system data with periods",
"cow": "auto",
"exclude": [],
"floor": "0",
"free": 9309372,
"id": "system.with.periods",
"include": [],
"luksStatus": "0",
"name": "system.with.periods",
"nameOrig": "system.with.periods",
"nfs": {},
"size": 0,
"smb": {},
"splitLevel": "1",
"type": "user",
"used": 33619300,
},
{
"allocator": "highwater",
"cachePool": "cache",
"color": "yellow-on",
"comment": "system data with 🚀",
"cow": "auto",
"exclude": [],
"floor": "0",
"free": 9309372,
"id": "system.with.🚀",
"include": [],
"luksStatus": "0",
"name": "system.with.🚀",
"nameOrig": "system.with.🚀",
"nfs": {},
"size": 0,
"smb": {},
"splitLevel": "1",
"type": "user",
"used": 33619300,
},
],
}
`);
@@ -211,6 +253,48 @@ test('Returns shares by type', async () => {
"type": "user",
"used": 33619300,
},
{
"allocator": "highwater",
"cachePool": "cache",
"color": "yellow-on",
"comment": "system data with periods",
"cow": "auto",
"exclude": [],
"floor": "0",
"free": 9309372,
"id": "system.with.periods",
"include": [],
"luksStatus": "0",
"name": "system.with.periods",
"nameOrig": "system.with.periods",
"nfs": {},
"size": 0,
"smb": {},
"splitLevel": "1",
"type": "user",
"used": 33619300,
},
{
"allocator": "highwater",
"cachePool": "cache",
"color": "yellow-on",
"comment": "system data with 🚀",
"cow": "auto",
"exclude": [],
"floor": "0",
"free": 9309372,
"id": "system.with.🚀",
"include": [],
"luksStatus": "0",
"name": "system.with.🚀",
"nameOrig": "system.with.🚀",
"nfs": {},
"size": 0,
"smb": {},
"splitLevel": "1",
"type": "user",
"used": 33619300,
},
]
`);
expect(getShares('disk')).toMatchInlineSnapshot('null');

View File

@@ -1,5 +1,6 @@
import { expect, test } from 'vitest';
import { beforeEach, describe, expect, test, vi } from 'vitest';
import { parseConfig } from '@app/core/utils/misc/parse-config.js';
import { store } from '@app/store/index.js';
import { FileLoadStatus } from '@app/store/types.js';
@@ -446,6 +447,44 @@ test('After init returns values from cfg file for all fields', { timeout: 30000
"splitLevel": "1",
"used": 33619300,
},
{
"allocator": "highwater",
"cache": false,
"cachePool": "cache",
"color": "yellow-on",
"comment": "system data with periods",
"cow": "auto",
"exclude": [],
"floor": "0",
"free": 9309372,
"id": "system.with.periods",
"include": [],
"luksStatus": "0",
"name": "system.with.periods",
"nameOrig": "system.with.periods",
"size": 0,
"splitLevel": "1",
"used": 33619300,
},
{
"allocator": "highwater",
"cache": false,
"cachePool": "cache",
"color": "yellow-on",
"comment": "system data with 🚀",
"cow": "auto",
"exclude": [],
"floor": "0",
"free": 9309372,
"id": "system.with.🚀",
"include": [],
"luksStatus": "0",
"name": "system.with.🚀",
"nameOrig": "system.with.🚀",
"size": 0,
"splitLevel": "1",
"used": 33619300,
},
]
`);
expect(nfsShares).toMatchInlineSnapshot(`
@@ -1110,3 +1149,209 @@ test('After init returns values from cfg file for all fields', { timeout: 30000
}
`);
});
describe('Share parsing with periods in names', () => {
beforeEach(() => {
vi.clearAllMocks();
});
test('parseConfig handles periods in INI section names', () => {
const mockIniContent = `
["share.with.periods"]
name=share.with.periods
useCache=yes
include=
exclude=
[normal_share]
name=normal_share
useCache=no
include=
exclude=
`;
const result = parseConfig<any>({
file: mockIniContent,
type: 'ini',
});
// The result should now have properly flattened keys
expect(result).toHaveProperty('shareWithPeriods');
expect(result).toHaveProperty('normalShare');
expect(result.shareWithPeriods.name).toBe('share.with.periods');
expect(result.normalShare.name).toBe('normal_share');
});
test('shares parser handles periods in share names correctly', async () => {
const { parse } = await import('@app/store/state-parsers/shares.js');
// The parser expects an object where values are share configs
const mockSharesState = {
shareWithPeriods: {
name: 'share.with.periods',
free: '1000000',
used: '500000',
size: '1500000',
include: '',
exclude: '',
useCache: 'yes',
},
normalShare: {
name: 'normal_share',
free: '2000000',
used: '750000',
size: '2750000',
include: '',
exclude: '',
useCache: 'no',
},
} as any;
const result = parse(mockSharesState);
expect(result).toHaveLength(2);
const periodShare = result.find((s) => s.name === 'share.with.periods');
const normalShare = result.find((s) => s.name === 'normal_share');
expect(periodShare).toBeDefined();
expect(periodShare?.id).toBe('share.with.periods');
expect(periodShare?.name).toBe('share.with.periods');
expect(periodShare?.cache).toBe(true);
expect(normalShare).toBeDefined();
expect(normalShare?.id).toBe('normal_share');
expect(normalShare?.name).toBe('normal_share');
expect(normalShare?.cache).toBe(false);
});
test('SMB parser handles periods in share names', async () => {
const { parse } = await import('@app/store/state-parsers/smb.js');
const mockSmbState = {
'share.with.periods': {
export: 'e',
security: 'public',
writeList: '',
readList: '',
volsizelimit: '0',
},
normal_share: {
export: 'e',
security: 'private',
writeList: 'user1,user2',
readList: '',
volsizelimit: '1000',
},
} as any;
const result = parse(mockSmbState);
expect(result).toHaveLength(2);
const periodShare = result.find((s) => s.name === 'share.with.periods');
const normalShare = result.find((s) => s.name === 'normal_share');
expect(periodShare).toBeDefined();
expect(periodShare?.name).toBe('share.with.periods');
expect(periodShare?.enabled).toBe(true);
expect(normalShare).toBeDefined();
expect(normalShare?.name).toBe('normal_share');
expect(normalShare?.writeList).toEqual(['user1', 'user2']);
});
test('NFS parser handles periods in share names', async () => {
const { parse } = await import('@app/store/state-parsers/nfs.js');
const mockNfsState = {
'share.with.periods': {
export: 'e',
security: 'public',
writeList: '',
readList: 'user1',
hostList: '',
},
normal_share: {
export: 'd',
security: 'private',
writeList: 'user2',
readList: '',
hostList: '192.168.1.0/24',
},
} as any;
const result = parse(mockNfsState);
expect(result).toHaveLength(2);
const periodShare = result.find((s) => s.name === 'share.with.periods');
const normalShare = result.find((s) => s.name === 'normal_share');
expect(periodShare).toBeDefined();
expect(periodShare?.name).toBe('share.with.periods');
expect(periodShare?.enabled).toBe(true);
expect(periodShare?.readList).toEqual(['user1']);
expect(normalShare).toBeDefined();
expect(normalShare?.name).toBe('normal_share');
expect(normalShare?.enabled).toBe(false);
});
});
describe('Share lookup with periods in names', () => {
test('getShares finds user shares with periods in names', async () => {
// Mock the store state
const mockStore = await import('@app/store/index.js');
const mockEmhttpState = {
shares: [
{
id: 'share.with.periods',
name: 'share.with.periods',
cache: true,
free: 1000000,
used: 500000,
size: 1500000,
include: [],
exclude: [],
},
{
id: 'normal_share',
name: 'normal_share',
cache: false,
free: 2000000,
used: 750000,
size: 2750000,
include: [],
exclude: [],
},
],
smbShares: [
{ name: 'share.with.periods', enabled: true, security: 'public' },
{ name: 'normal_share', enabled: true, security: 'private' },
],
nfsShares: [
{ name: 'share.with.periods', enabled: false },
{ name: 'normal_share', enabled: true },
],
disks: [],
};
const gettersSpy = vi.spyOn(mockStore, 'getters', 'get').mockReturnValue({
emhttp: () => mockEmhttpState,
} as any);
const { getShares } = await import('@app/core/utils/shares/get-shares.js');
const periodShare = getShares('user', { name: 'share.with.periods' });
const normalShare = getShares('user', { name: 'normal_share' });
expect(periodShare).not.toBeNull();
expect(periodShare?.name).toBe('share.with.periods');
expect(periodShare?.type).toBe('user');
expect(normalShare).not.toBeNull();
expect(normalShare?.name).toBe('normal_share');
expect(normalShare?.type).toBe('user');
gettersSpy.mockRestore();
});
});

View File

@@ -92,6 +92,44 @@ test('Returns parsed state file', async () => {
"splitLevel": "1",
"used": 33619300,
},
{
"allocator": "highwater",
"cache": false,
"cachePool": "cache",
"color": "yellow-on",
"comment": "system data with periods",
"cow": "auto",
"exclude": [],
"floor": "0",
"free": 9309372,
"id": "system.with.periods",
"include": [],
"luksStatus": "0",
"name": "system.with.periods",
"nameOrig": "system.with.periods",
"size": 0,
"splitLevel": "1",
"used": 33619300,
},
{
"allocator": "highwater",
"cache": false,
"cachePool": "cache",
"color": "yellow-on",
"comment": "system data with 🚀",
"cow": "auto",
"exclude": [],
"floor": "0",
"free": 9309372,
"id": "system.with.🚀",
"include": [],
"luksStatus": "0",
"name": "system.with.🚀",
"nameOrig": "system.with.🚀",
"size": 0,
"splitLevel": "1",
"used": 33619300,
},
]
`);
});

View File

@@ -23,6 +23,54 @@ type OptionsWithLoadedFile = {
type: ConfigType;
};
/**
* Flattens nested objects that were incorrectly created by periods in INI section names.
* For example: { system: { with: { periods: {...} } } } -> { "system.with.periods": {...} }
*/
const flattenPeriodSections = (obj: Record<string, any>, prefix = ''): Record<string, any> => {
const result: Record<string, any> = {};
const isNestedObject = (value: unknown) =>
Boolean(value && typeof value === 'object' && !Array.isArray(value));
// prevent prototype pollution/injection
const isUnsafeKey = (k: string) => k === '__proto__' || k === 'prototype' || k === 'constructor';
for (const [key, value] of Object.entries(obj)) {
if (isUnsafeKey(key)) continue;
const fullKey = prefix ? `${prefix}.${key}` : key;
if (!isNestedObject(value)) {
result[fullKey] = value;
continue;
}
const section = {};
const nestedObjs = {};
let hasSectionProps = false;
for (const [propKey, propValue] of Object.entries(value)) {
if (isUnsafeKey(propKey)) continue;
if (isNestedObject(propValue)) {
nestedObjs[propKey] = propValue;
} else {
section[propKey] = propValue;
hasSectionProps = true;
}
}
// Process direct properties first to maintain order
if (hasSectionProps) {
result[fullKey] = section;
}
// Then process nested objects
if (Object.keys(nestedObjs).length > 0) {
Object.assign(result, flattenPeriodSections(nestedObjs, fullKey));
}
}
return result;
};
/**
* Converts the following
* ```
@@ -127,6 +175,8 @@ export const parseConfig = <T extends Record<string, any>>(
let data: Record<string, any>;
try {
data = parseIni(fileContents);
// Fix nested objects created by periods in section names
data = flattenPeriodSections(data);
} catch (error) {
throw new AppError(
`Failed to parse config file: ${error instanceof Error ? error.message : String(error)}`