From ba9cfca7d6828be88eb0ed2a06183ca5ca86ad01 Mon Sep 17 00:00:00 2001 From: Thomas Watson Date: Sat, 1 Feb 2025 08:05:26 +0100 Subject: [PATCH] [DI] Probe file path matching algo should prefer shorter paths --- .../src/debugger/devtools_client/state.js | 7 +++-- .../debugger/devtools_client/state.spec.js | 31 ++++++++++++++++--- 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/packages/dd-trace/src/debugger/devtools_client/state.js b/packages/dd-trace/src/debugger/devtools_client/state.js index 30f4d2c879b..d165996dd92 100644 --- a/packages/dd-trace/src/debugger/devtools_client/state.js +++ b/packages/dd-trace/src/debugger/devtools_client/state.js @@ -43,8 +43,8 @@ module.exports = { // If both are boundaries, or if characters match exactly if (isBoundary || urlChar === pathChar) { if (isBoundary) { - lastBoundaryPos = matchLength atBoundary = true + lastBoundaryPos = matchLength } else { atBoundary = false } @@ -71,7 +71,10 @@ module.exports = { } // If we found a valid match and it's better than our previous best - if (atBoundary && lastBoundaryPos !== -1 && lastBoundaryPos > maxMatchLength) { + if (atBoundary && ( + lastBoundaryPos > maxMatchLength || + (lastBoundaryPos === maxMatchLength && url.length < bestMatch[0].length) // Prefer shorter paths + )) { maxMatchLength = lastBoundaryPos bestMatch[0] = url bestMatch[1] = scriptId diff --git a/packages/dd-trace/test/debugger/devtools_client/state.spec.js b/packages/dd-trace/test/debugger/devtools_client/state.spec.js index ff4455e7b7c..3533b3367c0 100644 --- a/packages/dd-trace/test/debugger/devtools_client/state.spec.js +++ b/packages/dd-trace/test/debugger/devtools_client/state.spec.js @@ -25,6 +25,13 @@ describe('findScriptFromPartialPath', function () { // Test case for when there's multiple partial matches listener({ params: { scriptId: 'should-match', url: 'file:///server/index.js' } }) listener({ params: { scriptId: 'should-not-match', url: 'file:///index.js' } }) + + // Test case for when there's two equal length partial matches + listener({ params: { scriptId: 'should-not-match-longest-a', url: 'file:///node_modules/foo/index.js' } }) + listener({ params: { scriptId: 'should-match-shortest-a', url: 'file:///foo/index.js' } }) + // The same, but in reverse order to ensure this doesn't influence the result + listener({ params: { scriptId: 'should-match-shortest-b', url: 'file:///bar/index.js' } }) + listener({ params: { scriptId: 'should-not-match-longest-b', url: 'file:///node_modules/bar/index.js' } }) } } } @@ -117,14 +124,30 @@ describe('findScriptFromPartialPath', function () { const result = state.findScriptFromPartialPath('server/index.js') expect(result).to.deep.equal(['file:///server/index.js', 'should-match', undefined]) }) + + it('should match the shorter of two equal length partial matches', function () { + const result1 = state.findScriptFromPartialPath('foo/index.js') + expect(result1).to.deep.equal(['file:///foo/index.js', 'should-match-shortest-a', undefined]) + + const result2 = state.findScriptFromPartialPath('bar/index.js') + expect(result2).to.deep.equal(['file:///bar/index.js', 'should-match-shortest-b', undefined]) + }) }) - describe('circuit breakers', function () { - it('should abort if the path is unknown', testPathNoMatch('this/path/does/not/exist.js')) + describe('should abort if the path is', function () { + it('unknown', testPathNoMatch('this/path/does/not/exist.js')) + + it('undefined', testPathNoMatch(undefined)) + + it('an empty string', testPathNoMatch('')) + + it('a slash', testPathNoMatch('/')) + + it('a backslash', testPathNoMatch('\\')) - it('should abort if the path is undefined', testPathNoMatch(undefined)) + it('a Windows drive letter', testPathNoMatch('c:')) - it('should abort if the path is an empty string', testPathNoMatch('')) + it('a Windows drive letter with a backslash', testPathNoMatch('c:\\')) }) function testPathNoMatch (path) {