Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(locators): escape quotes in regular expressions #27002

Merged
merged 1 commit into from
Sep 11, 2023
Merged

fix(locators): escape quotes in regular expressions #27002

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 12 additions & 8 deletions packages/playwright-core/src/utils/isomorphic/locatorGenerators.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import { escapeWithQuotes, toSnakeCase, toTitleCase } from './stringUtils';
import { escapeWithQuotes, normalizeEscapedRegexQuotes, toSnakeCase, toTitleCase } from './stringUtils';
import { type NestedSelectorBody, parseAttributeSelector, parseSelector, stringifySelector } from './selectorParser';
import type { ParsedSelector } from './selectorParser';

Expand Down Expand Up @@ -268,7 +268,7 @@ export class JavaScriptLocatorFactory implements LocatorFactory {
case 'role':
const attrs: string[] = [];
if (isRegExp(options.name)) {
attrs.push(`name: ${options.name}`);
attrs.push(`name: ${this.regexToSourceString(options.name)}`);
} else if (typeof options.name === 'string') {
attrs.push(`name: ${this.quote(options.name)}`);
if (options.exact)
Expand Down Expand Up @@ -313,21 +313,25 @@ export class JavaScriptLocatorFactory implements LocatorFactory {
return locators.join('.');
}

private regexToSourceString(re: RegExp) {
return normalizeEscapedRegexQuotes(String(re));
}

private toCallWithExact(method: string, body: string | RegExp, exact?: boolean) {
if (isRegExp(body))
return `${method}(${body})`;
return `${method}(${this.regexToSourceString(body)})`;
return exact ? `${method}(${this.quote(body)}, { exact: true })` : `${method}(${this.quote(body)})`;
}

private toHasText(body: string | RegExp) {
if (isRegExp(body))
return String(body);
return this.regexToSourceString(body);
return this.quote(body);
}

private toTestIdValue(value: string | RegExp): string {
if (isRegExp(value))
return String(value);
return this.regexToSourceString(value);
return this.quote(value);
}

Expand Down Expand Up @@ -407,7 +411,7 @@ export class PythonLocatorFactory implements LocatorFactory {

private regexToString(body: RegExp) {
const suffix = body.flags.includes('i') ? ', re.IGNORECASE' : '';
return `re.compile(r"${body.source.replace(/\\\//, '/').replace(/"/g, '\\"')}"${suffix})`;
return `re.compile(r"${normalizeEscapedRegexQuotes(body.source).replace(/\\\//, '/').replace(/"/g, '\\"')}"${suffix})`;
}

private toCallWithExact(method: string, body: string | RegExp, exact: boolean) {
Expand Down Expand Up @@ -508,7 +512,7 @@ export class JavaLocatorFactory implements LocatorFactory {

private regexToString(body: RegExp) {
const suffix = body.flags.includes('i') ? ', Pattern.CASE_INSENSITIVE' : '';
return `Pattern.compile(${this.quote(body.source)}${suffix})`;
return `Pattern.compile(${this.quote(normalizeEscapedRegexQuotes(body.source))}${suffix})`;
}

private toCallWithExact(clazz: string, method: string, body: string | RegExp, exact: boolean) {
Expand Down Expand Up @@ -603,7 +607,7 @@ export class CSharpLocatorFactory implements LocatorFactory {

private regexToString(body: RegExp): string {
const suffix = body.flags.includes('i') ? ', RegexOptions.IgnoreCase' : '';
return `new Regex(${this.quote(body.source)}${suffix})`;
return `new Regex(${this.quote(normalizeEscapedRegexQuotes(body.source))}${suffix})`;
}

private toCallWithExact(method: string, body: string | RegExp, exact: boolean) {
Expand Down
2 changes: 1 addition & 1 deletion packages/playwright-core/src/utils/isomorphic/locatorParser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ function transform(template: string, params: TemplateParams, testIdAttributeName
.replace(/(?:r)\$(\d+)(i)?/g, (_, ordinal, suffix) => {
const param = params[+ordinal - 1];
if (t.startsWith('internal:attr') || t.startsWith('internal:testid') || t.startsWith('internal:role'))
return new RegExp(param.text) + (suffix || '');
return escapeForAttributeSelector(new RegExp(param.text), false) + (suffix || '');
return escapeForTextSelector(new RegExp(param.text, suffix), false);
})
.replace(/\$(\d+)(i|s)?/g, (_, ordinal, suffix) => {
Expand Down
4 changes: 2 additions & 2 deletions packages/playwright-core/src/utils/isomorphic/locatorUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import { escapeForAttributeSelector, escapeForTextSelector, isString } from './stringUtils';
import { escapeForAttributeSelector, escapeForTextSelector } from './stringUtils';

export type ByRoleOptions = {
checked?: boolean;
Expand Down Expand Up @@ -71,7 +71,7 @@ export function getByRoleSelector(role: string, options: ByRoleOptions = {}): st
if (options.level !== undefined)
props.push(['level', String(options.level)]);
if (options.name !== undefined)
props.push(['name', isString(options.name) ? escapeForAttributeSelector(options.name, !!options.exact) : String(options.name)]);
props.push(['name', escapeForAttributeSelector(options.name, !!options.exact)]);
if (options.pressed !== undefined)
props.push(['pressed', String(options.pressed)]);
return `internal:role=${role}${props.map(([n, v]) => `[${n}=${v}]`).join('')}`;
Expand Down
15 changes: 13 additions & 2 deletions packages/playwright-core/src/utils/isomorphic/stringUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,26 @@ export function normalizeWhiteSpace(text: string): string {
return text.replace(/\u200b/g, '').trim().replace(/\s+/g, ' ');
}

export function normalizeEscapedRegexQuotes(source: string) {
// This function reverses the effect of escapeRegexForSelector below.
// Odd number of backslashes followed by the quote -> remove unneeded backslash.
return source.replace(/(^|[^\\])(\\\\)*\\(['"`])/g, '$1$2$3');
}

function escapeRegexForSelector(re: RegExp): string {
// Even number of backslashes followed by the quote -> insert a backslash.
return String(re).replace(/(^|[^\\])(\\\\)*(["'`])/g, '$1$2\\$3').replace(/>>/g, '\\>\\>');
}

export function escapeForTextSelector(text: string | RegExp, exact: boolean): string {
if (typeof text !== 'string')
return String(text).replace(/>>/g, '\\>\\>');
return escapeRegexForSelector(text);
return `${JSON.stringify(text)}${exact ? 's' : 'i'}`;
}

export function escapeForAttributeSelector(value: string | RegExp, exact: boolean): string {
if (typeof value !== 'string')
return String(value).replace(/>>/g, '\\>\\>');
return escapeRegexForSelector(value);
// TODO: this should actually be
// cssEscape(value).replace(/\\ /g, ' ')
// However, our attribute selectors do not conform to CSS parsing spec,
Expand Down
7 changes: 7 additions & 0 deletions tests/library/locator-generator.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,13 @@ it('reverse engineer locators', async ({ page }) => {
csharp: 'GetByTestId(new Regex("He\\"llo"))'
});

expect.soft(generate(page.getByTestId(/He\\"llo/))).toEqual({
javascript: 'getByTestId(/He\\\\"llo/)',
python: 'get_by_test_id(re.compile(r"He\\\\\\"llo"))',
java: 'getByTestId(Pattern.compile("He\\\\\\\\\\"llo"))',
csharp: 'GetByTestId(new Regex("He\\\\\\\\\\"llo"))'
});

expect.soft(generate(page.getByText('Hello', { exact: true }))).toEqual({
csharp: 'GetByText("Hello", new() { Exact = true })',
java: 'getByText("Hello", new Page.GetByTextOptions().setExact(true))',
Expand Down
32 changes: 32 additions & 0 deletions tests/page/locator-query.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,38 @@ it('should filter by regex with quotes', async ({ page }) => {
await expect(page.locator('div', { hasText: /Hello "world"/ })).toHaveText('Hello "world"');
});

it('should filter by regex with a single quote', async ({ page }) => {
await page.setContent(`<button>let's let's<span>hello</span></button>`);
await expect.soft(page.locator('button', { hasText: /let's/i }).locator('span')).toHaveText('hello');
await expect.soft(page.getByRole('button', { name: /let's/i }).locator('span')).toHaveText('hello');
await expect.soft(page.locator('button', { hasText: /let\'s/i }).locator('span')).toHaveText('hello');
await expect.soft(page.getByRole('button', { name: /let\'s/i }).locator('span')).toHaveText('hello');
await expect.soft(page.locator('button', { hasText: /'s/i }).locator('span')).toHaveText('hello');
await expect.soft(page.getByRole('button', { name: /'s/i }).locator('span')).toHaveText('hello');
await expect.soft(page.locator('button', { hasText: /\'s/i }).locator('span')).toHaveText('hello');
await expect.soft(page.getByRole('button', { name: /\'s/i }).locator('span')).toHaveText('hello');
await expect.soft(page.locator('button', { hasText: /let['abc]s/i }).locator('span')).toHaveText('hello');
await expect.soft(page.getByRole('button', { name: /let['abc]s/i }).locator('span')).toHaveText('hello');
await expect.soft(page.locator('button', { hasText: /let\\'s/i })).not.toBeVisible();
await expect.soft(page.getByRole('button', { name: /let\\'s/i })).not.toBeVisible();
await expect.soft(page.locator('button', { hasText: /let's let\'s/i }).locator('span')).toHaveText('hello');
await expect.soft(page.getByRole('button', { name: /let's let\'s/i }).locator('span')).toHaveText('hello');
await expect.soft(page.locator('button', { hasText: /let\'s let's/i }).locator('span')).toHaveText('hello');
await expect.soft(page.getByRole('button', { name: /let\'s let's/i }).locator('span')).toHaveText('hello');

await page.setContent(`<button>let\\'s let\\'s<span>hello</span></button>`);
await expect.soft(page.locator('button', { hasText: /let\'s/i })).not.toBeVisible();
await expect.soft(page.getByRole('button', { name: /let\'s/i })).not.toBeVisible();
await expect.soft(page.locator('button', { hasText: /let\\'s/i }).locator('span')).toHaveText('hello');
await expect.soft(page.getByRole('button', { name: /let\\'s/i }).locator('span')).toHaveText('hello');
await expect.soft(page.locator('button', { hasText: /let\\\'s/i }).locator('span')).toHaveText('hello');
await expect.soft(page.getByRole('button', { name: /let\\\'s/i }).locator('span')).toHaveText('hello');
await expect.soft(page.locator('button', { hasText: /let\\'s let\\\'s/i }).locator('span')).toHaveText('hello');
await expect.soft(page.getByRole('button', { name: /let\\'s let\\\'s/i }).locator('span')).toHaveText('hello');
await expect.soft(page.locator('button', { hasText: /let\\\'s let\\'s/i }).locator('span')).toHaveText('hello');
await expect.soft(page.getByRole('button', { name: /let\\\'s let\\'s/i }).locator('span')).toHaveText('hello');
});

it('should filter by regex and regexp flags', async ({ page }) => {
await page.setContent(`<div>Hello "world"</div><div>Hello world</div>`);
await expect(page.locator('div', { hasText: /hElLo "world"/i })).toHaveText('Hello "world"');
Expand Down
4 changes: 4 additions & 0 deletions tests/page/selectors-text.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ it('should work @smoke', async ({ page }) => {
expect(await page.$(`text="lo wo"`)).toBe(null);
expect((await page.$$(`text=lo \nwo`)).length).toBe(1);
expect((await page.$$(`text="lo \nwo"`)).length).toBe(0);

await page.setContent(`<div>let's<span>hello</span></div>`);
expect(await page.$eval(`text=/let's/i >> span`, e => e.outerHTML)).toBe('<span>hello</span>');
expect(await page.$eval(`text=/let\\'s/i >> span`, e => e.outerHTML)).toBe('<span>hello</span>');
});

it('should work with :text', async ({ page }) => {
Expand Down