mirror of
https://github.com/usebruno/bruno.git
synced 2026-06-15 03:41:28 +00:00
fix(apispec): prevent crash on non-array specs and fix Windows spec listing (BRU-3556) (#8255)
This commit is contained in:
@@ -5,6 +5,7 @@ import { useTheme } from 'providers/Theme';
|
||||
import { openApiSpec } from 'providers/ReduxStore/slices/apiSpec';
|
||||
import ApiSpecItem from './ApiSpecItem';
|
||||
import StyledWrapper from './StyledWrapper';
|
||||
import { matchLoadedApiSpecs } from './matchLoadedApiSpecs';
|
||||
import toast from 'react-hot-toast';
|
||||
|
||||
const LinkStyle = styled.span`
|
||||
@@ -22,13 +23,11 @@ const ApiSpecs = () => {
|
||||
const apiSpecs = React.useMemo(() => {
|
||||
if (!activeWorkspace) return [];
|
||||
|
||||
const workspaceApiSpecs = activeWorkspace.apiSpecs || [];
|
||||
const workspaceApiSpecs = Array.isArray(activeWorkspace.apiSpecs) ? activeWorkspace.apiSpecs : [];
|
||||
|
||||
// Map workspace API specs to loaded API specs from Redux store
|
||||
return workspaceApiSpecs.map((ws) => {
|
||||
const loadedApiSpec = allApiSpecs.find((apiSpec) => apiSpec.pathname === ws.path);
|
||||
return loadedApiSpec;
|
||||
}).filter(Boolean);
|
||||
// Pair workspace API specs to loaded specs in redux, matching by normalized
|
||||
// path so Windows (backslash) and stored (forward-slash) paths line up.
|
||||
return matchLoadedApiSpecs(workspaceApiSpecs, allApiSpecs);
|
||||
}, [allApiSpecs, activeWorkspace, activeWorkspace?.apiSpecs]);
|
||||
|
||||
const handleOpenApiSpec = () => {
|
||||
|
||||
@@ -0,0 +1,29 @@
|
||||
import { normalizePath } from 'utils/common/path';
|
||||
|
||||
/**
|
||||
* Pairs each workspace API spec entry (from workspace.yml) with its loaded
|
||||
* counterpart in the redux store, matching by normalized (posixified) path.
|
||||
*
|
||||
* The two paths are derived independently: the workspace entry's path is stored
|
||||
* posixified (forward slashes) in workspace.yml, while the loaded spec's pathname
|
||||
* comes from the file watcher in native form (backslashes on Windows). A raw
|
||||
* `===` compare therefore fails on Windows (`C:/ws/api.yaml` !== `C:\ws\api.yaml`),
|
||||
* which hides the spec from the sidebar until a workspace switch. Normalizing both
|
||||
* sides makes them match on Windows while being a no-op on macOS/Linux.
|
||||
*
|
||||
* @param {Array} workspaceApiSpecs - spec entries from the active workspace (each has `path`)
|
||||
* @param {Array} allApiSpecs - loaded specs in redux (each has `pathname`)
|
||||
* @returns {Array} loaded specs that correspond to the workspace entries
|
||||
*/
|
||||
export const matchLoadedApiSpecs = (workspaceApiSpecs, allApiSpecs) => {
|
||||
if (!Array.isArray(workspaceApiSpecs)) return [];
|
||||
const loadedApiSpecs = Array.isArray(allApiSpecs) ? allApiSpecs : [];
|
||||
|
||||
return workspaceApiSpecs
|
||||
.map((ws) => {
|
||||
const wsPath = normalizePath(ws?.path);
|
||||
if (!wsPath) return undefined;
|
||||
return loadedApiSpecs.find((apiSpec) => normalizePath(apiSpec?.pathname) === wsPath);
|
||||
})
|
||||
.filter(Boolean);
|
||||
};
|
||||
@@ -0,0 +1,55 @@
|
||||
import { matchLoadedApiSpecs } from './matchLoadedApiSpecs';
|
||||
|
||||
const loaded = (pathname, extra = {}) => ({ uid: pathname, pathname, ...extra });
|
||||
|
||||
describe('matchLoadedApiSpecs', () => {
|
||||
it('matches workspace specs to loaded specs by identical path (macOS/Linux)', () => {
|
||||
const ws = [{ name: 'a', path: '/Users/me/ws/a.yaml' }];
|
||||
const all = [loaded('/Users/me/ws/a.yaml'), loaded('/Users/me/ws/other.yaml')];
|
||||
expect(matchLoadedApiSpecs(ws, all)).toEqual([loaded('/Users/me/ws/a.yaml')]);
|
||||
});
|
||||
|
||||
it('matches when paths differ only by separator (Windows: backslash vs forward-slash)', () => {
|
||||
// workspace.yml stores forward-slash; the file watcher reports native backslash.
|
||||
const ws = [{ name: 'a', path: 'C:/Users/qa/Downloads/test.yaml' }];
|
||||
const all = [loaded('C:\\Users\\qa\\Downloads\\test.yaml')];
|
||||
const result = matchLoadedApiSpecs(ws, all);
|
||||
expect(result).toHaveLength(1);
|
||||
expect(result[0].pathname).toBe('C:\\Users\\qa\\Downloads\\test.yaml');
|
||||
});
|
||||
|
||||
it('matches mixed separators within a single path', () => {
|
||||
const ws = [{ name: 'a', path: 'C:/ws/sub/a.yaml' }];
|
||||
const all = [loaded('C:\\ws/sub\\a.yaml')];
|
||||
expect(matchLoadedApiSpecs(ws, all)).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('preserves workspace order and drops entries with no loaded counterpart', () => {
|
||||
const ws = [
|
||||
{ name: 'a', path: 'C:/ws/a.yaml' },
|
||||
{ name: 'missing', path: 'C:/ws/missing.yaml' },
|
||||
{ name: 'b', path: 'C:/ws/b.yaml' }
|
||||
];
|
||||
const all = [loaded('C:\\ws\\b.yaml'), loaded('C:\\ws\\a.yaml')];
|
||||
const result = matchLoadedApiSpecs(ws, all);
|
||||
expect(result.map((s) => s.pathname)).toEqual(['C:\\ws\\a.yaml', 'C:\\ws\\b.yaml']);
|
||||
});
|
||||
|
||||
it('does not match entries with a missing/empty path (no empty-string false positive)', () => {
|
||||
const ws = [{ name: 'noPath' }, { name: 'emptyPath', path: '' }];
|
||||
const all = [loaded(undefined), loaded('')];
|
||||
expect(matchLoadedApiSpecs(ws, all)).toEqual([]);
|
||||
});
|
||||
|
||||
it('returns [] when workspaceApiSpecs is not an array', () => {
|
||||
expect(matchLoadedApiSpecs(undefined, [])).toEqual([]);
|
||||
expect(matchLoadedApiSpecs({ broken: 'map' }, [])).toEqual([]);
|
||||
expect(matchLoadedApiSpecs('string', [])).toEqual([]);
|
||||
});
|
||||
|
||||
it('returns [] when there are no loaded specs', () => {
|
||||
const ws = [{ name: 'a', path: 'C:/ws/a.yaml' }];
|
||||
expect(matchLoadedApiSpecs(ws, [])).toEqual([]);
|
||||
expect(matchLoadedApiSpecs(ws, undefined)).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -550,10 +550,12 @@ export const loadWorkspaceApiSpecs = (workspaceUid) => {
|
||||
}));
|
||||
|
||||
const allApiSpecs = getState().apiSpec.apiSpecs;
|
||||
const alreadyOpenApiSpecs = allApiSpecs.map((a) => a.pathname);
|
||||
// Compare by normalized path so a spec already loaded under a native (Windows)
|
||||
// path isn't treated as "not open" and needlessly re-opened.
|
||||
const alreadyOpenApiSpecs = allApiSpecs.map((a) => normalizePath(a.pathname));
|
||||
|
||||
for (const apiSpec of apiSpecs) {
|
||||
if (apiSpec.path && !alreadyOpenApiSpecs.includes(apiSpec.path)) {
|
||||
if (apiSpec.path && !alreadyOpenApiSpecs.includes(normalizePath(apiSpec.path))) {
|
||||
try {
|
||||
await ipcRenderer.invoke('renderer:open-api-spec-file', apiSpec.path, workspace.pathname);
|
||||
} catch (error) {
|
||||
|
||||
@@ -11,16 +11,6 @@ const {
|
||||
|
||||
const DEFAULT_WORKSPACE_NAME = 'My Workspace';
|
||||
|
||||
const normalizeWorkspaceConfig = (config) => {
|
||||
return {
|
||||
...config,
|
||||
name: config.info?.name,
|
||||
type: config.info?.type,
|
||||
collections: config.collections || [],
|
||||
apiSpecs: config.specs || []
|
||||
};
|
||||
};
|
||||
|
||||
const prepareWorkspaceConfigForClient = (workspaceConfig, isDefault) => {
|
||||
if (isDefault) {
|
||||
return {
|
||||
@@ -56,7 +46,7 @@ const openApiSpec = async (win, watcher, apiSpecPath, options = {}) => {
|
||||
|
||||
if (fs.existsSync(workspaceFilePath)) {
|
||||
const workspaceConfig = readWorkspaceConfig(options.workspacePath);
|
||||
const specs = workspaceConfig.specs || [];
|
||||
const specs = workspaceConfig.specs;
|
||||
|
||||
const specName = path.basename(apiSpecPath, path.extname(apiSpecPath));
|
||||
|
||||
@@ -75,10 +65,9 @@ const openApiSpec = async (win, watcher, apiSpecPath, options = {}) => {
|
||||
});
|
||||
|
||||
const updatedConfig = readWorkspaceConfig(options.workspacePath);
|
||||
const normalizedConfig = normalizeWorkspaceConfig(updatedConfig);
|
||||
const workspaceUid = getWorkspaceUid(options.workspacePath);
|
||||
const isDefault = workspaceUid === 'default';
|
||||
const configForClient = prepareWorkspaceConfigForClient(normalizedConfig, isDefault);
|
||||
const configForClient = prepareWorkspaceConfigForClient(updatedConfig, isDefault);
|
||||
win.webContents.send('main:workspace-config-updated', options.workspacePath, workspaceUid, configForClient);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4,7 +4,7 @@ const path = require('path');
|
||||
const chokidar = require('chokidar');
|
||||
const yaml = require('js-yaml');
|
||||
const { generateUidBasedOnHash, uuid } = require('../utils/common');
|
||||
const { getWorkspaceUid } = require('../utils/workspace-config');
|
||||
const { getWorkspaceUid, normalizeWorkspaceConfig } = require('../utils/workspace-config');
|
||||
const { parseEnvironment } = require('@usebruno/filestore');
|
||||
const EnvironmentSecretsStore = require('../store/env-secrets');
|
||||
const { decryptStringSafe } = require('../utils/encryption');
|
||||
@@ -19,16 +19,6 @@ const envHasSecrets = (environment) => {
|
||||
return secrets && secrets.length > 0;
|
||||
};
|
||||
|
||||
const normalizeWorkspaceConfig = (config) => {
|
||||
return {
|
||||
...config,
|
||||
name: config.info?.name,
|
||||
type: config.info?.type,
|
||||
collections: config.collections || [],
|
||||
apiSpecs: config.specs || []
|
||||
};
|
||||
};
|
||||
|
||||
const handleWorkspaceFileChange = (win, workspacePath) => {
|
||||
try {
|
||||
const workspaceFilePath = path.join(workspacePath, 'workspace.yml');
|
||||
|
||||
@@ -214,7 +214,7 @@ const registerWorkspaceIpc = (mainWindow, workspaceWatcher) => {
|
||||
return [];
|
||||
}
|
||||
|
||||
const specs = workspaceConfig.specs || [];
|
||||
const specs = Array.isArray(workspaceConfig.specs) ? workspaceConfig.specs : [];
|
||||
|
||||
const resolvedSpecs = specs
|
||||
.map((spec) => {
|
||||
|
||||
@@ -200,12 +200,19 @@ const createWorkspaceConfig = (workspaceName) => ({
|
||||
});
|
||||
|
||||
const normalizeWorkspaceConfig = (config) => {
|
||||
// Coerce `specs` to an array once. A malformed workspace.yml (e.g. `specs`
|
||||
// authored as a map) would otherwise flow through as a non-array and crash
|
||||
// both the renderer sidebar (.map) and the write paths (.findIndex/.filter).
|
||||
const specs = Array.isArray(config.specs) ? config.specs : [];
|
||||
return {
|
||||
...config,
|
||||
name: config.info?.name,
|
||||
type: config.info?.type,
|
||||
collections: config.collections || [],
|
||||
apiSpecs: config.specs || []
|
||||
specs,
|
||||
// Distinct array (not an alias of `specs`) so a later in-place mutation of
|
||||
// one field can't silently change the other.
|
||||
apiSpecs: [...specs]
|
||||
};
|
||||
};
|
||||
|
||||
@@ -686,6 +693,7 @@ module.exports = {
|
||||
validateWorkspacePath,
|
||||
validateWorkspaceDirectory,
|
||||
createWorkspaceConfig,
|
||||
normalizeWorkspaceConfig,
|
||||
readWorkspaceConfig,
|
||||
writeWorkspaceConfig,
|
||||
validateWorkspaceConfig,
|
||||
|
||||
@@ -268,3 +268,105 @@ describe('Git remote on workspace collections', () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('workspace specs normalization', () => {
|
||||
const {
|
||||
readWorkspaceConfig,
|
||||
addApiSpecToWorkspace,
|
||||
removeApiSpecFromWorkspace
|
||||
} = require('../../src/utils/workspace-config');
|
||||
let workspacePath;
|
||||
|
||||
// Writes workspace.yml with a verbatim `specs:` block so we control its YAML shape.
|
||||
const writeWorkspaceYml = (specsYaml) => {
|
||||
const content = [
|
||||
'opencollection: 1.0.0',
|
||||
'info:',
|
||||
' name: Test',
|
||||
' type: workspace',
|
||||
'collections: []',
|
||||
specsYaml,
|
||||
'docs: \'\''
|
||||
].join('\n');
|
||||
fs.writeFileSync(path.join(workspacePath, 'workspace.yml'), content);
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
workspacePath = fs.mkdtempSync(path.join(os.tmpdir(), 'bruno-ws-'));
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
fs.rmSync(workspacePath, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
// --- Regression guard: the `|| []` -> `Array.isArray(...) ? ... : []` swap must
|
||||
// preserve behavior for every VALID shape, and only change non-array inputs. ---
|
||||
describe('readWorkspaceConfig coerces specs to an array', () => {
|
||||
const cases = [
|
||||
{
|
||||
name: 'valid populated list is preserved unchanged',
|
||||
yaml: ['specs:', ' - name: foo', ' path: foo.yaml', ' - name: bar', ' path: bar.yaml'].join('\n'),
|
||||
expected: [
|
||||
{ name: 'foo', path: 'foo.yaml' },
|
||||
{ name: 'bar', path: 'bar.yaml' }
|
||||
]
|
||||
},
|
||||
{ name: 'empty list stays empty', yaml: 'specs: []', expected: [] },
|
||||
{ name: 'missing specs key -> []', yaml: '# no specs key', expected: [] },
|
||||
{ name: 'null specs -> []', yaml: 'specs: null', expected: [] },
|
||||
{ name: 'map (object) specs -> []', yaml: ['specs:', ' brokenEntry: not a list'].join('\n'), expected: [] },
|
||||
{ name: 'string specs -> []', yaml: 'specs: "oops a string"', expected: [] },
|
||||
{ name: 'number specs -> []', yaml: 'specs: 42', expected: [] },
|
||||
{ name: 'boolean specs -> []', yaml: 'specs: true', expected: [] },
|
||||
{
|
||||
// An array of junk is still an array: coercion preserves it (no crash on .map);
|
||||
// invalid entries are dropped later by sanitizeSpecs on write, not here.
|
||||
name: 'array with non-object elements is preserved as-is',
|
||||
yaml: 'specs: [1, "two", null]',
|
||||
expected: [1, 'two', null]
|
||||
}
|
||||
];
|
||||
|
||||
test.each(cases)('$name', ({ yaml, expected }) => {
|
||||
writeWorkspaceYml(yaml);
|
||||
const config = readWorkspaceConfig(workspacePath);
|
||||
// Both the legacy `specs` field and the renderer-facing `apiSpecs` must be arrays.
|
||||
expect(Array.isArray(config.specs)).toBe(true);
|
||||
expect(Array.isArray(config.apiSpecs)).toBe(true);
|
||||
expect(config.specs).toEqual(expected);
|
||||
expect(config.apiSpecs).toEqual(expected);
|
||||
// apiSpecs mirrors specs by value but is a distinct array, so an in-place
|
||||
// mutation of one field can't silently change the other.
|
||||
expect(config.apiSpecs).not.toBe(config.specs);
|
||||
});
|
||||
});
|
||||
|
||||
// --- Write paths must not throw on an already-malformed workspace.yml and must self-heal. ---
|
||||
describe('write paths survive a malformed (non-array) specs', () => {
|
||||
const malformedYaml = ['specs:', ' brokenEntry: not a list'].join('\n');
|
||||
const specsInYml = () => {
|
||||
const raw = fs.readFileSync(path.join(workspacePath, 'workspace.yml'), 'utf8');
|
||||
return yaml.load(raw).specs;
|
||||
};
|
||||
|
||||
test('addApiSpecToWorkspace does not throw and writes a valid list', async () => {
|
||||
writeWorkspaceYml(malformedYaml);
|
||||
const specPath = path.join(workspacePath, 'api.yaml');
|
||||
await expect(
|
||||
addApiSpecToWorkspace(workspacePath, { name: 'api', path: specPath })
|
||||
).resolves.toBeDefined();
|
||||
|
||||
const stored = specsInYml();
|
||||
expect(Array.isArray(stored)).toBe(true);
|
||||
expect(stored).toEqual([{ name: 'api', path: 'api.yaml' }]);
|
||||
});
|
||||
|
||||
test('removeApiSpecFromWorkspace does not throw on malformed specs', async () => {
|
||||
writeWorkspaceYml(malformedYaml);
|
||||
const result = await removeApiSpecFromWorkspace(workspacePath, path.join(workspacePath, 'whatever.yaml'));
|
||||
expect(result.removedApiSpec).toBeNull();
|
||||
// Round-trip through readWorkspaceConfig (which coerces) must yield a safe array.
|
||||
expect(Array.isArray(readWorkspaceConfig(workspacePath).specs)).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user