From aae3e03e57096c3dc51589a3de240d49a8b9fadf Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Thu, 21 Nov 2024 10:09:41 -0800 Subject: [PATCH] dev-middleware: Refactor urlRegex rewriting, regex-escape IP4 addresses (#47872) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/47872 Largely a refactoring to the way we currently rewrite `url`/`urlRegex` in `Debugger.setBreakpointByURL` CDP requests (debugger->target). Rewriting regexes is fragile, it only really works if we can assume `'localhost'` appears literally and that ports and protocols don't need changing. The intention here is to freeze the current behaviour so as not to break anyone relying on it (if anyone is), and decouple it from more robust rewriting we want to generalise. Also adds simple regex escaping to host names (always IPv4 addresses) we inject into regex patterns, since the previous approach could've led to false matches in unlikely edge cases. Changelog: [General][Fixed] dev-middleware: Regex-escape IP addresses in urlRegex replacements Reviewed By: huntie Differential Revision: D66238782 fbshipit-source-id: 4cd0029081d68c193e36d3713057ffdc7ef0656f --- .../InspectorProxyCdpRewritingHacks-test.js | 2 +- .../src/inspector-proxy/Device.js | 64 +++++++++++-------- 2 files changed, 39 insertions(+), 27 deletions(-) diff --git a/packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js b/packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js index 8c8a6fc345f248..6b9190dda05ca2 100644 --- a/packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js +++ b/packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js @@ -391,7 +391,7 @@ describe.each(['HTTP', 'HTTPS'])( }, }); expect(setBreakpointByUrlRegexMessage.params.urlRegex).toEqual( - `${sourceHost}:${serverRef.port}|example.com:2000`, + `${sourceHost.replaceAll('.', '\\.')}:${serverRef.port}|example.com:2000`, ); } finally { device.close(); diff --git a/packages/dev-middleware/src/inspector-proxy/Device.js b/packages/dev-middleware/src/inspector-proxy/Device.js index 977519a17b04d8..b739d8e2c00b46 100644 --- a/packages/dev-middleware/src/inspector-proxy/Device.js +++ b/packages/dev-middleware/src/inspector-proxy/Device.js @@ -869,36 +869,48 @@ export default class Device { debuggerInfo: DebuggerConnection, ): CDPRequest<'Debugger.setBreakpointByUrl'> { // If we replaced Android emulator's address to localhost we need to change it back. - if (debuggerInfo.originalSourceURLAddress != null) { - const processedReq = {...req, params: {...req.params}}; - if (processedReq.params.url != null) { - processedReq.params.url = processedReq.params.url.replace( - 'localhost', - debuggerInfo.originalSourceURLAddress, - ); + const {originalSourceURLAddress, prependedFilePrefix} = debuggerInfo; + const processedReq = {...req, params: {...req.params}}; + if (originalSourceURLAddress != null && processedReq.params.url != null) { + processedReq.params.url = processedReq.params.url.replace( + 'localhost', + originalSourceURLAddress, + ); - if ( - processedReq.params.url && - processedReq.params.url.startsWith(FILE_PREFIX) && - debuggerInfo.prependedFilePrefix - ) { - // Remove fake URL prefix if we modified URL in #processMessageFromDeviceLegacy. - // $FlowFixMe[incompatible-use] - processedReq.params.url = processedReq.params.url.slice( - FILE_PREFIX.length, - ); - } - } - if (processedReq.params.urlRegex != null) { - processedReq.params.urlRegex = processedReq.params.urlRegex.replace( - /localhost/g, - // $FlowFixMe[incompatible-call] - debuggerInfo.originalSourceURLAddress, + if ( + processedReq.params.url && + processedReq.params.url.startsWith(FILE_PREFIX) && + prependedFilePrefix + ) { + // Remove fake URL prefix if we modified URL in #processMessageFromDeviceLegacy. + // $FlowFixMe[incompatible-use] + processedReq.params.url = processedReq.params.url.slice( + FILE_PREFIX.length, ); } - return processedReq; } - return req; + + // Retain special case rewriting of localhost to device-relative IPs + // within regex patterns. We don't rewrite the protocol here because + // these patterns typically come from CDT reinterpreting the source URL + // `file://host/path` into the regex `host/path|file://host/path`. See: + // + // https://github.com/ChromeDevTools/devtools-frontend/blob/f913cc6d76f2e2639c05b11ba673fc880b5490dd/front_end/core/sdk/DebuggerModel.ts#L505 + // + // This has always been fragile and probably unnecessary - we don't set + // `file://` source URLs. It can be removed when we drop support for + // legacy targets, if not sooner. + if ( + REWRITE_HOSTS_TO_LOCALHOST.has(this.#deviceRelativeBaseUrl.hostname) && + processedReq.params.urlRegex != null + ) { + processedReq.params.urlRegex = processedReq.params.urlRegex.replaceAll( + 'localhost', + // regex-escape IPv4 + this.#deviceRelativeBaseUrl.hostname.replaceAll('.', '\\.'), + ); + } + return processedReq; } #processDebuggerGetScriptSource(