Skip to content

Java 8 parameter name discovery for methods and constructors [SPR-9643] #14277

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

Closed
spring-projects-issues opened this issue Jul 26, 2012 · 6 comments
Assignees
Labels
type: task A general task
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Jul 26, 2012

Juergen Hoeller opened SPR-9643 and commented

http://openjdk.java.net/jeps/118


This issue is a sub-task of #14273

0 votes, 7 watchers

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Fully integrated for 4.0 RC1 now: With the Parameter.isNamePresent() being available as of OpenJDK 8 b100, we can use a DefaultParameterNameDiscoverer which checks JDK 8's standard reflection first and then falls back to ASM-based debug symbol analysis. Anywhere where we've been defaulting to LocalVariableTableParameterNameDiscoverer before, we're using this new DefaultParameterNameDiscoverer now.

@spring-projects-issues
Copy link
Collaborator Author

Nick Williams commented

I have a question about this specific new feature in relation to SpEL, and about parameter name discovery in general in relation to SpEL. This also pertains to Spring Security (pre-post annotations), so Rob Winch might want to chime in as well.

My (albeit limited and possibly mistaken) understanding of Spring Framework's parameter name discovery and SpEL is that you can always rely on p0, p1, p2, etc., as "parameter names" within SpEL expressions as an alternative to using actual parameter names, where 0, 1, 2, etc. are the parameter indices. The reasoning for this, I assume, is that debug symbols are not always available. The documentation about this feature is limited in Spring Framework (nothing in the SpEL reference docs, and one sentence in the caching reference docs) and nonexistent in Spring Security (e.g., it's not clear if it can actually work this way in Spring Security). The only reason I even know about this alternative at all is from a few mentions in the forums.

The are several observations regarding parameter name discovery, and they can generally be divided into pre- and post-Java-8 categories:

Before Java 8

  • True parameter name discovery requires debug symbols to be present in order to work properly.
  • Without debug symbols, parameter names are exposed as a0, a1, a2, etc.

Starting With Java 8

  • True parameter name discovery can usually always happen. However, the class file feature that enables it is an optional feature. Compilers are not compelled to support it. If not supported, reflection returns arg0, arg1, arg2, etc. instead.
  • Spring Framework's code checks isNamePresent first before returning the reflected names. What's not clear here is what happens if isNamePresent returns false. Do we fall back to a0, a1, a2, etc. again?

Because of these problems, I feel it's important for parameter "names" to always be available as p0, p1, p2, etc. (in addition to discovered names), whether it's in caching, Spring Security, or elsewhere. It would seem that one simply cannot rely on parameter name information always being available, even with the new Java 8 features. In fact, a common problem I'm seeing while reading forums and Stack Overflow is that, even with debug symbols, Spring Security's @PreAuthorize parameter name discovery doesn't always work because other proxies get in the way and interfere with determining the correct name. If I can rely on p0, p1, p2, etc. always being available, I would (personally) always use these instead of names to ensure that my code can always run without error.

At the very least, I feel like the Spring Framework and Spring Security documentation could use some improvement on this topic. Is there something we can always rely on, even in the absence of Java 8 and debug symbols or the presence of a labyrinth of proxies? What is it? If there isn't, then I think some changes need to be made (either in SF, SS, or both), and I understand that may mean separate issues get created.

The truth is that debug symbols aren't always desirable in production environments, and "turn on debug symbols" is not an acceptable answer when someone wonders why they can't use method parameters in a SpEL expression. Clearer documentation, and better implementation if necessary, would be a big improvement, IMO.

@spring-projects-issues
Copy link
Collaborator Author

Rob Winch commented

Nick Williams The Parameter Name discovery is not really limited to SpEL, but is more general. For example, RequestMappingHandlerAdapter uses ParameterNameDiscoverer for binding parameters to Spring MVC arguments from the HTTP servlet request. Spring Security does, however, use the ParameterNameDiscoverer for determining what is available in the SpEL expressions.

The constant you are looking for is that much of the framework allows use of annotations to specify the parameter name. For example, Spring Web MVC reference states:

To process the @PathVariable annotation, Spring MVC needs to find the matching URI template variable by name. You can specify it in the annotation:
...
Or if the URI template variable name matches the method argument name you can omit that detail. As long as your code is not compiled without debugging information, Spring MVC will match the method argument name to the URI template variable name:

Spring Security currently suffers from being able to bind parameter names on interfaces. Since interfaces do not contain debug symbols, the parameter names cannot be discovered. This can be resolved by using JDK 8 with StandardReflectionParameterNameDiscoverer (or DefaultParameterNameDiscoverer if you want fallback options) if -parameter argument is specified to the compiler. Once resolved, SEC-2151 will allow using annotations to specify the parameter name for Spring Security in a consistent way.

@spring-projects-issues
Copy link
Collaborator Author

Nick Williams commented

Understood that discovery is broader that SpEL; my questions were SpEL-specific.

But what about matching by parameter index like p0, p1, etc.? Does/can Spring security do this? It seems if this works in the caching system, other Spring projects should follow that convention. That would be the simplest solution. By the time I'm done writing my interfaces, method definitions take 10-15 lines because of all of the Bean Validation-related annotations. I really don't want to add yet another annotation...

@spring-projects-issues
Copy link
Collaborator Author

Rob Winch commented

Understood that discovery is broader that SpEL; my questions were SpEL-specific. But what about matching by parameter index like p0, p1, etc.? Does/can Spring security do this?

I guess I was confused by the context of your comment. Specifically you are commenting on a JIRA pertaining to ParameterNameDiscoverer with JDK 8 and the question seems more related to SpEL.

It seems if this works in the caching system, other Spring projects should follow that convention.

ParameterNameDiscoverer API could allow an implementation that falls back to use p0, p1, etc. However, the API does not allow for supporting both the parameter names as defined by the JDK and and p0, p1, etc simultaneously. The cache abstraction embeds this logic into LazyParamAwareEvaluationContext (its SpEL EvaluationContext).

In summary...

  • Yes Spring Security (and other projects) could allow for using the same conventions for parameter names within their SpEL context as the Spring cache abstraction.
  • ParameterNameDiscoverer is probably not the place that the logic for discovering multiple parameter names would be implemented because the API does not currently support multiple parameter names
  • You could easily inject a ParameterNameDiscoverer that always uses the p0, p1 naming conventions. However, it would not allow for supporting multiple parameter names for the same parameter at once.
  • If this is something you want, you should probably create the appropriate JIRAs in the respective projects

@spring-projects-issues
Copy link
Collaborator Author

Nick Williams commented

Okay. That all makes sense. I'll open an SEC issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: task A general task
Projects
None yet
Development

No branches or pull requests

2 participants