From 46016d090b241527ab43397d3a72e5865d576aef Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Wed, 16 Dec 2015 15:24:35 +0100 Subject: [PATCH] Use LinkedHashmap to preserve insert order In several places in the spring-webmvc module, URL patterns / objects relationships are kept in `HashMap`s. When matching with actual URLs, the algorithm uses a pattern comparator to sort the matching patterns and select the most specific. But the underlying collection implementation does not keep the original order which can lead to inconsistencies. This commit changes the underlying collection implementation to `LinkedHashmap`s, in order to keep the insert order if the comparator does not reorder entries. Issue: SPR-13798 (cherry picked from commit 3be35c0) --- .../handler/SimpleUrlHandlerMapping.java | 15 ++++----- .../servlet/resource/ResourceUrlProvider.java | 23 +++++++------ .../resource/VersionResourceResolver.java | 32 +++++++++---------- 3 files changed, 37 insertions(+), 33 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/SimpleUrlHandlerMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/SimpleUrlHandlerMapping.java index abef0a7ff7e..16a333c2435 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/SimpleUrlHandlerMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/SimpleUrlHandlerMapping.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,7 +16,7 @@ package org.springframework.web.servlet.handler; -import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.Map; import java.util.Properties; @@ -41,11 +41,10 @@ import org.springframework.util.CollectionUtils; * If the path doesn't begin with a slash, one is prepended. * *

Supports direct matches (given "/test" -> registered "/test") and "*" - * matches (given "/test" -> registered "/t*"). Note that the default is - * to map within the current servlet mapping if applicable; see the - * {@link #setAlwaysUseFullPath "alwaysUseFullPath"} property for details. - * For details on the pattern options, see the - * {@link org.springframework.util.AntPathMatcher} javadoc. + * pattern matches (given "/test" -> registered "/t*"). Note that the default + * is to map within the current servlet mapping if applicable; see the + * {@link #setAlwaysUseFullPath "alwaysUseFullPath"} property. For details on the + * pattern options, see the {@link org.springframework.util.AntPathMatcher} javadoc. * @author Rod Johnson * @author Juergen Hoeller @@ -55,7 +54,7 @@ import org.springframework.util.CollectionUtils; */ public class SimpleUrlHandlerMapping extends AbstractUrlHandlerMapping { - private final Map urlMap = new HashMap(); + private final Map urlMap = new LinkedHashMap(); /** diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlProvider.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlProvider.java index dcd29275675..a79000d881c 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlProvider.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlProvider.java @@ -19,7 +19,7 @@ package org.springframework.web.servlet.resource; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; -import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import javax.servlet.http.HttpServletRequest; @@ -55,7 +55,7 @@ public class ResourceUrlProvider implements ApplicationListener handlerMap = new HashMap(); + private final Map handlerMap = new LinkedHashMap(); private boolean autodetect = true; @@ -165,17 +165,17 @@ public class ResourceUrlProvider implements ApplicationListener matchingPatterns = new ArrayList(); for (String pattern : this.handlerMap.keySet()) { if (getPathMatcher().match(pattern, lookupPath)) { matchingPatterns.add(pattern); } } + if (!matchingPatterns.isEmpty()) { Comparator patternComparator = getPathMatcher().getPatternComparator(lookupPath); Collections.sort(matchingPatterns, patternComparator); @@ -213,7 +215,7 @@ public class ResourceUrlProvider implements ApplicationListener VersionStrategy */ - private final Map versionStrategyMap = new HashMap(); + private final Map versionStrategyMap = new LinkedHashMap(); /** @@ -146,14 +146,14 @@ public class VersionResourceResolver extends AbstractResourceResolver { String candidateVersion = versionStrategy.extractVersion(requestPath); if (StringUtils.isEmpty(candidateVersion)) { if (logger.isTraceEnabled()) { - logger.trace("No version found in path=\"" + requestPath + "\""); + logger.trace("No version found in path \"" + requestPath + "\""); } return null; } String simplePath = versionStrategy.removeVersion(requestPath, candidateVersion); if (logger.isTraceEnabled()) { - logger.trace("Extracted version from path, re-resolving without version, path=\"" + simplePath + "\""); + logger.trace("Extracted version from path, re-resolving without version: \"" + simplePath + "\""); } Resource baseResource = chain.resolveResource(request, simplePath, locations); @@ -164,14 +164,14 @@ public class VersionResourceResolver extends AbstractResourceResolver { String actualVersion = versionStrategy.getResourceVersion(baseResource); if (candidateVersion.equals(actualVersion)) { if (logger.isTraceEnabled()) { - logger.trace("resource matches extracted version"); + logger.trace("Resource matches extracted version ["+ candidateVersion + "]"); } return baseResource; } else { if (logger.isTraceEnabled()) { - logger.trace("Potential resource found for [" + requestPath + "], but version [" + - candidateVersion + "] doesn't match."); + logger.trace("Potential resource found for \"" + requestPath + "\", but version [" + + candidateVersion + "] does not match"); } return null; } @@ -186,12 +186,12 @@ public class VersionResourceResolver extends AbstractResourceResolver { return null; } if (logger.isTraceEnabled()) { - logger.trace("Getting the original resource to determine version"); + logger.trace("Getting the original resource to determine version for path \"" + resourceUrlPath + "\""); } Resource resource = chain.resolveResource(null, baseUrl, locations); String version = versionStrategy.getResourceVersion(resource); if (logger.isTraceEnabled()) { - logger.trace("Version=" + version); + logger.trace("Determined version [" + version + "] for " + resource); } return versionStrategy.addVersion(baseUrl, version); } @@ -204,17 +204,17 @@ public class VersionResourceResolver extends AbstractResourceResolver { */ protected VersionStrategy getStrategyForPath(String requestPath) { String path = "/".concat(requestPath); - List matchingPatterns = new ArrayList(); + List matchingPatterns = new ArrayList(); for (String pattern : this.versionStrategyMap.keySet()) { if (this.pathMatcher.match(pattern, path)) { - matchingPatterns.add(pattern); + matchingPatterns.add(pattern); } } - if (!matchingPatterns.isEmpty()) { - Comparator comparator = this.pathMatcher.getPatternComparator(path); - Collections.sort(matchingPatterns, comparator); - return this.versionStrategyMap.get(matchingPatterns.get(0)); - } + if (!matchingPatterns.isEmpty()) { + Comparator comparator = this.pathMatcher.getPatternComparator(path); + Collections.sort(matchingPatterns, comparator); + return this.versionStrategyMap.get(matchingPatterns.get(0)); + } return null; }