From 15d11a24327b88539581407105a4b33f09d81490 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Thu, 5 Sep 2024 06:19:31 -0500 Subject: [PATCH] StackTraceDeobfuscator should never provide null methodName (#9997) Improves accuracy of deobfuscated stack traces by using the best answer between original JS names, symbol maps, and source maps, by both avoiding taking an identifier from the sourcemap if none is provided, and normalizing away chromium specific prefixes in stack trace elements. Fixes #9996 Follow-up #9936 --- .../core/server/StackTraceDeobfuscator.java | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/user/src/com/google/gwt/core/server/StackTraceDeobfuscator.java b/user/src/com/google/gwt/core/server/StackTraceDeobfuscator.java index f6f37ef59f..25d851c7c0 100644 --- a/user/src/com/google/gwt/core/server/StackTraceDeobfuscator.java +++ b/user/src/com/google/gwt/core/server/StackTraceDeobfuscator.java @@ -26,6 +26,7 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.net.URL; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -186,9 +187,9 @@ public final StackTraceElement[] resymbolize(StackTraceElement[] st, String stro return null; } // Warm the symbol cache for all symbols in this stack trace. - Set requiredSymbols = new HashSet(); + Set requiredSymbols = new HashSet<>(); for (StackTraceElement ste : st) { - requiredSymbols.add(ste.getMethodName()); + requiredSymbols.add(normalizeSymbol(ste.getMethodName())); } loadSymbolMap(strongName, requiredSymbols); @@ -199,6 +200,17 @@ public final StackTraceElement[] resymbolize(StackTraceElement[] st, String stro return newSt; } + /** + * Helper to clean up client-provided symbols. + */ + private static String normalizeSymbol(String symbol) { + // Chrome prefixes "new " for constructor calls, eliminate the prefix + if (symbol.startsWith("new ")) { + symbol = symbol.substring(4); + } + return symbol; + } + /** * Best effort resymbolization of a single stack trace element. * @@ -305,7 +317,13 @@ public final StackTraceElement resymbolize(StackTraceElement ste, String strongN if (declaringClass == null || declaringClass.equals(ste.getClassName())) { declaringClass = mappingForLine.getOriginalFile(); - methodName = mappingForLine.getIdentifier().orElse(null); + if (mappingForLine.getIdentifier().isPresent()) { + // Only overwrite the name if the sourcemap had an explicit identifier + methodName = mappingForLine.getIdentifier().get(); + } else if (methodName == null) { + // No other name was provided by symbolMaps, fall back to JS name + methodName = ste.getMethodName(); + } } fileName = mappingForLine.getOriginalFile(); lineNumber = mappingForLine.getLineNumber(); @@ -366,9 +384,8 @@ private String loadStreamAsString(InputStream stream) { } private String loadOneSymbol(String strongName, String symbol) { - Set symbolSet = new HashSet(); - symbolSet.add(symbol); - Map symbolMap = loadSymbolMap(strongName, symbolSet); + symbol = normalizeSymbol(symbol); + Map symbolMap = loadSymbolMap(strongName, Collections.singleton(symbol)); return symbolMap.get(symbol); }