Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions js/.changeset/fix-normalize-selector-array-validation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
'browser-commander': patch
---

Fix normalizeSelector to validate input type and reject arrays

When `normalizeSelector` receives an invalid type (array, number, or non-text-selector object), it now returns `null` with a warning instead of returning the invalid value unchanged.

This prevents downstream `querySelectorAll` errors with invalid selector syntax (like trailing commas when arrays are accidentally passed).

Fixes #23
21 changes: 21 additions & 0 deletions js/src/elements/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,25 @@ export async function normalizeSelector(options = {}) {
throw new Error('selector is required in options');
}

// DEFENSIVE: Validate selector type - must be string or Puppeteer text selector object
// Arrays and other invalid types should be rejected to prevent downstream querySelectorAll errors
if (Array.isArray(selector)) {
console.warn(
`normalizeSelector received invalid selector type: array. Expected string or text selector object.`
);
return null;
}

if (
typeof selector !== 'string' &&
(typeof selector !== 'object' || !selector._isPuppeteerTextSelector)
) {
console.warn(
`normalizeSelector received invalid selector type: ${typeof selector}. Expected string or text selector object.`
);
return null;
}

// Handle Playwright text selectors (strings containing :has-text or :text-is)
// These are valid for Playwright's locator API but NOT for document.querySelectorAll
if (
Expand Down Expand Up @@ -267,6 +286,8 @@ export async function normalizeSelector(options = {}) {
}
}

// This line should be unreachable after validation, but kept as a safeguard
// istanbul ignore next
return selector;
}

Expand Down
34 changes: 32 additions & 2 deletions js/tests/unit/elements/selectors.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,11 +207,41 @@ describe('selectors', () => {
assert.strictEqual(result, 'button');
});

it('should return non-text-selector object unchanged', async () => {
it('should return null for invalid object selector', async () => {
const page = createMockPlaywrightPage();
const obj = { someKey: 'value' };
const result = await normalizeSelector({ page, selector: obj });
assert.strictEqual(result, obj);
assert.strictEqual(result, null);
});

it('should return null for array selector', async () => {
const page = createMockPlaywrightPage();
const arr = ['[data-qa="test"]', []];
const result = await normalizeSelector({ page, selector: arr });
assert.strictEqual(result, null);
});

it('should return null for number selector', async () => {
const page = createMockPlaywrightPage();
const result = await normalizeSelector({ page, selector: 123 });
assert.strictEqual(result, null);
});

it('should accept valid Puppeteer text selector object', async () => {
const page = createMockPuppeteerPage();
page.evaluate = async () => '[data-qa="test"]';
const textSelector = {
_isPuppeteerTextSelector: true,
baseSelector: 'button',
text: 'Click me',
exact: false,
};
const result = await normalizeSelector({
page,
engine: 'puppeteer',
selector: textSelector,
});
assert.strictEqual(result, '[data-qa="test"]');
});
});

Expand Down
Loading