Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #3386 #3395

Merged
merged 2 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,20 @@
private TestService service;

@GetMapping("/foo")
public String apiFoo(@RequestParam(required = false) Long t) throws Exception {
public String apiFoo(@RequestParam(required = false) Long t) {
if (t == null) {
t = System.currentTimeMillis();
}
service.test();
return service.hello(t);
}

@GetMapping("/bar")
public String apiBar(@RequestParam(required = false) String t) {
service.test();
return service.hello(t);

Check warning

Code scanning / CodeQL

Cross-site scripting Medium

Cross-site scripting vulnerability due to a
user-provided value
.
}

@GetMapping("/baz/{name}")
public String apiBaz(@PathVariable("name") String name) {
return service.helloAnother(name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,7 @@ public interface TestService {

String hello(long s);

String hello(String s);

String helloAnother(String name);
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
package com.alibaba.csp.sentinel.demo.annotation.aop.service;

import com.alibaba.csp.sentinel.annotation.SentinelResource;
import com.alibaba.csp.sentinel.slots.block.BlockException;

import org.springframework.stereotype.Service;

/**
Expand All @@ -41,6 +39,15 @@ public String hello(long s) {
return String.format("Hello at %d", s);
}

@Override
@SentinelResource(value = "helloStr", fallback = "helloFallback")
public String hello(String s) {
if (s == null || s.trim().isEmpty()) {
throw new IllegalArgumentException("unknown");
}
return String.format("Hello, %s", s);
}

@Override
@SentinelResource(value = "helloAnother", defaultFallback = "defaultFallback",
exceptionsToIgnore = {IllegalStateException.class})
Expand All @@ -60,6 +67,12 @@ public String helloFallback(long s, Throwable ex) {
return "Oops, error occurred at " + s;
}

private String helloFallback(String ignored, Throwable e) {
// Do some log here.
e.printStackTrace();
return "Hello, stranger";
}

public String defaultFallback() {
System.out.println("Go to default fallback");
return "default_fallback";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
*
* @author Eric Zhao
* @author zhaoyuguang
* @author dowenliu-xyz([email protected])
*/
public abstract class AbstractSentinelAspectSupport {

Expand Down Expand Up @@ -177,12 +178,13 @@ private Method extractFallbackMethod(ProceedingJoinPoint pjp, String fallbackNam
}
boolean mustStatic = locationClass != null && locationClass.length >= 1;
Class<?> clazz = mustStatic ? locationClass[0] : pjp.getTarget().getClass();
MethodWrapper m = ResourceMetadataRegistry.lookupFallback(clazz, fallbackName);
Method originMethod = resolveMethod(pjp);
MethodWrapper m = ResourceMetadataRegistry.lookupFallback(clazz, fallbackName, originMethod.getParameterTypes());
if (m == null) {
// First time, resolve the fallback.
Method method = resolveFallbackInternal(pjp, fallbackName, clazz, mustStatic);
Method method = resolveFallbackInternal(originMethod, fallbackName, clazz, mustStatic);
// Cache the method instance.
ResourceMetadataRegistry.updateFallbackFor(clazz, fallbackName, method);
ResourceMetadataRegistry.updateFallbackFor(clazz, fallbackName, originMethod.getParameterTypes(), method);
return method;
}
if (!m.isPresent()) {
Expand Down Expand Up @@ -232,9 +234,7 @@ private Method extractDefaultFallbackMethod(ProceedingJoinPoint pjp, String defa
return m.getMethod();
}

private Method resolveFallbackInternal(ProceedingJoinPoint pjp, /*@NonNull*/ String name, Class<?> clazz,
boolean mustStatic) {
Method originMethod = resolveMethod(pjp);
private Method resolveFallbackInternal(Method originMethod, String name, Class<?> clazz, boolean mustStatic) {
// Fallback function allows two kinds of parameter list.
Class<?>[] defaultParamTypes = originMethod.getParameterTypes();
Class<?>[] paramTypesWithException = Arrays.copyOf(defaultParamTypes, defaultParamTypes.length + 1);
Expand All @@ -261,12 +261,13 @@ private Method extractBlockHandlerMethod(ProceedingJoinPoint pjp, String name, C
// By default current class.
clazz = pjp.getTarget().getClass();
}
MethodWrapper m = ResourceMetadataRegistry.lookupBlockHandler(clazz, name);
Method originMethod = resolveMethod(pjp);
MethodWrapper m = ResourceMetadataRegistry.lookupBlockHandler(clazz, name, originMethod.getParameterTypes());
if (m == null) {
// First time, resolve the block handler.
Method method = resolveBlockHandlerInternal(pjp, name, clazz, mustStatic);
Method method = resolveBlockHandlerInternal(originMethod, name, clazz, mustStatic);
// Cache the method instance.
ResourceMetadataRegistry.updateBlockHandlerFor(clazz, name, method);
ResourceMetadataRegistry.updateBlockHandlerFor(clazz, name, originMethod.getParameterTypes(), method);
return method;
}
if (!m.isPresent()) {
Expand All @@ -275,9 +276,7 @@ private Method extractBlockHandlerMethod(ProceedingJoinPoint pjp, String name, C
return m.getMethod();
}

private Method resolveBlockHandlerInternal(ProceedingJoinPoint pjp, /*@NonNull*/ String name, Class<?> clazz,
boolean mustStatic) {
Method originMethod = resolveMethod(pjp);
private Method resolveBlockHandlerInternal(Method originMethod, String name, Class<?> clazz, boolean mustStatic) {
Class<?>[] originList = originMethod.getParameterTypes();
Class<?>[] parameterTypes = Arrays.copyOf(originList, originList.length + 1);
parameterTypes[parameterTypes.length - 1] = BlockException.class;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,59 +16,111 @@
package com.alibaba.csp.sentinel.annotation.aspectj;

import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.stream.Collectors;

import com.alibaba.csp.sentinel.util.StringUtil;

/**
* Registry for resource configuration metadata (e.g. fallback method)
*
* @author Eric Zhao
* @author dowenliu-xyz([email protected])
*/
final class ResourceMetadataRegistry {

private static final Map<String, MethodWrapper> FALLBACK_MAP = new ConcurrentHashMap<>();
private static final Map<String, MethodWrapper> DEFAULT_FALLBACK_MAP = new ConcurrentHashMap<>();
private static final Map<String, MethodWrapper> BLOCK_HANDLER_MAP = new ConcurrentHashMap<>();

/**
* @deprecated use {@link #lookupFallback(Class, String, Class[])}
*/
@Deprecated
static MethodWrapper lookupFallback(Class<?> clazz, String name) {
return FALLBACK_MAP.get(getKey(clazz, name));
}

static MethodWrapper lookupFallback(Class<?> clazz, String name, Class<?>[] parameterTypes) {
return FALLBACK_MAP.get(getKey(clazz, name, parameterTypes));
}

static MethodWrapper lookupDefaultFallback(Class<?> clazz, String name) {
return DEFAULT_FALLBACK_MAP.get(getKey(clazz, name));
}

/**
* @deprecated use {@link #lookupBlockHandler(Class, String, Class[])}
*/
@Deprecated
static MethodWrapper lookupBlockHandler(Class<?> clazz, String name) {
return BLOCK_HANDLER_MAP.get(getKey(clazz, name));
}

static MethodWrapper lookupBlockHandler(Class<?> clazz, String name, Class<?>[] parameterTypes) {
return BLOCK_HANDLER_MAP.get(getKey(clazz, name, parameterTypes));
}

/**
* @deprecated use {@link #updateFallbackFor(Class, String, Class[], Method)}
*/
@Deprecated
static void updateFallbackFor(Class<?> clazz, String name, Method method) {
if (clazz == null || StringUtil.isBlank(name)) {
throw new IllegalArgumentException("Bad argument");
}
FALLBACK_MAP.put(getKey(clazz, name), MethodWrapper.wrap(method));
}

static void updateFallbackFor(Class<?> clazz, String handlerName, Class<?>[] parameterTypes, Method handlerMethod) {
if (clazz == null || StringUtil.isBlank(handlerName)) {
throw new IllegalArgumentException("Bad argument");

Check warning on line 79 in sentinel-extension/sentinel-annotation-aspectj/src/main/java/com/alibaba/csp/sentinel/annotation/aspectj/ResourceMetadataRegistry.java

View check run for this annotation

Codecov / codecov/patch

sentinel-extension/sentinel-annotation-aspectj/src/main/java/com/alibaba/csp/sentinel/annotation/aspectj/ResourceMetadataRegistry.java#L79

Added line #L79 was not covered by tests
}
FALLBACK_MAP.put(getKey(clazz, handlerName, parameterTypes), MethodWrapper.wrap(handlerMethod));
}

static void updateDefaultFallbackFor(Class<?> clazz, String name, Method method) {
if (clazz == null || StringUtil.isBlank(name)) {
throw new IllegalArgumentException("Bad argument");
}
DEFAULT_FALLBACK_MAP.put(getKey(clazz, name), MethodWrapper.wrap(method));
}


/**
* @deprecated use {@link #updateBlockHandlerFor(Class, String, Class[], Method)}
*/
@Deprecated
static void updateBlockHandlerFor(Class<?> clazz, String name, Method method) {
if (clazz == null || StringUtil.isBlank(name)) {
throw new IllegalArgumentException("Bad argument");
}
BLOCK_HANDLER_MAP.put(getKey(clazz, name), MethodWrapper.wrap(method));
}

static void updateBlockHandlerFor(Class<?> clazz, String handlerName, Class<?>[] parameterTypes,
Method handlerMethod) {
if (clazz == null || StringUtil.isBlank(handlerName)) {
throw new IllegalArgumentException("Bad argument");
}
BLOCK_HANDLER_MAP.put(getKey(clazz, handlerName, parameterTypes), MethodWrapper.wrap(handlerMethod));
}

private static String getKey(Class<?> clazz, String name) {
return String.format("%s:%s", clazz.getCanonicalName(), name);
}

private static String getKey(Class<?> clazz, String name, Class<?>[] parameterTypes) {
return String.format(
"%s:%s;%s",
clazz.getCanonicalName(),
name,
Arrays.stream(parameterTypes).map(Class::getCanonicalName).collect(Collectors.joining(","))
);
}

/**
* Only for internal test.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

/**
* @author Eric Zhao
* @author dowenliu-xyz([email protected])
*/
public class ResourceMetadataRegistryTest {

Expand All @@ -45,14 +46,15 @@ public void tearDown() throws Exception {
public void testUpdateThenLookupFallback() {
Class<?> clazz = FooService.class;
String methodName = "someMethodFallback";
Class<?>[] parameterTypes = new Class<?>[]{String.class, int.class};
Method method = clazz.getMethods()[0];
assertThat(ResourceMetadataRegistry.lookupFallback(clazz, methodName)).isNull();
assertThat(ResourceMetadataRegistry.lookupFallback(clazz, methodName, parameterTypes)).isNull();

ResourceMetadataRegistry.updateFallbackFor(clazz, methodName, null);
assertThat(ResourceMetadataRegistry.lookupFallback(clazz, methodName).isPresent()).isFalse();
ResourceMetadataRegistry.updateFallbackFor(clazz, methodName, parameterTypes, null);
assertThat(ResourceMetadataRegistry.lookupFallback(clazz, methodName, parameterTypes).isPresent()).isFalse();

ResourceMetadataRegistry.updateFallbackFor(clazz, methodName, method);
MethodWrapper wrapper = ResourceMetadataRegistry.lookupFallback(clazz, methodName);
ResourceMetadataRegistry.updateFallbackFor(clazz, methodName, parameterTypes, method);
MethodWrapper wrapper = ResourceMetadataRegistry.lookupFallback(clazz, methodName, parameterTypes);
assertThat(wrapper.isPresent()).isTrue();
assertThat(wrapper.getMethod()).isSameAs(method);
}
Expand All @@ -61,25 +63,26 @@ public void testUpdateThenLookupFallback() {
public void testUpdateThenLookupBlockHandler() {
Class<?> clazz = FooService.class;
String methodName = "someMethodBlockHand;er";
Class<?>[] parameterTypes = new Class<?>[]{String.class, int.class};
Method method = clazz.getMethods()[1];
assertThat(ResourceMetadataRegistry.lookupBlockHandler(clazz, methodName)).isNull();
assertThat(ResourceMetadataRegistry.lookupBlockHandler(clazz, methodName, parameterTypes)).isNull();

ResourceMetadataRegistry.updateBlockHandlerFor(clazz, methodName, null);
assertThat(ResourceMetadataRegistry.lookupBlockHandler(clazz, methodName).isPresent()).isFalse();
ResourceMetadataRegistry.updateBlockHandlerFor(clazz, methodName, parameterTypes, null);
assertThat(ResourceMetadataRegistry.lookupBlockHandler(clazz, methodName, parameterTypes).isPresent()).isFalse();

ResourceMetadataRegistry.updateBlockHandlerFor(clazz, methodName, method);
MethodWrapper wrapper = ResourceMetadataRegistry.lookupBlockHandler(clazz, methodName);
ResourceMetadataRegistry.updateBlockHandlerFor(clazz, methodName, parameterTypes, method);
MethodWrapper wrapper = ResourceMetadataRegistry.lookupBlockHandler(clazz, methodName, parameterTypes);
assertThat(wrapper.isPresent()).isTrue();
assertThat(wrapper.getMethod()).isSameAs(method);
}

@Test(expected = IllegalArgumentException.class)
public void testUpdateBlockHandlerBadArgument() {
ResourceMetadataRegistry.updateBlockHandlerFor(null, "sxs", String.class.getMethods()[0]);
ResourceMetadataRegistry.updateBlockHandlerFor(null, "sxs", new Class<?>[]{}, String.class.getMethods()[0]);
}

@Test(expected = IllegalArgumentException.class)
public void testUpdateFallbackBadArgument() {
ResourceMetadataRegistry.updateBlockHandlerFor(String.class, "", String.class.getMethods()[0]);
ResourceMetadataRegistry.updateBlockHandlerFor(String.class, "", new Class[0], String.class.getMethods()[0]);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
*
* @author Eric Zhao
* @author seasidesky
* @author dowenliu-xyz([email protected])
*/
public abstract class AbstractSentinelInterceptorSupport {

Expand Down Expand Up @@ -170,12 +171,13 @@
}
boolean mustStatic = locationClass != null && locationClass.length >= 1;
Class<?> clazz = mustStatic ? locationClass[0] : ctx.getTarget().getClass();
MethodWrapper m = ResourceMetadataRegistry.lookupFallback(clazz, fallbackName);
Method originMethod = resolveMethod(ctx);
MethodWrapper m = ResourceMetadataRegistry.lookupFallback(clazz, fallbackName, originMethod.getParameterTypes());

Check warning on line 175 in sentinel-extension/sentinel-annotation-cdi-interceptor/src/main/java/com/alibaba/csp/sentinel/annotation/cdi/interceptor/AbstractSentinelInterceptorSupport.java

View check run for this annotation

Codecov / codecov/patch

sentinel-extension/sentinel-annotation-cdi-interceptor/src/main/java/com/alibaba/csp/sentinel/annotation/cdi/interceptor/AbstractSentinelInterceptorSupport.java#L174-L175

Added lines #L174 - L175 were not covered by tests
if (m == null) {
// First time, resolve the fallback.
Method method = resolveFallbackInternal(ctx, fallbackName, clazz, mustStatic);
Method method = resolveFallbackInternal(originMethod, fallbackName, clazz, mustStatic);

Check warning on line 178 in sentinel-extension/sentinel-annotation-cdi-interceptor/src/main/java/com/alibaba/csp/sentinel/annotation/cdi/interceptor/AbstractSentinelInterceptorSupport.java

View check run for this annotation

Codecov / codecov/patch

sentinel-extension/sentinel-annotation-cdi-interceptor/src/main/java/com/alibaba/csp/sentinel/annotation/cdi/interceptor/AbstractSentinelInterceptorSupport.java#L178

Added line #L178 was not covered by tests
// Cache the method instance.
ResourceMetadataRegistry.updateFallbackFor(clazz, fallbackName, method);
ResourceMetadataRegistry.updateFallbackFor(clazz, fallbackName, originMethod.getParameterTypes(), method);

Check warning on line 180 in sentinel-extension/sentinel-annotation-cdi-interceptor/src/main/java/com/alibaba/csp/sentinel/annotation/cdi/interceptor/AbstractSentinelInterceptorSupport.java

View check run for this annotation

Codecov / codecov/patch

sentinel-extension/sentinel-annotation-cdi-interceptor/src/main/java/com/alibaba/csp/sentinel/annotation/cdi/interceptor/AbstractSentinelInterceptorSupport.java#L180

Added line #L180 was not covered by tests
return method;
}
if (!m.isPresent()) {
Expand Down Expand Up @@ -225,9 +227,7 @@
return m.getMethod();
}

private Method resolveFallbackInternal(InvocationContext ctx, /*@NonNull*/ String name, Class<?> clazz,
boolean mustStatic) {
Method originMethod = resolveMethod(ctx);
private Method resolveFallbackInternal(Method originMethod, String name, Class<?> clazz, boolean mustStatic) {
// Fallback function allows two kinds of parameter list.
Class<?>[] defaultParamTypes = originMethod.getParameterTypes();
Class<?>[] paramTypesWithException = Arrays.copyOf(defaultParamTypes, defaultParamTypes.length + 1);
Expand All @@ -254,12 +254,13 @@
// By default current class.
clazz = ctx.getTarget().getClass();
}
MethodWrapper m = ResourceMetadataRegistry.lookupBlockHandler(clazz, name);
Method originMethod = resolveMethod(ctx);
MethodWrapper m = ResourceMetadataRegistry.lookupBlockHandler(clazz, name, originMethod.getParameterTypes());

Check warning on line 258 in sentinel-extension/sentinel-annotation-cdi-interceptor/src/main/java/com/alibaba/csp/sentinel/annotation/cdi/interceptor/AbstractSentinelInterceptorSupport.java

View check run for this annotation

Codecov / codecov/patch

sentinel-extension/sentinel-annotation-cdi-interceptor/src/main/java/com/alibaba/csp/sentinel/annotation/cdi/interceptor/AbstractSentinelInterceptorSupport.java#L257-L258

Added lines #L257 - L258 were not covered by tests
if (m == null) {
// First time, resolve the block handler.
Method method = resolveBlockHandlerInternal(ctx, name, clazz, mustStatic);
Method method = resolveBlockHandlerInternal(originMethod, name, clazz, mustStatic);

Check warning on line 261 in sentinel-extension/sentinel-annotation-cdi-interceptor/src/main/java/com/alibaba/csp/sentinel/annotation/cdi/interceptor/AbstractSentinelInterceptorSupport.java

View check run for this annotation

Codecov / codecov/patch

sentinel-extension/sentinel-annotation-cdi-interceptor/src/main/java/com/alibaba/csp/sentinel/annotation/cdi/interceptor/AbstractSentinelInterceptorSupport.java#L261

Added line #L261 was not covered by tests
// Cache the method instance.
ResourceMetadataRegistry.updateBlockHandlerFor(clazz, name, method);
ResourceMetadataRegistry.updateBlockHandlerFor(clazz, name, originMethod.getParameterTypes(), method);

Check warning on line 263 in sentinel-extension/sentinel-annotation-cdi-interceptor/src/main/java/com/alibaba/csp/sentinel/annotation/cdi/interceptor/AbstractSentinelInterceptorSupport.java

View check run for this annotation

Codecov / codecov/patch

sentinel-extension/sentinel-annotation-cdi-interceptor/src/main/java/com/alibaba/csp/sentinel/annotation/cdi/interceptor/AbstractSentinelInterceptorSupport.java#L263

Added line #L263 was not covered by tests
return method;
}
if (!m.isPresent()) {
Expand All @@ -268,9 +269,7 @@
return m.getMethod();
}

private Method resolveBlockHandlerInternal(InvocationContext ctx, /*@NonNull*/ String name, Class<?> clazz,
boolean mustStatic) {
Method originMethod = resolveMethod(ctx);
private Method resolveBlockHandlerInternal(Method originMethod, String name, Class<?> clazz, boolean mustStatic) {
Class<?>[] originList = originMethod.getParameterTypes();
Class<?>[] parameterTypes = Arrays.copyOf(originList, originList.length + 1);
parameterTypes[parameterTypes.length - 1] = BlockException.class;
Expand Down
Loading
Loading