From 53b39eb753300bfeb385d19de062267d0cbd98a9 Mon Sep 17 00:00:00 2001 From: Astushi Yoshikawa Date: Thu, 5 Dec 2019 21:47:23 +0900 Subject: [PATCH 1/2] MvcUriComponentsBuilder prepends slash See gh-24143 --- .../annotation/MvcUriComponentsBuilder.java | 10 +++--- .../MvcUriComponentsBuilderTests.java | 33 +++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilder.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilder.java index 27ef77bf84d..0107807b55b 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilder.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilder.java @@ -102,6 +102,8 @@ public class MvcUriComponentsBuilder { */ public static final String MVC_URI_COMPONENTS_CONTRIBUTOR_BEAN_NAME = "mvcUriComponentsContributor"; + /** Path separator: "/". */ + public static final String PATH_SEPARATOR = "/"; private static final Log logger = LogFactory.getLog(MvcUriComponentsBuilder.class); @@ -545,7 +547,7 @@ public class MvcUriComponentsBuilder { String typePath = getClassMapping(controllerType); String methodPath = getMethodMapping(method); String path = pathMatcher.combine(typePath, methodPath); - builder.path(path); + builder.path(path.startsWith(PATH_SEPARATOR) ? path : PATH_SEPARATOR + path); return applyContributors(builder, method, args); } @@ -576,11 +578,11 @@ public class MvcUriComponentsBuilder { Assert.notNull(controllerType, "'controllerType' must not be null"); RequestMapping mapping = AnnotatedElementUtils.findMergedAnnotation(controllerType, RequestMapping.class); if (mapping == null) { - return "/"; + return PATH_SEPARATOR; } String[] paths = mapping.path(); if (ObjectUtils.isEmpty(paths) || !StringUtils.hasLength(paths[0])) { - return "/"; + return PATH_SEPARATOR; } if (paths.length > 1 && logger.isTraceEnabled()) { logger.trace("Using first of multiple paths on " + controllerType.getName()); @@ -596,7 +598,7 @@ public class MvcUriComponentsBuilder { } String[] paths = requestMapping.path(); if (ObjectUtils.isEmpty(paths) || !StringUtils.hasLength(paths[0])) { - return "/"; + return PATH_SEPARATOR; } if (paths.length > 1 && logger.isTraceEnabled()) { logger.trace("Using first of multiple paths on " + method.toGenericString()); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilderTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilderTests.java index 103118d8ebc..52ec013363e 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilderTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilderTests.java @@ -434,6 +434,20 @@ public class MvcUriComponentsBuilderTests { assertThat(url).isEqualTo("/base/people/_%2B_/addresses/DE%3BFR"); } + @Test + public void fromMappingNameWithNonSlashPath() { + + initWebApplicationContext(NonSlashPathWebConfig.class); + + this.request.setServerName("example.org"); + this.request.setServerPort(9999); + this.request.setContextPath("/base"); + + String mappingName = "NSPC#getAddressesForCountry"; + String url = fromMappingName(mappingName).arg(0, "DE;FR").encode().buildAndExpand("_+_"); + assertThat(url).isEqualTo("/base/people/DE%3BFR"); + } + @Test public void fromControllerWithPrefix() { @@ -499,6 +513,15 @@ public class MvcUriComponentsBuilderTests { } + @RequestMapping({"people"}) + static class NonSlashPathController { + + @RequestMapping("/{country}") + HttpEntity getAddressesForCountry(@PathVariable String country) { + return null; + } + } + @RequestMapping({"/persons", "/people"}) private class InvalidController { } @@ -622,6 +645,16 @@ public class MvcUriComponentsBuilderTests { } + @EnableWebMvc + static class NonSlashPathWebConfig implements WebMvcConfigurer { + + @Bean + public NonSlashPathController controller() { + return new NonSlashPathController(); + } + } + + @EnableWebMvc static class PathPrefixWebConfig implements WebMvcConfigurer { From d509d5ae6f413c623a5e763bf832e604090d6cad Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 8 Jan 2020 16:46:53 +0000 Subject: [PATCH 2/2] Polishing contribution See gh-24143 --- .../annotation/MvcUriComponentsBuilder.java | 15 ++++++++------- .../MvcUriComponentsBuilderTests.java | 17 ++++++++--------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilder.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilder.java index 0107807b55b..765823866b8 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilder.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilder.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -102,8 +102,6 @@ public class MvcUriComponentsBuilder { */ public static final String MVC_URI_COMPONENTS_CONTRIBUTOR_BEAN_NAME = "mvcUriComponentsContributor"; - /** Path separator: "/". */ - public static final String PATH_SEPARATOR = "/"; private static final Log logger = LogFactory.getLog(MvcUriComponentsBuilder.class); @@ -547,7 +545,10 @@ public class MvcUriComponentsBuilder { String typePath = getClassMapping(controllerType); String methodPath = getMethodMapping(method); String path = pathMatcher.combine(typePath, methodPath); - builder.path(path.startsWith(PATH_SEPARATOR) ? path : PATH_SEPARATOR + path); + if (StringUtils.hasLength(path) && !path.startsWith("/")) { + path = "/" + path; + } + builder.path(path); return applyContributors(builder, method, args); } @@ -578,11 +579,11 @@ public class MvcUriComponentsBuilder { Assert.notNull(controllerType, "'controllerType' must not be null"); RequestMapping mapping = AnnotatedElementUtils.findMergedAnnotation(controllerType, RequestMapping.class); if (mapping == null) { - return PATH_SEPARATOR; + return "/"; } String[] paths = mapping.path(); if (ObjectUtils.isEmpty(paths) || !StringUtils.hasLength(paths[0])) { - return PATH_SEPARATOR; + return "/"; } if (paths.length > 1 && logger.isTraceEnabled()) { logger.trace("Using first of multiple paths on " + controllerType.getName()); @@ -598,7 +599,7 @@ public class MvcUriComponentsBuilder { } String[] paths = requestMapping.path(); if (ObjectUtils.isEmpty(paths) || !StringUtils.hasLength(paths[0])) { - return PATH_SEPARATOR; + return "/"; } if (paths.length > 1 && logger.isTraceEnabled()) { logger.trace("Using first of multiple paths on " + method.toGenericString()); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilderTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilderTests.java index 52ec013363e..99abb24debd 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilderTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MvcUriComponentsBuilderTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 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. @@ -435,15 +435,15 @@ public class MvcUriComponentsBuilderTests { } @Test - public void fromMappingNameWithNonSlashPath() { + public void fromMappingNameWithPathWithoutLeadingSlash() { - initWebApplicationContext(NonSlashPathWebConfig.class); + initWebApplicationContext(PathWithoutLeadingSlashConfig.class); this.request.setServerName("example.org"); this.request.setServerPort(9999); this.request.setContextPath("/base"); - String mappingName = "NSPC#getAddressesForCountry"; + String mappingName = "PWLSC#getAddressesForCountry"; String url = fromMappingName(mappingName).arg(0, "DE;FR").encode().buildAndExpand("_+_"); assertThat(url).isEqualTo("/base/people/DE%3BFR"); } @@ -512,9 +512,8 @@ public class MvcUriComponentsBuilderTests { } } - @RequestMapping({"people"}) - static class NonSlashPathController { + static class PathWithoutLeadingSlashController { @RequestMapping("/{country}") HttpEntity getAddressesForCountry(@PathVariable String country) { @@ -646,11 +645,11 @@ public class MvcUriComponentsBuilderTests { @EnableWebMvc - static class NonSlashPathWebConfig implements WebMvcConfigurer { + static class PathWithoutLeadingSlashConfig implements WebMvcConfigurer { @Bean - public NonSlashPathController controller() { - return new NonSlashPathController(); + public PathWithoutLeadingSlashController controller() { + return new PathWithoutLeadingSlashController(); } }