From 15dedb78f47c2a18fd27d45de8ea684c4ac93b17 Mon Sep 17 00:00:00 2001 From: Max Schmitt Date: Mon, 5 Aug 2024 13:53:53 +0200 Subject: [PATCH] address feedback --- .../server/socksClientCertificatesInterceptor.ts | 15 +++++++-------- .../src/utils/isomorphic/stringUtils.ts | 8 ++++++++ packages/trace-viewer/src/snapshotRenderer.ts | 13 +++---------- tests/library/client-certificates.spec.ts | 4 ++-- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts b/packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts index 17a4a708a6..6e0fba980e 100644 --- a/packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts +++ b/packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts @@ -22,7 +22,7 @@ import fs from 'fs'; import tls from 'tls'; import stream from 'stream'; import { createSocket, createTLSSocket } from '../utils/happy-eyeballs'; -import { ManualPromise, rewriteErrorMessage } from '../utils'; +import { escapeHTML, ManualPromise, rewriteErrorMessage } from '../utils'; import type { SocksSocketClosedPayload, SocksSocketDataPayload, SocksSocketRequestedPayload } from '../common/socksProxy'; import { SocksProxy } from '../common/socksProxy'; import type * as channels from '@protocol/channels'; @@ -152,7 +152,9 @@ class SocksProxyConnection { const handleError = (error: Error) => { error = rewriteOpenSSLErrorIfNeeded(error); debugLogger.log('client-certificates', `error when connecting to target: ${error.message.replaceAll('\n', ' ')}`); - const responseBody = 'Playwright client-certificate error: ' + error.message.replaceAll('\n', '
'); + const responseBody = escapeHTML('Playwright client-certificate error: ' + error.message) + .replaceAll('\n', '
') + .replaceAll(/(https?:\/\/[^\s]+)/g, '$1'); if (internalTLS?.alpnProtocol === 'h2') { // This method is available only in Node.js 20+ if ('performServerHandshake' in http2) { @@ -304,11 +306,8 @@ export function rewriteOpenSSLErrorIfNeeded(error: Error): Error { return error; return rewriteErrorMessage(error, [ 'Unsupported TLS certificate.', - 'Node.js (OpenSSL) has deprecated certifiates with the security algorithm of the given client-certifiate.', - 'To fix this issue, you need to modernise the certificates by running the following command:', - 'openssl pkcs12 -in oldPfxFile.pfx -nodes -legacy -out decryptedPfxFile.tmp', - 'openssl pkcs12 -in decryptedPfxFile.tmp -export -out newPfxFile.pfx', - 'Then, you can use the newPfxFile.pfx in your client-certificates configuration.', - 'For more information, please refer to OpenSSL: https://github.com/openssl/openssl/blob/master/README-PROVIDERS.md#the-legacy-provider', + 'Most likely, the security algorithm of the given certificate was deprecated by OpenSSL.', + 'For more details, see https://github.com/openssl/openssl/blob/master/README-PROVIDERS.md#the-legacy-provider', + 'You could probably modernize the certificate by following the steps at https://github.com/nodejs/node/issues/40672#issuecomment-1243648223', ].join('\n')); } \ No newline at end of file diff --git a/packages/playwright-core/src/utils/isomorphic/stringUtils.ts b/packages/playwright-core/src/utils/isomorphic/stringUtils.ts index df213a1085..23c947cc49 100644 --- a/packages/playwright-core/src/utils/isomorphic/stringUtils.ts +++ b/packages/playwright-core/src/utils/isomorphic/stringUtils.ts @@ -132,3 +132,11 @@ export function escapeRegExp(s: string) { // From https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions#escaping return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); // $& means the whole matched string } + +const escaped = { '&': '&', '<': '<', '>': '>', '"': '"', '\'': ''' }; +export function escapeHTMLAttribute(s: string): string { + return s.replace(/[&<>"']/ug, char => (escaped as any)[char]); +} +export function escapeHTML(s: string): string { + return s.replace(/[&<]/ug, char => (escaped as any)[char]); +} diff --git a/packages/trace-viewer/src/snapshotRenderer.ts b/packages/trace-viewer/src/snapshotRenderer.ts index 730cab4d19..0b458c0cb5 100644 --- a/packages/trace-viewer/src/snapshotRenderer.ts +++ b/packages/trace-viewer/src/snapshotRenderer.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { escapeHTMLAttribute, escapeHTML } from '@isomorphic/stringUtils'; import type { FrameSnapshot, NodeNameAttributesChildNodesSnapshot, NodeSnapshot, RenderedFrameSnapshot, ResourceSnapshot, SubtreeReferenceSnapshot } from '@trace/snapshot'; function isNodeNameAttributesChildNodesSnapshot(n: NodeSnapshot): n is NodeNameAttributesChildNodesSnapshot { @@ -57,7 +58,7 @@ export class SnapshotRenderer { // Old snapshotter was sending lower-case. if (parentTag === 'STYLE' || parentTag === 'style') return rewriteURLsInStyleSheetForCustomProtocol(n); - return escapeText(n); + return escapeHTML(n); } if (!(n as any)._string) { @@ -106,7 +107,7 @@ export class SnapshotRenderer { attrValue = 'link://' + value; else if (attr.toLowerCase() === 'href' || attr.toLowerCase() === 'src' || attr === kCurrentSrcAttribute) attrValue = rewriteURLForCustomProtocol(value); - builder.push(' ', attrName, '="', escapeAttribute(attrValue), '"'); + builder.push(' ', attrName, '="', escapeHTMLAttribute(attrValue), '"'); } builder.push('>'); for (const child of children) @@ -193,14 +194,6 @@ export class SnapshotRenderer { } const autoClosing = new Set(['AREA', 'BASE', 'BR', 'COL', 'COMMAND', 'EMBED', 'HR', 'IMG', 'INPUT', 'KEYGEN', 'LINK', 'MENUITEM', 'META', 'PARAM', 'SOURCE', 'TRACK', 'WBR']); -const escaped = { '&': '&', '<': '<', '>': '>', '"': '"', '\'': ''' }; - -function escapeAttribute(s: string): string { - return s.replace(/[&<>"']/ug, char => (escaped as any)[char]); -} -function escapeText(s: string): string { - return s.replace(/[&<]/ug, char => (escaped as any)[char]); -} function snapshotNodes(snapshot: FrameSnapshot): NodeSnapshot[] { if (!(snapshot as any)._nodes) { diff --git a/tests/library/client-certificates.spec.ts b/tests/library/client-certificates.spec.ts index fd54f020a4..bcf7234f7c 100644 --- a/tests/library/client-certificates.spec.ts +++ b/tests/library/client-certificates.spec.ts @@ -200,7 +200,7 @@ test.describe('fetch', () => { await request.dispose(); }); - test('should pass with matching certificates in legacy pfx format', async ({ playwright, startCCServer, asset, browserName }) => { + test('should fail with matching certificates in legacy pfx format', async ({ playwright, startCCServer, asset, browserName }) => { const serverURL = await startCCServer({ useFakeLocalhost: browserName === 'webkit' && process.platform === 'darwin' }); const request = await playwright.request.newContext({ ignoreHTTPSErrors: true, @@ -317,7 +317,7 @@ test.describe('browser', () => { await page.close(); }); - test('should pass with matching certificates in legacy pfx format', async ({ browser, startCCServer, asset, browserName }) => { + test('should fail with matching certificates in legacy pfx format', async ({ browser, startCCServer, asset, browserName }) => { const serverURL = await startCCServer({ useFakeLocalhost: browserName === 'webkit' && process.platform === 'darwin' }); const page = await browser.newPage({ ignoreHTTPSErrors: true,