From 91daa7a347d467f1b14a97a95887275fa9211f4c Mon Sep 17 00:00:00 2001 From: Sergei Egorov Date: Tue, 3 Sep 2019 17:06:58 +0200 Subject: [PATCH 1/3] Make it possible to allow/disallow methods by their descriptor --- .../AllowancesByteBuddyTransformer.java | 26 +++++++--- .../java/reactor/blockhound/BlockHound.java | 52 +++++++++++++------ 2 files changed, 56 insertions(+), 22 deletions(-) diff --git a/agent/src/main/java/reactor/blockhound/AllowancesByteBuddyTransformer.java b/agent/src/main/java/reactor/blockhound/AllowancesByteBuddyTransformer.java index f80ea6e..07b7c3e 100644 --- a/agent/src/main/java/reactor/blockhound/AllowancesByteBuddyTransformer.java +++ b/agent/src/main/java/reactor/blockhound/AllowancesByteBuddyTransformer.java @@ -37,9 +37,9 @@ */ class AllowancesByteBuddyTransformer implements AgentBuilder.Transformer { - private Map> allowances; + private Map>> allowances; - AllowancesByteBuddyTransformer(Map> allowances) { + AllowancesByteBuddyTransformer(Map>> allowances) { this.allowances = allowances; } @@ -50,7 +50,7 @@ public DynamicType.Builder transform( ClassLoader classLoader, JavaModule module ) { - Map methods = allowances.get(typeDescription.getName()); + Map> methods = allowances.get(typeDescription.getName()); if (methods == null) { return builder; @@ -60,7 +60,14 @@ public DynamicType.Builder transform( .withCustomMapping() .bind(new AllowedArgument.Factory(methods)) .to(AllowAdvice.class) - .on(method -> methods.containsKey(method.getName())); + .on(method -> { + Map byDescriptor = methods.get(method.getName()); + if (byDescriptor == null) { + return false; + } + + return byDescriptor.containsKey("*") || byDescriptor.containsKey(method.getDescriptor()); + }); return builder.visit(advice); } @@ -76,9 +83,9 @@ public DynamicType.Builder transform( */ class Factory implements Advice.OffsetMapping.Factory { - final Map methods; + final Map> methods; - Factory(Map methods) { + Factory(Map> methods) { this.methods = methods; } @@ -94,7 +101,12 @@ public Advice.OffsetMapping make( AdviceType adviceType ) { return (instrumentedType, instrumentedMethod, assigner, argumentHandler, sort) -> { - boolean allowed = methods.get(instrumentedMethod.getName()); + Map byDescriptor = methods.get(instrumentedMethod.getName()); + + Boolean allowed = byDescriptor.get(instrumentedMethod.getDescriptor()); + if (allowed == null) { + allowed = byDescriptor.get("*"); + } return Advice.OffsetMapping.Target.ForStackManipulation.of(allowed); }; } diff --git a/agent/src/main/java/reactor/blockhound/BlockHound.java b/agent/src/main/java/reactor/blockhound/BlockHound.java index c289a05..395b4a1 100644 --- a/agent/src/main/java/reactor/blockhound/BlockHound.java +++ b/agent/src/main/java/reactor/blockhound/BlockHound.java @@ -146,20 +146,28 @@ public static class Builder { } }}; - private final Map> allowances = new HashMap>() {{ - put(ClassLoader.class.getName(), new HashMap() {{ - put("loadClass", true); + private final Map>> allowances = new HashMap>>() {{ + put(ClassLoader.class.getName(), new HashMap>() {{ + put("loadClass", new HashMap() {{ + put("(Ljava/lang/String;)Ljava/lang/Class;", true); + }}); }}); - put(Throwable.class.getName(), new HashMap() {{ - put("printStackTrace", true); + put(Throwable.class.getName(), new HashMap>() {{ + put("printStackTrace", new HashMap() {{ + put("*", true); + }}); }}); - put(ConcurrentHashMap.class.getName(), new HashMap() {{ - put("initTable", true); + put(ConcurrentHashMap.class.getName(), new HashMap>() {{ + put("initTable", new HashMap() {{ + put("*", true); + }}); }}); - put(Advice.class.getName(), new HashMap() {{ - put("to", true); + put(Advice.class.getName(), new HashMap>() {{ + put("to", new HashMap() {{ + put("*", true); + }}); }}); }}; @@ -169,24 +177,38 @@ public static class Builder { private Predicate threadPredicate = t -> false; - public Builder markAsBlocking(Class clazz, String methodName, String signature) { - return markAsBlocking(clazz.getName(), methodName, signature); + public Builder markAsBlocking(Class clazz, String methodName, String descriptor) { + return markAsBlocking(clazz.getName(), methodName, descriptor); } - public Builder markAsBlocking(String className, String methodName, String signature) { + public Builder markAsBlocking(String className, String methodName, String descriptor) { blockingMethods.computeIfAbsent(className.replace(".", "/"), __ -> new HashMap<>()) .computeIfAbsent(methodName, __ -> new HashSet<>()) - .add(signature); + .add(descriptor); return this; } public Builder allowBlockingCallsInside(String className, String methodName) { - allowances.computeIfAbsent(className, __ -> new HashMap<>()).put(methodName, true); + return allowBlockingCallsInside(className, methodName, "*"); + } + + public Builder allowBlockingCallsInside(String className, String methodName, String descriptor) { + allowances + .computeIfAbsent(className, __ -> new HashMap<>()) + .computeIfAbsent(methodName, __ -> new HashMap<>()) + .put(descriptor, true); return this; } public Builder disallowBlockingCallsInside(String className, String methodName) { - allowances.computeIfAbsent(className, __ -> new HashMap<>()).put(methodName, false); + return disallowBlockingCallsInside(className, methodName, "*"); + } + + public Builder disallowBlockingCallsInside(String className, String methodName, String descriptor) { + allowances + .computeIfAbsent(className, __ -> new HashMap<>()) + .computeIfAbsent(methodName, __ -> new HashMap<>()) + .put("*", false); return this; } From 9e3b1d4631e6138de871930439a19546ecac251a Mon Sep 17 00:00:00 2001 From: Sergei Egorov Date: Mon, 30 Sep 2019 16:22:07 +0300 Subject: [PATCH 2/3] fix hardcoded "*" --- agent/src/main/java/reactor/blockhound/BlockHound.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/src/main/java/reactor/blockhound/BlockHound.java b/agent/src/main/java/reactor/blockhound/BlockHound.java index 395b4a1..a23d306 100644 --- a/agent/src/main/java/reactor/blockhound/BlockHound.java +++ b/agent/src/main/java/reactor/blockhound/BlockHound.java @@ -208,7 +208,7 @@ public Builder disallowBlockingCallsInside(String className, String methodName, allowances .computeIfAbsent(className, __ -> new HashMap<>()) .computeIfAbsent(methodName, __ -> new HashMap<>()) - .put("*", false); + .put(descriptor, false); return this; } From d51aefac74eb1ba0118bcf207835f4c99f8fac77 Mon Sep 17 00:00:00 2001 From: Sergei Egorov Date: Mon, 30 Sep 2019 16:22:54 +0300 Subject: [PATCH 3/3] Add Javadoc to the newly added methods --- .../java/reactor/blockhound/BlockHound.java | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/agent/src/main/java/reactor/blockhound/BlockHound.java b/agent/src/main/java/reactor/blockhound/BlockHound.java index a23d306..526e143 100644 --- a/agent/src/main/java/reactor/blockhound/BlockHound.java +++ b/agent/src/main/java/reactor/blockhound/BlockHound.java @@ -192,6 +192,18 @@ public Builder allowBlockingCallsInside(String className, String methodName) { return allowBlockingCallsInside(className, methodName, "*"); } + /** + * Allows blocking calls inside a method of a class with name identified by the provided className + * and that matches both provided methodName and descriptor. + * + * The descriptor should be in JVM's format: + * https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/types.html#wp276 + * + * @param className class' name (e.g. "java.lang.Thread") + * @param methodName a method name + * @param descriptor a method descriptor (in JVM's format) + * @return this + */ public Builder allowBlockingCallsInside(String className, String methodName, String descriptor) { allowances .computeIfAbsent(className, __ -> new HashMap<>()) @@ -204,6 +216,18 @@ public Builder disallowBlockingCallsInside(String className, String methodName) return disallowBlockingCallsInside(className, methodName, "*"); } + /** + * Disallows blocking calls inside a method of a class with name identified by the provided className + * and that matches both provided methodName and descriptor. + * + * The descriptor should be in JVM's format: + * https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/types.html#wp276 + * + * @param className class' name (e.g. "java.lang.Thread") + * @param methodName a method name + * @param descriptor a method descriptor (in JVM's format) + * @return this + */ public Builder disallowBlockingCallsInside(String className, String methodName, String descriptor) { allowances .computeIfAbsent(className, __ -> new HashMap<>())