From 50b954240291d7e4302a679694b83db7096ff254 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Mon, 12 Oct 2020 17:57:43 +0200 Subject: [PATCH] Apply handleMissingValue in case of null conversion result as well Closes gh-23939 --- ...tractNamedValueMethodArgumentResolver.java | 13 +++++--- ...tractNamedValueMethodArgumentResolver.java | 16 ++++++---- ...tractNamedValueMethodArgumentResolver.java | 23 ++++++++------ ...uestHeaderMethodArgumentResolverTests.java | 31 +++++++++++++++++-- .../AbstractNamedValueArgumentResolver.java | 8 ++--- 5 files changed, 64 insertions(+), 27 deletions(-) diff --git a/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/reactive/AbstractNamedValueMethodArgumentResolver.java b/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/reactive/AbstractNamedValueMethodArgumentResolver.java index d69572061d2..2075aa9e889 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/reactive/AbstractNamedValueMethodArgumentResolver.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/reactive/AbstractNamedValueMethodArgumentResolver.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. @@ -82,7 +82,6 @@ public abstract class AbstractNamedValueMethodArgumentResolver implements SyncHa @Override public Object resolveArgumentValue(MethodParameter parameter, Message message) { - NamedValueInfo namedValueInfo = getNamedValueInfo(parameter); MethodParameter nestedParameter = parameter.nestedIfOptional(); @@ -108,6 +107,11 @@ public abstract class AbstractNamedValueMethodArgumentResolver implements SyncHa if (parameter != nestedParameter || !ClassUtils.isAssignableValue(parameter.getParameterType(), arg)) { arg = this.conversionService.convert(arg, TypeDescriptor.forObject(arg), new TypeDescriptor(parameter)); + // Check for null value after conversion of incoming argument value + if (arg == null && namedValueInfo.defaultValue == null && + namedValueInfo.required && !nestedParameter.isOptional()) { + handleMissingValue(namedValueInfo.name, nestedParameter, message); + } } return arg; @@ -144,10 +148,9 @@ public abstract class AbstractNamedValueMethodArgumentResolver implements SyncHa if (info.name.isEmpty()) { name = parameter.getParameterName(); if (name == null) { - Class type = parameter.getParameterType(); throw new IllegalArgumentException( - "Name for argument of type [" + type.getName() + "] not specified, " + - "and parameter name information not found in class file either."); + "Name for argument of type [" + parameter.getNestedParameterType().getName() + + "] not specified, and parameter name information not found in class file either."); } } return new NamedValueInfo(name, info.required, diff --git a/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/support/AbstractNamedValueMethodArgumentResolver.java b/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/support/AbstractNamedValueMethodArgumentResolver.java index de31ebee247..b4a367c8726 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/support/AbstractNamedValueMethodArgumentResolver.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/support/AbstractNamedValueMethodArgumentResolver.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. @@ -80,8 +80,8 @@ public abstract class AbstractNamedValueMethodArgumentResolver implements Handle // Possibly remove after discussion in gh-23882. //noinspection ConstantConditions - this.conversionService = conversionService != null ? - conversionService : DefaultConversionService.getSharedInstance(); + this.conversionService = (conversionService != null ? + conversionService : DefaultConversionService.getSharedInstance()); this.configurableBeanFactory = beanFactory; this.expressionContext = (beanFactory != null ? new BeanExpressionContext(beanFactory, null) : null); @@ -116,6 +116,11 @@ public abstract class AbstractNamedValueMethodArgumentResolver implements Handle if (parameter != nestedParameter || !ClassUtils.isAssignableValue(parameter.getParameterType(), arg)) { arg = this.conversionService.convert(arg, TypeDescriptor.forObject(arg), new TypeDescriptor(parameter)); + // Check for null value after conversion of incoming argument value + if (arg == null && namedValueInfo.defaultValue == null && + namedValueInfo.required && !nestedParameter.isOptional()) { + handleMissingValue(namedValueInfo.name, nestedParameter, message); + } } handleResolvedValue(arg, namedValueInfo.name, parameter, message); @@ -154,10 +159,9 @@ public abstract class AbstractNamedValueMethodArgumentResolver implements Handle if (info.name.isEmpty()) { name = parameter.getParameterName(); if (name == null) { - Class type = parameter.getParameterType(); throw new IllegalArgumentException( - "Name for argument of type [" + type.getName() + "] not specified, " + - "and parameter name information not found in class file either."); + "Name for argument of type [" + parameter.getNestedParameterType().getName() + + "] not specified, and parameter name information not found in class file either."); } } return new NamedValueInfo(name, info.required, diff --git a/spring-web/src/main/java/org/springframework/web/method/annotation/AbstractNamedValueMethodArgumentResolver.java b/spring-web/src/main/java/org/springframework/web/method/annotation/AbstractNamedValueMethodArgumentResolver.java index 7b5ea38f499..c389b6f9c7e 100644 --- a/spring-web/src/main/java/org/springframework/web/method/annotation/AbstractNamedValueMethodArgumentResolver.java +++ b/spring-web/src/main/java/org/springframework/web/method/annotation/AbstractNamedValueMethodArgumentResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 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. @@ -99,7 +99,7 @@ public abstract class AbstractNamedValueMethodArgumentResolver implements Handle NamedValueInfo namedValueInfo = getNamedValueInfo(parameter); MethodParameter nestedParameter = parameter.nestedIfOptional(); - Object resolvedName = resolveStringValue(namedValueInfo.name); + Object resolvedName = resolveEmbeddedValuesAndExpressions(namedValueInfo.name); if (resolvedName == null) { throw new IllegalArgumentException( "Specified name must not resolve to null: [" + namedValueInfo.name + "]"); @@ -108,7 +108,7 @@ public abstract class AbstractNamedValueMethodArgumentResolver implements Handle Object arg = resolveName(resolvedName.toString(), nestedParameter, webRequest); if (arg == null) { if (namedValueInfo.defaultValue != null) { - arg = resolveStringValue(namedValueInfo.defaultValue); + arg = resolveEmbeddedValuesAndExpressions(namedValueInfo.defaultValue); } else if (namedValueInfo.required && !nestedParameter.isOptional()) { handleMissingValue(namedValueInfo.name, nestedParameter, webRequest); @@ -116,7 +116,7 @@ public abstract class AbstractNamedValueMethodArgumentResolver implements Handle arg = handleNullValue(namedValueInfo.name, arg, nestedParameter.getNestedParameterType()); } else if ("".equals(arg) && namedValueInfo.defaultValue != null) { - arg = resolveStringValue(namedValueInfo.defaultValue); + arg = resolveEmbeddedValuesAndExpressions(namedValueInfo.defaultValue); } if (binderFactory != null) { @@ -132,6 +132,11 @@ public abstract class AbstractNamedValueMethodArgumentResolver implements Handle throw new MethodArgumentTypeMismatchException(arg, ex.getRequiredType(), namedValueInfo.name, parameter, ex.getCause()); } + // Check for null value after conversion of incoming argument value + if (arg == null && namedValueInfo.defaultValue == null && + namedValueInfo.required && !nestedParameter.isOptional()) { + handleMissingValue(namedValueInfo.name, nestedParameter, webRequest); + } } handleResolvedValue(arg, namedValueInfo.name, parameter, mavContainer, webRequest); @@ -169,8 +174,8 @@ public abstract class AbstractNamedValueMethodArgumentResolver implements Handle name = parameter.getParameterName(); if (name == null) { throw new IllegalArgumentException( - "Name for argument type [" + parameter.getNestedParameterType().getName() + - "] not available, and parameter name information not found in class file either."); + "Name for argument of type [" + parameter.getNestedParameterType().getName() + + "] not specified, and parameter name information not found in class file either."); } } String defaultValue = (ValueConstants.DEFAULT_NONE.equals(info.defaultValue) ? null : info.defaultValue); @@ -182,13 +187,13 @@ public abstract class AbstractNamedValueMethodArgumentResolver implements Handle * potentially containing placeholders and expressions. */ @Nullable - private Object resolveStringValue(String value) { - if (this.configurableBeanFactory == null) { + private Object resolveEmbeddedValuesAndExpressions(String value) { + if (this.configurableBeanFactory == null || this.expressionContext == null) { return value; } String placeholdersResolved = this.configurableBeanFactory.resolveEmbeddedValue(value); BeanExpressionResolver exprResolver = this.configurableBeanFactory.getBeanExpressionResolver(); - if (exprResolver == null || this.expressionContext == null) { + if (exprResolver == null) { return value; } return exprResolver.evaluate(placeholdersResolved, this.expressionContext); diff --git a/spring-web/src/test/java/org/springframework/web/method/annotation/RequestHeaderMethodArgumentResolverTests.java b/spring-web/src/test/java/org/springframework/web/method/annotation/RequestHeaderMethodArgumentResolverTests.java index 6acee9f04fb..d03ad39d125 100644 --- a/spring-web/src/test/java/org/springframework/web/method/annotation/RequestHeaderMethodArgumentResolverTests.java +++ b/spring-web/src/test/java/org/springframework/web/method/annotation/RequestHeaderMethodArgumentResolverTests.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. @@ -31,6 +31,7 @@ import org.springframework.core.MethodParameter; import org.springframework.core.annotation.SynthesizingMethodParameter; import org.springframework.format.support.DefaultFormattingConversionService; import org.springframework.util.ReflectionUtils; +import org.springframework.web.bind.MissingRequestHeaderException; import org.springframework.web.bind.ServletRequestBindingException; import org.springframework.web.bind.annotation.RequestHeader; import org.springframework.web.bind.support.ConfigurableWebBindingInitializer; @@ -66,6 +67,7 @@ class RequestHeaderMethodArgumentResolverTests { private MethodParameter paramDate; private MethodParameter paramInstant; private MethodParameter paramUuid; + private MethodParameter paramUuidOptional; private MockHttpServletRequest servletRequest; @@ -90,6 +92,7 @@ class RequestHeaderMethodArgumentResolverTests { paramDate = new SynthesizingMethodParameter(method, 7); paramInstant = new SynthesizingMethodParameter(method, 8); paramUuid = new SynthesizingMethodParameter(method, 9); + paramUuidOptional = new SynthesizingMethodParameter(method, 10); servletRequest = new MockHttpServletRequest(); webRequest = new ServletWebRequest(servletRequest, new MockHttpServletResponse()); @@ -265,7 +268,28 @@ class RequestHeaderMethodArgumentResolverTests { ConfigurableWebBindingInitializer bindingInitializer = new ConfigurableWebBindingInitializer(); bindingInitializer.setConversionService(new DefaultFormattingConversionService()); - Object result = resolver.resolveArgument(paramUuid, null, webRequest, + + assertThatExceptionOfType(MissingRequestHeaderException.class).isThrownBy(() -> + resolver.resolveArgument(paramUuid, null, webRequest, + new DefaultDataBinderFactory(bindingInitializer))); + } + + @Test + void uuidConversionWithEmptyValueOptional() throws Exception { + uuidConversionWithEmptyOrBlankValueOptional(""); + } + + @Test + void uuidConversionWithBlankValueOptional() throws Exception { + uuidConversionWithEmptyOrBlankValueOptional(" "); + } + + private void uuidConversionWithEmptyOrBlankValueOptional(String uuid) throws Exception { + servletRequest.addHeader("name", uuid); + + ConfigurableWebBindingInitializer bindingInitializer = new ConfigurableWebBindingInitializer(); + bindingInitializer.setConversionService(new DefaultFormattingConversionService()); + Object result = resolver.resolveArgument(paramUuidOptional, null, webRequest, new DefaultDataBinderFactory(bindingInitializer)); assertThat(result).isNull(); @@ -282,7 +306,8 @@ class RequestHeaderMethodArgumentResolverTests { @RequestHeader("name") Map unsupported, @RequestHeader("name") Date dateParam, @RequestHeader("name") Instant instantParam, - @RequestHeader("name") UUID uuid) { + @RequestHeader("name") UUID uuid, + @RequestHeader(name = "name", required = false) UUID uuidOptional) { } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractNamedValueArgumentResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractNamedValueArgumentResolver.java index c7cd8e3af8c..9c0fb6021ea 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractNamedValueArgumentResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractNamedValueArgumentResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 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. @@ -144,9 +144,9 @@ public abstract class AbstractNamedValueArgumentResolver extends HandlerMethodAr if (info.name.isEmpty()) { name = parameter.getParameterName(); if (name == null) { - String type = parameter.getNestedParameterType().getName(); - throw new IllegalArgumentException("Name for argument type [" + type + "] not " + - "available, and parameter name information not found in class file either."); + throw new IllegalArgumentException( + "Name for argument of type [" + parameter.getNestedParameterType().getName() + + "] not specified, and parameter name information not found in class file either."); } } String defaultValue = (ValueConstants.DEFAULT_NONE.equals(info.defaultValue) ? null : info.defaultValue);