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

Not annotation pointcut matches dynamic proxy methods incorrectly #27119

Closed
quaff opened this issue Jul 2, 2021 · 7 comments · Fixed by #30534
Closed

Not annotation pointcut matches dynamic proxy methods incorrectly #27119

quaff opened this issue Jul 2, 2021 · 7 comments · Fixed by #30534
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: superseded An issue that has been superseded by another type: bug A general bug

Comments

@quaff
Copy link
Contributor

quaff commented Jul 2, 2021

"target" and "not @annotation" combination not works as expected, but "target" and "@annotation" combination works fine.

	// works as expected
	@Around("execution(public * *(..)) and target(repository) and @annotation(org.springframework.transaction.annotation.Transactional)")
	public Object interceptPositive(ProceedingJoinPoint pjp, Repository repository) throws Throwable {
		Method method = ((MethodSignature) pjp.getSignature()).getMethod();
		if (!method.isAnnotationPresent(Transactional.class))
			throw new RuntimeException("annotation should present here");
		return pjp.proceed();
	}

	// not works as expected
	@Around("execution(public * *(..)) and target(repository) and not @annotation(org.springframework.transaction.annotation.Transactional)")
	public Object interceptNegative(ProceedingJoinPoint pjp, Repository repository) throws Throwable {
		Method method = ((MethodSignature) pjp.getSignature()).getMethod();
		if (method.isAnnotationPresent(Transactional.class))
			throw new RuntimeException("annotation should not present here");
		return pjp.proceed();
	}

test-pointcut.zip is a test project

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jul 2, 2021
@sbrannen
Copy link
Member

sbrannen commented Jul 6, 2021

@quaff, have you tried the following?

@Around("execution(public * *(..)) and target(repository) and !@annotation(org.springframework.transaction.annotation.Transactional)")

@sbrannen sbrannen added in: core Issues in core modules (aop, beans, core, context, expression) status: waiting-for-feedback We need additional information before we can continue labels Jul 6, 2021
@sbrannen sbrannen changed the title Pointcut "target" and "not @annotation" combination not works as expected Pointcut "target" and "not @annotation" combination does not work as expected Jul 6, 2021
@quaff
Copy link
Contributor Author

quaff commented Jul 7, 2021

@quaff, have you tried the following?

@Around("execution(public * *(..)) and target(repository) and !@annotation(org.springframework.transaction.annotation.Transactional)")

Yes, same result.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jul 7, 2021
@sbrannen sbrannen added this to the Triage Queue milestone Jan 3, 2022
@jhoeller jhoeller self-assigned this Jan 4, 2022
@jhoeller jhoeller added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 18, 2022
@quaff
Copy link
Contributor Author

quaff commented Mar 8, 2023

Still not fixed in v6.0.6.

@sbrannen sbrannen removed the status: feedback-provided Feedback has been provided label Mar 8, 2023
@sbrannen sbrannen changed the title Pointcut "target" and "not @annotation" combination does not work as expected Pointcut target and not @annotation combination does not work as expected Mar 8, 2023
@sbrannen sbrannen modified the milestones: General Backlog, 6.1.x Mar 8, 2023
@sbrannen
Copy link
Member

sbrannen commented Mar 8, 2023

Still not fixed in v6.0.6.

Indeed.

I've just assigned to this to the 6.1.x backlog, so that somebody from the team can pick it up.

@nakulmitra
Copy link

nakulmitra commented May 14, 2023

@sbrannen what if I used && in place of and, will it work?

@Around("execution(public * *(..)) && target(repository) && !@annotation(org.springframework.transaction.annotation.Transactional)")

@quaff
Copy link
Contributor Author

quaff commented May 15, 2023

@sbrannen what if I used && in place of and, will it work?

@Around("execution(public * *(..)) && target(repository) && !@annotation(org.springframework.transaction.annotation.Transactional)")

I don't think it will works, && is simply substitution of and.

@quaff quaff changed the title Pointcut target and not @annotation combination does not work as expected Not annotation pointcut matches dynamic proxy methods incorrectly May 24, 2023
quaff added a commit to quaff/spring-framework that referenced this issue May 24, 2023
Prior to this commit, AspectJExpressionPointcut will use method on dynamic proxy class for matching,
such method doesn't retain annotations then `!@(annotation)` could be false positive,
we should use the method on interface instead.

Fix spring-projectsGH-27119
quaff added a commit to quaff/spring-framework that referenced this issue May 24, 2023
Prior to this commit, AspectJExpressionPointcut will use method on dynamic proxy class for matching,
such method doesn't retain annotations then `!@(annotation)` could be false positive,
we should use the method on interface instead.

Fix spring-projectsGH-27119
quaff added a commit to quaff/spring-framework that referenced this issue May 24, 2023
Prior to this commit, AspectJExpressionPointcut doesn't fall back to original method if `!@annotation()` is used, it can cause false positive result.

Fix spring-projectsGH-27119
quaff added a commit to quaff/spring-framework that referenced this issue May 25, 2023
Prior to this commit, AspectJExpressionPointcut doesn't fall back to original method if `!@annotation()` is used, it can cause false positive result.

Fix spring-projectsGH-27119
@jhoeller
Copy link
Contributor

Superseded by PR #30534.

@jhoeller jhoeller added the status: superseded An issue that has been superseded by another label Nov 23, 2023
@jhoeller jhoeller removed this from the 6.1.x milestone Nov 23, 2023
jhoeller pushed a commit that referenced this issue Nov 23, 2023
Prior to this commit, AspectJExpressionPointcut doesn't fall back to original method if `!@annotation()` is used, it can cause false positive result.

Fix GH-27119
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: superseded An issue that has been superseded by another type: bug A general bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants