Issue 1:
Ari Miller opened SPR-4551 and commented

My description of the problem below uses the 2.5.2 version of the classes described. Note the problem also occurs in 2.5.1.

We believe we have discovered an issue with the RequestMapping method resolution logic, that can cause particular requests to be handled by the wrong RequestMapping method based on the order in which the controller methods are returned by Class.getDeclaredMethods() (which is not deterministic).

The AnnotationMethodHandlerAdapter.ServletHandlerMethodResolver.resolveHandlerMethod will return the appropriate method to invoke for a given request. It starts with a series of methods that might potentially be used to handle the request, based on the @RequestMapping annotating those methods in the target handler. The problems stems from the dependency on initial order in the logic used to select from multiple potential methods:

Lines 452 - 472 of AnnotationMethodHandlerAdapter:

[CODE]

else if (!targetHandlerMethods.isEmpty()) {

RequestMappingInfo bestMappingMatch = null;

String bestPathMatch = null;

for (RequestMappingInfo mapping : targetHandlerMethods.keySet()) {

String mappedPath = targetPathMatches.get(mapping);

if (bestMappingMatch == null) {

bestMappingMatch = mapping;

bestPathMatch = mappedPath;

}

else {

if ((mappedPath != null && (bestPathMatch == null ||

mappedPath.equals(lookupPath) || bestPathMatch.length() < mappedPath.length())) ||

(bestMappingMatch.methods.length == 0 && mapping.methods.length > 0) ||

bestMappingMatch.params.length < mapping.params.length) {

bestMappingMatch = mapping;

bestPathMatch = mappedPath;

}

}

}

return targetHandlerMethods.get(bestMappingMatch);

}

[/CODE]

Note that the if((mappedPath != null ... ) logic gives multiple possible opportunities for a method to be deemed a betterMappingMatch than the current best match. Given certain requestMapping annotations, this results in whichever method is last in the LinkedHashMap targetHandlerMethods being returned, because the last method will meet one of the || conditions in that if statement.

Here is a specific example, with two methods on a single controller that could handle an incoming request with the path /enterAccessCode.do:

@RequestMapping("/**/enterAccessCode.do")

public ModelAndView methodWithPathMapping -- this is the method we want to handle the request

@RequestMapping(method = {RequestMethod.GET, RequestMethod.POST})

public ModelAndView methodWithMethodMapping()

Say the LinkedHashMap targetHandlerMethods has the following order:

methodWithPathMapping

methodWithMethodMapping

Initially, the bestMappingMatch starts with methodWithPathMapping.

When it gets to the if statement, methodWithMethodMapping becomes the bestMappingMatch, because this part of the statement is true:

(bestMappingMatch.methods.length == 0 && mapping.methods.length > 0)

This is not the desired behavior.

If you have the reverse order:

methodWithMethodMapping

methodWithPathMapping

methodWithMethodMapping starts out as the bestMappingMatch, but know the first part of the if statement is true, so methodWithPathMapping becomes the best match.

As to this not being deterministic:

HandlerMethodResolver.handlerMethods is a LinkedHashSet created based on ReflectionUtils.doWithMethods, which in turn depends on the

result of targetClass.getDeclaredMethods() (Javadoc declares: The elements in the array returned are not sorted and are not in any particular order). HandlerMethodResolver.handlerMethods is iterated through to create targetHandlerMethods, which is once again then not in a deterministic order. We've seen this result in different behavior depending on the JVM (and I think hardware) -- some hardware consistently uses the appropriate method to handle the incoming request, some hardware, because of the different method order, uses the inappropriate methodWithMethodMapping.

Our workaround for this is to avoid having our general handler use method = {RequestMethod.GET, RequestMethod.POST} for the methodWithMethodMapping -- this makes all of the if statement || blocks false.

My claim is that the logic to determine the best mapping match in the if block should be insensitive to initial order when finding the bestMappingMatch.

Here is a crude and untested way to accomplish that:

Replace:

[CODE]

if ((mappedPath != null && (bestPathMatch == null ||

mappedPath.equals(lookupPath) || bestPathMatch.length() < mappedPath.length())) ||

(bestMappingMatch.methods.length == 0 && mapping.methods.length > 0) ||

bestMappingMatch.params.length < mapping.params.length) {

bestMappingMatch = mapping;

bestPathMatch = mappedPath;

}

[/CODE]

with

[CODE]

private boolean isBetterPathMatch(String mappedPath, String mappedPathToCompare, String lookupPath) {

return (mappedPath != null && (mappedPathToCompare == null ||

mappedPath.equals(lookupPath) || mappedPathToCompare.length() < mappedPath.length()));

}

private boolean isBetterMethodMatch(RequestMappingInfo mapping, RequestMappingInfo mappingToCompare) {

return mappingToCompare.methods.length == 0 && mapping.methods.length > 0;

}

private boolean isBetterParamMatch(RequestMappingInfo mapping, RequestMappingInfo mappingToCompare) {

return mappingToCompare.params.length < mapping.params.length;

}

if (isBetterPathMatch(mapppedPath, bestPathMatch, lookupPath) ||

(! isBetterPathMatch(bestPathMatch, mappedPath, lookupPath) && isBetterMethodMatch(mapping, bestMappingMatch)) ||

(! isBetterPathMatch(bestPathMatch, mappedPath, lookupPath) && ! isBetterMethodMatch(bestMappingMatch, mapping) && isBetterParamMatch(mapping, bestMappingMatch)) {

bestMappingMatch = mapping;

bestPathMatch = mappedPath;

}

[/CODE]

Affects: 2.5.1, 2.5.2

Attachments:

4551Fix.diff (2.63 kB)

AnnotationMethodHandlerAdapter.java (29.97 kB)

SpringAnnotationMethodHandlerAdapterNotDeterministicTest.java (3.74 kB)

Issue Links:

#9744 @RequestMapping method resolution issue with duplicate paths

Issue 2:
Abhishek Gupta opened SPR-4873 and commented

Sub: Additional sql-state classification required for Sybase and DB2

We currently classify SQLExceptions as Transient or Non-Transient based on SQL-States we provide in a configuration file.

Following are the SQL-States for DB2 and Sybase that we use:

SQLStates for DB2,DB2UDB:

08001 Transient Exception No Connection (application requester could not connect to the target.)

40003 Transient Exception No Connection

S1000 Transient Exception Communication Failure

57011 Transient Exception Out of virtual memory

57019 Transient Exception DB currently in use

40001 Transient Exception Deadlock detected

23505 Non-Transient Exception Duplicate Key

SQLStates for Sybase:

JZ006 Transient Exception Unexpected I/O error

JW0I0 Transient Exception Internal error with a timed I/O stream

JZ0I1 Transient Exception I/O timeout

JZ0I2 Transient Exception I/O timeout

JZ0C0 Transient Exception Connecton already closed

JZ0P1 Transient Exception Unexptected result type

JZ0EM Transient Exception End of Data

JZ0P4 Transient Exception Protocol Error

40001 Transient Exception Deadlock detected

23000 Non-Transient Data Integrity Violated

The descriptions for the above sybase sql-states can be found here: http://manuals.sybase.com/onlinebooks/group-jcarc/jcg0400e/jcrg/`@Generic__BookTextView`/7402

Not all of the above given error-states are supported in spring's SQLExceptionTranslator. It would be nice to have these available out of the box in spring, so we can completely replace our sql-state based error-handling with that of spring's.

Affects: 2.5.2, 2.5.3, 2.5.4

Issue Links:

#9575 Adding hook into SQL State based exception translation