Skip to content

Commit

Permalink
fix(locators): escape quotes in regular expressions (microsoft#27002)
Browse files Browse the repository at this point in the history
  • Loading branch information
dgozman authored and Germandrummer92 committed Oct 27, 2023
1 parent a8edc6a commit 7ee80e5
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 13 deletions.
20 changes: 12 additions & 8 deletions packages/playwright-core/src/utils/isomorphic/locatorGenerators.ts
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
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
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
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
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
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
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

0 comments on commit 7ee80e5

Please sign in to comment.