diff --git a/build.gradle b/build.gradle index e5aa6b6b7d1d..6021fa574ddf 100644 --- a/build.gradle +++ b/build.gradle @@ -28,8 +28,8 @@ configure(allprojects) { project -> dependencyManagement { imports { mavenBom "com.fasterxml.jackson:jackson-bom:2.12.5" - mavenBom "io.netty:netty-bom:4.1.69.Final" - mavenBom "io.projectreactor:reactor-bom:2020.0.12" + mavenBom "io.netty:netty-bom:4.1.70.Final" + mavenBom "io.projectreactor:reactor-bom:2020.0.13" mavenBom "io.r2dbc:r2dbc-bom:Arabba-SR10" mavenBom "io.rsocket:rsocket-bom:1.1.1" mavenBom "org.eclipse.jetty:jetty-bom:9.4.44.v20210927" @@ -128,14 +128,14 @@ configure(allprojects) { project -> dependency "org.webjars:webjars-locator-core:0.48" dependency "org.webjars:underscorejs:1.8.3" - dependencySet(group: 'org.apache.tomcat', version: '9.0.53') { + dependencySet(group: 'org.apache.tomcat', version: '9.0.54') { entry 'tomcat-util' entry('tomcat-websocket') { exclude group: "org.apache.tomcat", name: "tomcat-websocket-api" exclude group: "org.apache.tomcat", name: "tomcat-servlet-api" } } - dependencySet(group: 'org.apache.tomcat.embed', version: '9.0.53') { + dependencySet(group: 'org.apache.tomcat.embed', version: '9.0.54') { entry 'tomcat-embed-core' entry 'tomcat-embed-websocket' } @@ -192,7 +192,7 @@ configure(allprojects) { project -> dependency "org.hamcrest:hamcrest:2.1" dependency "org.awaitility:awaitility:3.1.6" dependency "org.assertj:assertj-core:3.21.0" - dependencySet(group: 'org.xmlunit', version: '2.8.2') { + dependencySet(group: 'org.xmlunit', version: '2.8.3') { entry 'xmlunit-assertj' entry('xmlunit-matchers') { exclude group: "org.hamcrest", name: "hamcrest-core" @@ -206,10 +206,10 @@ configure(allprojects) { project -> } dependency "io.mockk:mockk:1.12.0" - dependency("net.sourceforge.htmlunit:htmlunit:2.53.0") { + dependency("net.sourceforge.htmlunit:htmlunit:2.54.0") { exclude group: "commons-logging", name: "commons-logging" } - dependency("org.seleniumhq.selenium:htmlunit-driver:2.53.0") { + dependency("org.seleniumhq.selenium:htmlunit-driver:2.54.0") { exclude group: "commons-logging", name: "commons-logging" } dependency("org.seleniumhq.selenium:selenium-java:3.141.59") { diff --git a/gradle.properties b/gradle.properties index aea3f16f967a..de2ed3c71aba 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,4 +1,4 @@ -version=5.3.12-SNAPSHOT +version=5.3.13 org.gradle.jvmargs=-Xmx1536M org.gradle.caching=true org.gradle.parallel=true diff --git a/spring-beans/src/main/java/org/springframework/beans/PropertyEditorRegistrySupport.java b/spring-beans/src/main/java/org/springframework/beans/PropertyEditorRegistrySupport.java index d1354e1d89b0..e17a4e52e697 100644 --- a/spring-beans/src/main/java/org/springframework/beans/PropertyEditorRegistrySupport.java +++ b/spring-beans/src/main/java/org/springframework/beans/PropertyEditorRegistrySupport.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2021 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. @@ -422,10 +422,13 @@ private PropertyEditor getCustomEditor(@Nullable Class requiredType) { } if (editor == null) { // Find editor for superclass or interface. - for (Iterator> it = this.customEditors.keySet().iterator(); it.hasNext() && editor == null;) { - Class key = it.next(); + for (Map.Entry, PropertyEditor> entry : this.customEditors.entrySet()) { + if (editor != null) { + break; + } + Class key = entry.getKey(); if (key.isAssignableFrom(requiredType)) { - editor = this.customEditors.get(key); + editor = entry.getValue(); // Cache editor for search type, to avoid the overhead // of repeated assignable-from checks. if (this.customEditorCache == null) { diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java b/spring-beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java index e7131a562ef7..37f5884e671e 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java @@ -459,7 +459,7 @@ private InjectionMetadata findAutowiringMetadata(String beanName, Class clazz return metadata; } - private InjectionMetadata buildAutowiringMetadata(final Class clazz) { + private InjectionMetadata buildAutowiringMetadata(Class clazz) { if (!AnnotationUtils.isCandidateClass(clazz, this.autowiredAnnotationTypes)) { return InjectionMetadata.EMPTY; } diff --git a/spring-context-support/src/main/java/org/springframework/scheduling/quartz/SchedulerFactoryBean.java b/spring-context-support/src/main/java/org/springframework/scheduling/quartz/SchedulerFactoryBean.java index 50ef456cfc9a..e0982a2e5ff9 100644 --- a/spring-context-support/src/main/java/org/springframework/scheduling/quartz/SchedulerFactoryBean.java +++ b/spring-context-support/src/main/java/org/springframework/scheduling/quartz/SchedulerFactoryBean.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2021 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. @@ -570,7 +570,7 @@ private void initSchedulerFactory(StdSchedulerFactory schedulerFactory) throws S CollectionUtils.mergePropertiesIntoMap(this.quartzProperties, mergedProps); if (this.dataSource != null) { - mergedProps.setProperty(StdSchedulerFactory.PROP_JOB_STORE_CLASS, LocalDataSourceJobStore.class.getName()); + mergedProps.putIfAbsent(StdSchedulerFactory.PROP_JOB_STORE_CLASS, LocalDataSourceJobStore.class.getName()); } // Determine scheduler name across local settings and Quartz properties... diff --git a/spring-context-support/src/test/java/org/springframework/scheduling/quartz/QuartzSupportTests.java b/spring-context-support/src/test/java/org/springframework/scheduling/quartz/QuartzSupportTests.java index 6c7d5b8691c7..9d461d2e400f 100644 --- a/spring-context-support/src/test/java/org/springframework/scheduling/quartz/QuartzSupportTests.java +++ b/spring-context-support/src/test/java/org/springframework/scheduling/quartz/QuartzSupportTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2021 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. @@ -18,6 +18,7 @@ import java.util.HashMap; import java.util.Map; +import java.util.Properties; import javax.sql.DataSource; @@ -30,6 +31,7 @@ import org.quartz.SchedulerFactory; import org.quartz.impl.JobDetailImpl; import org.quartz.impl.SchedulerRepository; +import org.quartz.impl.jdbcjobstore.JobStoreTX; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.support.BeanDefinitionBuilder; @@ -40,6 +42,8 @@ import org.springframework.core.task.TaskExecutor; import org.springframework.core.testfixture.EnabledForTestGroups; import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.jdbc.datasource.embedded.EmbeddedDatabase; +import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; @@ -57,10 +61,10 @@ * @author Sam Brannen * @since 20.02.2004 */ -public class QuartzSupportTests { +class QuartzSupportTests { @Test - public void schedulerFactoryBeanWithApplicationContext() throws Exception { + void schedulerFactoryBeanWithApplicationContext() throws Exception { TestBean tb = new TestBean("tb", 99); StaticApplicationContext ac = new StaticApplicationContext(); @@ -97,7 +101,7 @@ protected Scheduler createScheduler(SchedulerFactory schedulerFactory, String sc @Test @EnabledForTestGroups(LONG_RUNNING) - public void schedulerWithTaskExecutor() throws Exception { + void schedulerWithTaskExecutor() throws Exception { CountingTaskExecutor taskExecutor = new CountingTaskExecutor(); DummyJob.count = 0; @@ -130,7 +134,7 @@ public void schedulerWithTaskExecutor() throws Exception { @Test @SuppressWarnings({ "unchecked", "rawtypes" }) - public void jobDetailWithRunnableInsteadOfJob() { + void jobDetailWithRunnableInsteadOfJob() { JobDetailImpl jobDetail = new JobDetailImpl(); assertThatIllegalArgumentException().isThrownBy(() -> jobDetail.setJobClass((Class) DummyRunnable.class)); @@ -138,7 +142,7 @@ public void jobDetailWithRunnableInsteadOfJob() { @Test @EnabledForTestGroups(LONG_RUNNING) - public void schedulerWithQuartzJobBean() throws Exception { + void schedulerWithQuartzJobBean() throws Exception { DummyJob.param = 0; DummyJob.count = 0; @@ -171,7 +175,7 @@ public void schedulerWithQuartzJobBean() throws Exception { @Test @EnabledForTestGroups(LONG_RUNNING) - public void schedulerWithSpringBeanJobFactory() throws Exception { + void schedulerWithSpringBeanJobFactory() throws Exception { DummyJob.param = 0; DummyJob.count = 0; @@ -206,7 +210,7 @@ public void schedulerWithSpringBeanJobFactory() throws Exception { @Test @EnabledForTestGroups(LONG_RUNNING) - public void schedulerWithSpringBeanJobFactoryAndParamMismatchNotIgnored() throws Exception { + void schedulerWithSpringBeanJobFactoryAndParamMismatchNotIgnored() throws Exception { DummyJob.param = 0; DummyJob.count = 0; @@ -242,7 +246,7 @@ public void schedulerWithSpringBeanJobFactoryAndParamMismatchNotIgnored() throws @Test @EnabledForTestGroups(LONG_RUNNING) - public void schedulerWithSpringBeanJobFactoryAndQuartzJobBean() throws Exception { + void schedulerWithSpringBeanJobFactoryAndQuartzJobBean() throws Exception { DummyJobBean.param = 0; DummyJobBean.count = 0; @@ -276,7 +280,7 @@ public void schedulerWithSpringBeanJobFactoryAndQuartzJobBean() throws Exception @Test @EnabledForTestGroups(LONG_RUNNING) - public void schedulerWithSpringBeanJobFactoryAndJobSchedulingData() throws Exception { + void schedulerWithSpringBeanJobFactoryAndJobSchedulingData() throws Exception { DummyJob.param = 0; DummyJob.count = 0; @@ -294,7 +298,7 @@ public void schedulerWithSpringBeanJobFactoryAndJobSchedulingData() throws Excep } @Test // SPR-772 - public void multipleSchedulers() throws Exception { + void multipleSchedulers() throws Exception { try (ClassPathXmlApplicationContext ctx = context("multipleSchedulers.xml")) { Scheduler scheduler1 = (Scheduler) ctx.getBean("scheduler1"); Scheduler scheduler2 = (Scheduler) ctx.getBean("scheduler2"); @@ -305,7 +309,7 @@ public void multipleSchedulers() throws Exception { } @Test // SPR-16884 - public void multipleSchedulersWithQuartzProperties() throws Exception { + void multipleSchedulersWithQuartzProperties() throws Exception { try (ClassPathXmlApplicationContext ctx = context("multipleSchedulersWithQuartzProperties.xml")) { Scheduler scheduler1 = (Scheduler) ctx.getBean("scheduler1"); Scheduler scheduler2 = (Scheduler) ctx.getBean("scheduler2"); @@ -317,12 +321,13 @@ public void multipleSchedulersWithQuartzProperties() throws Exception { @Test @EnabledForTestGroups(LONG_RUNNING) - public void twoAnonymousMethodInvokingJobDetailFactoryBeans() throws Exception { - Thread.sleep(3000); + void twoAnonymousMethodInvokingJobDetailFactoryBeans() throws Exception { try (ClassPathXmlApplicationContext ctx = context("multipleAnonymousMethodInvokingJobDetailFB.xml")) { QuartzTestBean exportService = (QuartzTestBean) ctx.getBean("exportService"); QuartzTestBean importService = (QuartzTestBean) ctx.getBean("importService"); + Thread.sleep(400); + assertThat(exportService.getImportCount()).as("doImport called exportService").isEqualTo(0); assertThat(exportService.getExportCount()).as("doExport not called on exportService").isEqualTo(2); assertThat(importService.getImportCount()).as("doImport not called on importService").isEqualTo(2); @@ -332,12 +337,13 @@ public void twoAnonymousMethodInvokingJobDetailFactoryBeans() throws Exception { @Test @EnabledForTestGroups(LONG_RUNNING) - public void schedulerAccessorBean() throws Exception { - Thread.sleep(3000); + void schedulerAccessorBean() throws Exception { try (ClassPathXmlApplicationContext ctx = context("schedulerAccessorBean.xml")) { QuartzTestBean exportService = (QuartzTestBean) ctx.getBean("exportService"); QuartzTestBean importService = (QuartzTestBean) ctx.getBean("importService"); + Thread.sleep(400); + assertThat(exportService.getImportCount()).as("doImport called exportService").isEqualTo(0); assertThat(exportService.getExportCount()).as("doExport not called on exportService").isEqualTo(2); assertThat(importService.getImportCount()).as("doImport not called on importService").isEqualTo(2); @@ -347,7 +353,7 @@ public void schedulerAccessorBean() throws Exception { @Test @SuppressWarnings("resource") - public void schedulerAutoStartsOnContextRefreshedEventByDefault() throws Exception { + void schedulerAutoStartsOnContextRefreshedEventByDefault() throws Exception { StaticApplicationContext context = new StaticApplicationContext(); context.registerBeanDefinition("scheduler", new RootBeanDefinition(SchedulerFactoryBean.class)); Scheduler bean = context.getBean("scheduler", Scheduler.class); @@ -358,7 +364,7 @@ public void schedulerAutoStartsOnContextRefreshedEventByDefault() throws Excepti @Test @SuppressWarnings("resource") - public void schedulerAutoStartupFalse() throws Exception { + void schedulerAutoStartupFalse() throws Exception { StaticApplicationContext context = new StaticApplicationContext(); BeanDefinition beanDefinition = BeanDefinitionBuilder.genericBeanDefinition(SchedulerFactoryBean.class) .addPropertyValue("autoStartup", false).getBeanDefinition(); @@ -370,7 +376,7 @@ public void schedulerAutoStartupFalse() throws Exception { } @Test - public void schedulerRepositoryExposure() throws Exception { + void schedulerRepositoryExposure() throws Exception { try (ClassPathXmlApplicationContext ctx = context("schedulerRepositoryExposure.xml")) { assertThat(ctx.getBean("scheduler")).isSameAs(SchedulerRepository.getInstance().lookup("myScheduler")); } @@ -381,7 +387,7 @@ public void schedulerRepositoryExposure() throws Exception { * TODO: Against Quartz 2.2, this test's job doesn't actually execute anymore... */ @Test - public void schedulerWithHsqlDataSource() throws Exception { + void schedulerWithHsqlDataSource() throws Exception { DummyJob.param = 0; DummyJob.count = 0; @@ -396,12 +402,36 @@ public void schedulerWithHsqlDataSource() throws Exception { } } + @Test + @SuppressWarnings("resource") + void schedulerFactoryBeanWithCustomJobStore() throws Exception { + StaticApplicationContext context = new StaticApplicationContext(); + + String dbName = "mydb"; + EmbeddedDatabase database = new EmbeddedDatabaseBuilder().setName(dbName).build(); + + Properties properties = new Properties(); + properties.setProperty("org.quartz.jobStore.class", JobStoreTX.class.getName()); + properties.setProperty("org.quartz.jobStore.dataSource", dbName); + + BeanDefinition beanDefinition = BeanDefinitionBuilder.genericBeanDefinition(SchedulerFactoryBean.class) + .addPropertyValue("autoStartup", false) + .addPropertyValue("dataSource", database) + .addPropertyValue("quartzProperties", properties) + .getBeanDefinition(); + context.registerBeanDefinition("scheduler", beanDefinition); + + Scheduler scheduler = context.getBean(Scheduler.class); + + assertThat(scheduler.getMetaData().getJobStoreClass()).isEqualTo(JobStoreTX.class); + } + private ClassPathXmlApplicationContext context(String path) { return new ClassPathXmlApplicationContext(path, getClass()); } - public static class CountingTaskExecutor implements TaskExecutor { + private static class CountingTaskExecutor implements TaskExecutor { private int count; @@ -413,12 +443,14 @@ public void execute(Runnable task) { } - public static class DummyJob implements Job { + private static class DummyJob implements Job { private static int param; private static int count; + @SuppressWarnings("unused") + // Must be public public void setParam(int value) { if (param > 0) { throw new IllegalStateException("Param already set"); @@ -433,12 +465,13 @@ public synchronized void execute(JobExecutionContext jobExecutionContext) throws } - public static class DummyJobBean extends QuartzJobBean { + private static class DummyJobBean extends QuartzJobBean { private static int param; private static int count; + @SuppressWarnings("unused") public void setParam(int value) { if (param > 0) { throw new IllegalStateException("Param already set"); @@ -453,7 +486,7 @@ protected synchronized void executeInternal(JobExecutionContext jobExecutionCont } - public static class DummyRunnable implements Runnable { + private static class DummyRunnable implements Runnable { @Override public void run() { diff --git a/spring-context-support/src/test/resources/org/springframework/scheduling/quartz/multipleAnonymousMethodInvokingJobDetailFB.xml b/spring-context-support/src/test/resources/org/springframework/scheduling/quartz/multipleAnonymousMethodInvokingJobDetailFB.xml index e45ccd195feb..58e771f3ad4a 100644 --- a/spring-context-support/src/test/resources/org/springframework/scheduling/quartz/multipleAnonymousMethodInvokingJobDetailFB.xml +++ b/spring-context-support/src/test/resources/org/springframework/scheduling/quartz/multipleAnonymousMethodInvokingJobDetailFB.xml @@ -19,7 +19,7 @@ - + @@ -30,7 +30,7 @@ - + diff --git a/spring-context-support/src/test/resources/org/springframework/scheduling/quartz/schedulerAccessorBean.xml b/spring-context-support/src/test/resources/org/springframework/scheduling/quartz/schedulerAccessorBean.xml index 0ba9c4a57a84..3634002664cc 100644 --- a/spring-context-support/src/test/resources/org/springframework/scheduling/quartz/schedulerAccessorBean.xml +++ b/spring-context-support/src/test/resources/org/springframework/scheduling/quartz/schedulerAccessorBean.xml @@ -21,7 +21,7 @@ - + @@ -32,7 +32,7 @@ - + diff --git a/spring-context/src/main/java/org/springframework/context/annotation/CommonAnnotationBeanPostProcessor.java b/spring-context/src/main/java/org/springframework/context/annotation/CommonAnnotationBeanPostProcessor.java index 8a3ca5a9f889..ecfdf8f83a9a 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/CommonAnnotationBeanPostProcessor.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/CommonAnnotationBeanPostProcessor.java @@ -343,7 +343,7 @@ public PropertyValues postProcessPropertyValues( } - private InjectionMetadata findResourceMetadata(String beanName, final Class clazz, @Nullable PropertyValues pvs) { + private InjectionMetadata findResourceMetadata(String beanName, Class clazz, @Nullable PropertyValues pvs) { // Fall back to class name as cache key, for backwards compatibility with custom callers. String cacheKey = (StringUtils.hasLength(beanName) ? beanName : clazz.getName()); // Quick check on the concurrent map first, with minimal locking. @@ -363,7 +363,7 @@ private InjectionMetadata findResourceMetadata(String beanName, final Class c return metadata; } - private InjectionMetadata buildResourceMetadata(final Class clazz) { + private InjectionMetadata buildResourceMetadata(Class clazz) { if (!AnnotationUtils.isCandidateClass(clazz, resourceAnnotationTypes)) { return InjectionMetadata.EMPTY; } diff --git a/spring-context/src/main/java/org/springframework/context/annotation/ComponentScanAnnotationParser.java b/spring-context/src/main/java/org/springframework/context/annotation/ComponentScanAnnotationParser.java index 90575862c6bd..0a3c751ff395 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/ComponentScanAnnotationParser.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/ComponentScanAnnotationParser.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2021 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,13 +16,10 @@ package org.springframework.context.annotation; -import java.lang.annotation.Annotation; -import java.util.ArrayList; import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; import java.util.Set; -import java.util.regex.Pattern; import org.springframework.beans.BeanUtils; import org.springframework.beans.factory.config.BeanDefinitionHolder; @@ -33,12 +30,7 @@ import org.springframework.core.env.Environment; import org.springframework.core.io.ResourceLoader; import org.springframework.core.type.filter.AbstractTypeHierarchyTraversingFilter; -import org.springframework.core.type.filter.AnnotationTypeFilter; -import org.springframework.core.type.filter.AspectJTypeFilter; -import org.springframework.core.type.filter.AssignableTypeFilter; -import org.springframework.core.type.filter.RegexPatternTypeFilter; import org.springframework.core.type.filter.TypeFilter; -import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.util.StringUtils; @@ -73,7 +65,7 @@ public ComponentScanAnnotationParser(Environment environment, ResourceLoader res } - public Set parse(AnnotationAttributes componentScan, final String declaringClass) { + public Set parse(AnnotationAttributes componentScan, String declaringClass) { ClassPathBeanDefinitionScanner scanner = new ClassPathBeanDefinitionScanner(this.registry, componentScan.getBoolean("useDefaultFilters"), this.environment, this.resourceLoader); @@ -93,13 +85,17 @@ public Set parse(AnnotationAttributes componentScan, final scanner.setResourcePattern(componentScan.getString("resourcePattern")); - for (AnnotationAttributes filter : componentScan.getAnnotationArray("includeFilters")) { - for (TypeFilter typeFilter : typeFiltersFor(filter)) { + for (AnnotationAttributes includeFilterAttributes : componentScan.getAnnotationArray("includeFilters")) { + List typeFilters = TypeFilterUtils.createTypeFiltersFor(includeFilterAttributes, this.environment, + this.resourceLoader, this.registry); + for (TypeFilter typeFilter : typeFilters) { scanner.addIncludeFilter(typeFilter); } } - for (AnnotationAttributes filter : componentScan.getAnnotationArray("excludeFilters")) { - for (TypeFilter typeFilter : typeFiltersFor(filter)) { + for (AnnotationAttributes excludeFilterAttributes : componentScan.getAnnotationArray("excludeFilters")) { + List typeFilters = TypeFilterUtils.createTypeFiltersFor(excludeFilterAttributes, this.environment, + this.resourceLoader, this.registry); + for (TypeFilter typeFilter : typeFilters) { scanner.addExcludeFilter(typeFilter); } } @@ -132,49 +128,4 @@ protected boolean matchClassName(String className) { return scanner.doScan(StringUtils.toStringArray(basePackages)); } - private List typeFiltersFor(AnnotationAttributes filterAttributes) { - List typeFilters = new ArrayList<>(); - FilterType filterType = filterAttributes.getEnum("type"); - - for (Class filterClass : filterAttributes.getClassArray("classes")) { - switch (filterType) { - case ANNOTATION: - Assert.isAssignable(Annotation.class, filterClass, - "@ComponentScan ANNOTATION type filter requires an annotation type"); - @SuppressWarnings("unchecked") - Class annotationType = (Class) filterClass; - typeFilters.add(new AnnotationTypeFilter(annotationType)); - break; - case ASSIGNABLE_TYPE: - typeFilters.add(new AssignableTypeFilter(filterClass)); - break; - case CUSTOM: - Assert.isAssignable(TypeFilter.class, filterClass, - "@ComponentScan CUSTOM type filter requires a TypeFilter implementation"); - - TypeFilter filter = ParserStrategyUtils.instantiateClass(filterClass, TypeFilter.class, - this.environment, this.resourceLoader, this.registry); - typeFilters.add(filter); - break; - default: - throw new IllegalArgumentException("Filter type not supported with Class value: " + filterType); - } - } - - for (String expression : filterAttributes.getStringArray("pattern")) { - switch (filterType) { - case ASPECTJ: - typeFilters.add(new AspectJTypeFilter(expression, this.resourceLoader.getClassLoader())); - break; - case REGEX: - typeFilters.add(new RegexPatternTypeFilter(Pattern.compile(expression))); - break; - default: - throw new IllegalArgumentException("Filter type not supported with String pattern: " + filterType); - } - } - - return typeFilters; - } - } diff --git a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassEnhancer.java b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassEnhancer.java index 17079522c7f6..1b215e524cc9 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassEnhancer.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassEnhancer.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2021 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. @@ -453,8 +453,8 @@ private boolean isCurrentlyInvokedFactoryMethod(Method method) { * instance directly. If a FactoryBean instance is fetched through the container via &-dereferencing, * it will not be proxied. This too is aligned with the way XML configuration works. */ - private Object enhanceFactoryBean(final Object factoryBean, Class exposedType, - final ConfigurableBeanFactory beanFactory, final String beanName) { + private Object enhanceFactoryBean(Object factoryBean, Class exposedType, + ConfigurableBeanFactory beanFactory, String beanName) { try { Class clazz = factoryBean.getClass(); @@ -489,8 +489,8 @@ private Object enhanceFactoryBean(final Object factoryBean, Class exposedType return createCglibProxyForFactoryBean(factoryBean, beanFactory, beanName); } - private Object createInterfaceProxyForFactoryBean(final Object factoryBean, Class interfaceType, - final ConfigurableBeanFactory beanFactory, final String beanName) { + private Object createInterfaceProxyForFactoryBean(Object factoryBean, Class interfaceType, + ConfigurableBeanFactory beanFactory, String beanName) { return Proxy.newProxyInstance( factoryBean.getClass().getClassLoader(), new Class[] {interfaceType}, @@ -502,8 +502,8 @@ private Object createInterfaceProxyForFactoryBean(final Object factoryBean, Clas }); } - private Object createCglibProxyForFactoryBean(final Object factoryBean, - final ConfigurableBeanFactory beanFactory, final String beanName) { + private Object createCglibProxyForFactoryBean(Object factoryBean, + ConfigurableBeanFactory beanFactory, String beanName) { Enhancer enhancer = new Enhancer(); enhancer.setSuperclass(factoryBean.getClass()); diff --git a/spring-context/src/main/java/org/springframework/context/annotation/Lazy.java b/spring-context/src/main/java/org/springframework/context/annotation/Lazy.java index 9d04a9df26ee..8369957f5917 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/Lazy.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/Lazy.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2021 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. @@ -47,6 +47,11 @@ * or {@link javax.inject.Inject}: In that context, it leads to the creation of a * lazy-resolution proxy for all affected dependencies, as an alternative to using * {@link org.springframework.beans.factory.ObjectFactory} or {@link javax.inject.Provider}. + * Please note that such a lazy-resolution proxy will always be injected; if the target + * dependency does not exist, you will only be able to find out through an exception on + * invocation. As a consequence, such an injection point results in unintuitive behavior + * for optional dependencies. For a programmatic equivalent, allowing for lazy references + * with more sophistication, consider {@link org.springframework.beans.factory.ObjectProvider}. * * @author Chris Beams * @author Juergen Hoeller diff --git a/spring-context/src/main/java/org/springframework/context/annotation/TypeFilterUtils.java b/spring-context/src/main/java/org/springframework/context/annotation/TypeFilterUtils.java new file mode 100644 index 000000000000..340c232f60f1 --- /dev/null +++ b/spring-context/src/main/java/org/springframework/context/annotation/TypeFilterUtils.java @@ -0,0 +1,119 @@ +/* + * Copyright 2002-2021 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.context.annotation; + +import java.lang.annotation.Annotation; +import java.util.ArrayList; +import java.util.List; +import java.util.regex.Pattern; + +import org.springframework.beans.factory.support.BeanDefinitionRegistry; +import org.springframework.core.annotation.AnnotationAttributes; +import org.springframework.core.env.Environment; +import org.springframework.core.io.ResourceLoader; +import org.springframework.core.type.filter.AnnotationTypeFilter; +import org.springframework.core.type.filter.AspectJTypeFilter; +import org.springframework.core.type.filter.AssignableTypeFilter; +import org.springframework.core.type.filter.RegexPatternTypeFilter; +import org.springframework.core.type.filter.TypeFilter; +import org.springframework.util.Assert; + +/** + * Collection of utilities for working with {@link ComponentScan @ComponentScan} + * {@linkplain ComponentScan.Filter type filters}. + * + * @author Chris Beams + * @author Juergen Hoeller + * @author Sam Brannen + * @since 5.3.13 + * @see ComponentScan.Filter + * @see org.springframework.core.type.filter.TypeFilter + */ +public abstract class TypeFilterUtils { + + /** + * Create {@linkplain TypeFilter type filters} from the supplied + * {@link AnnotationAttributes}, such as those sourced from + * {@link ComponentScan#includeFilters()} or {@link ComponentScan#excludeFilters()}. + *

Each {@link TypeFilter} will be instantiated using an appropriate + * constructor, with {@code BeanClassLoaderAware}, {@code BeanFactoryAware}, + * {@code EnvironmentAware}, and {@code ResourceLoaderAware} contracts + * invoked if they are implemented by the type filter. + * @param filterAttributes {@code AnnotationAttributes} for a + * {@link ComponentScan.Filter @Filter} declaration + * @param environment the {@code Environment} to make available to filters + * @param resourceLoader the {@code ResourceLoader} to make available to filters + * @param registry the {@code BeanDefinitionRegistry} to make available to filters + * as a {@link org.springframework.beans.factory.BeanFactory} if applicable + * @return a list of instantiated and configured type filters + * @see TypeFilter + * @see AnnotationTypeFilter + * @see AssignableTypeFilter + * @see AspectJTypeFilter + * @see RegexPatternTypeFilter + * @see org.springframework.beans.factory.BeanClassLoaderAware + * @see org.springframework.beans.factory.BeanFactoryAware + * @see org.springframework.context.EnvironmentAware + * @see org.springframework.context.ResourceLoaderAware + */ + public static List createTypeFiltersFor(AnnotationAttributes filterAttributes, Environment environment, + ResourceLoader resourceLoader, BeanDefinitionRegistry registry) { + + List typeFilters = new ArrayList<>(); + FilterType filterType = filterAttributes.getEnum("type"); + + for (Class filterClass : filterAttributes.getClassArray("classes")) { + switch (filterType) { + case ANNOTATION: + Assert.isAssignable(Annotation.class, filterClass, + "@ComponentScan ANNOTATION type filter requires an annotation type"); + @SuppressWarnings("unchecked") + Class annotationType = (Class) filterClass; + typeFilters.add(new AnnotationTypeFilter(annotationType)); + break; + case ASSIGNABLE_TYPE: + typeFilters.add(new AssignableTypeFilter(filterClass)); + break; + case CUSTOM: + Assert.isAssignable(TypeFilter.class, filterClass, + "@ComponentScan CUSTOM type filter requires a TypeFilter implementation"); + TypeFilter filter = ParserStrategyUtils.instantiateClass(filterClass, TypeFilter.class, + environment, resourceLoader, registry); + typeFilters.add(filter); + break; + default: + throw new IllegalArgumentException("Filter type not supported with Class value: " + filterType); + } + } + + for (String expression : filterAttributes.getStringArray("pattern")) { + switch (filterType) { + case ASPECTJ: + typeFilters.add(new AspectJTypeFilter(expression, resourceLoader.getClassLoader())); + break; + case REGEX: + typeFilters.add(new RegexPatternTypeFilter(Pattern.compile(expression))); + break; + default: + throw new IllegalArgumentException("Filter type not supported with String pattern: " + filterType); + } + } + + return typeFilters; + } + +} diff --git a/spring-context/src/main/java/org/springframework/context/expression/CachedExpressionEvaluator.java b/spring-context/src/main/java/org/springframework/context/expression/CachedExpressionEvaluator.java index 7b868fc9871e..c2d197a122e0 100644 --- a/spring-context/src/main/java/org/springframework/context/expression/CachedExpressionEvaluator.java +++ b/spring-context/src/main/java/org/springframework/context/expression/CachedExpressionEvaluator.java @@ -75,7 +75,7 @@ protected ParameterNameDiscoverer getParameterNameDiscoverer() { /** * Return the {@link Expression} for the specified SpEL value - *

Parse the expression if it hasn't been already. + *

{@link #parseExpression(String) Parse the expression} if it hasn't been already. * @param cache the cache to use * @param elementKey the element on which the expression is defined * @param expression the expression to parse @@ -86,12 +86,21 @@ protected Expression getExpression(Map cache, ExpressionKey expressionKey = createKey(elementKey, expression); Expression expr = cache.get(expressionKey); if (expr == null) { - expr = getParser().parseExpression(expression); + expr = parseExpression(expression); cache.put(expressionKey, expr); } return expr; } + /** + * Parse the specified {@code expression}. + * @param expression the expression to parse + * @since 5.3.13 + */ + protected Expression parseExpression(String expression) { + return getParser().parseExpression(expression); + } + private ExpressionKey createKey(AnnotatedElementKey elementKey, String expression) { return new ExpressionKey(elementKey, expression); } diff --git a/spring-context/src/main/java/org/springframework/context/index/CandidateComponentsIndexLoader.java b/spring-context/src/main/java/org/springframework/context/index/CandidateComponentsIndexLoader.java index af068373d8c9..ee95954b10ee 100644 --- a/spring-context/src/main/java/org/springframework/context/index/CandidateComponentsIndexLoader.java +++ b/spring-context/src/main/java/org/springframework/context/index/CandidateComponentsIndexLoader.java @@ -107,7 +107,7 @@ private static CandidateComponentsIndex doLoadIndex(ClassLoader classLoader) { result.add(properties); } if (logger.isDebugEnabled()) { - logger.debug("Loaded " + result.size() + "] index(es)"); + logger.debug("Loaded " + result.size() + " index(es)"); } int totalCount = result.stream().mapToInt(Properties::size).sum(); return (totalCount > 0 ? new CandidateComponentsIndex(result) : null); diff --git a/spring-context/src/test/java/org/springframework/context/expression/CachedExpressionEvaluatorTests.java b/spring-context/src/test/java/org/springframework/context/expression/CachedExpressionEvaluatorTests.java index f81a7945d738..1594137c0411 100644 --- a/spring-context/src/test/java/org/springframework/context/expression/CachedExpressionEvaluatorTests.java +++ b/spring-context/src/test/java/org/springframework/context/expression/CachedExpressionEvaluatorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2021 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. diff --git a/spring-context/src/test/java/org/springframework/context/expression/MapAccessorTests.java b/spring-context/src/test/java/org/springframework/context/expression/MapAccessorTests.java index df73adaa36d1..8fd4d2fb9a99 100644 --- a/spring-context/src/test/java/org/springframework/context/expression/MapAccessorTests.java +++ b/spring-context/src/test/java/org/springframework/context/expression/MapAccessorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2021 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. @@ -67,6 +67,17 @@ public void mapAccessorCompilable() { assertThat(ex.getValue(sec,mapGetter)).isEqualTo("bar"); assertThat(SpelCompiler.compile(ex)).isTrue(); assertThat(ex.getValue(sec,mapGetter)).isEqualTo("bar"); + + // basic isWritable + ex = sep.parseExpression("foo"); + assertThat(ex.isWritable(sec,testMap)).isTrue(); + + // basic write + ex = sep.parseExpression("foo2"); + ex.setValue(sec, testMap, "bar2"); + assertThat(ex.getValue(sec,testMap)).isEqualTo("bar2"); + assertThat(SpelCompiler.compile(ex)).isTrue(); + assertThat(ex.getValue(sec,testMap)).isEqualTo("bar2"); } public static class MapGetter { diff --git a/spring-context/src/test/java/org/springframework/format/support/FormattingConversionServiceTests.java b/spring-context/src/test/java/org/springframework/format/support/FormattingConversionServiceTests.java index eeb6895eddea..8aff305f22fa 100644 --- a/spring-context/src/test/java/org/springframework/format/support/FormattingConversionServiceTests.java +++ b/spring-context/src/test/java/org/springframework/format/support/FormattingConversionServiceTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2021 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. @@ -321,28 +321,6 @@ public Date convert(MyDate source) { TypeDescriptor.valueOf(String.class)); } - @Test - public void registerDefaultValueViaFormatter() { - registerDefaultValue(Date.class, new Date()); - } - - private void registerDefaultValue(Class clazz, final T defaultValue) { - formattingService.addFormatterForFieldType(clazz, new Formatter() { - @Override - public T parse(String text, Locale locale) { - return defaultValue; - } - @Override - public String print(T t, Locale locale) { - return defaultValue.toString(); - } - @Override - public String toString() { - return defaultValue.toString(); - } - }); - } - @Test public void introspectedFormatter() { formattingService.addFormatter(new NumberStyleFormatter("#,#00.0#")); diff --git a/spring-core/src/main/java/org/springframework/core/convert/TypeDescriptor.java b/spring-core/src/main/java/org/springframework/core/convert/TypeDescriptor.java index 1bc8cb281b1f..f6e25824e004 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/TypeDescriptor.java +++ b/spring-core/src/main/java/org/springframework/core/convert/TypeDescriptor.java @@ -37,7 +37,7 @@ /** * Contextual descriptor about a type to convert from or to. - * Capable of representing arrays and generic collection types. + *

Capable of representing arrays and generic collection types. * * @author Keith Donald * @author Andy Clement @@ -345,9 +345,9 @@ public TypeDescriptor getElementTypeDescriptor() { * from the provided collection or array element. *

Narrows the {@link #getElementTypeDescriptor() elementType} property to the class * of the provided collection or array element. For example, if this describes a - * {@code java.util.List<java.lang.Number<} and the element argument is an + * {@code java.util.List} and the element argument is a * {@code java.lang.Integer}, the returned TypeDescriptor will be {@code java.lang.Integer}. - * If this describes a {@code java.util.List<?>} and the element argument is an + * If this describes a {@code java.util.List} and the element argument is a * {@code java.lang.Integer}, the returned TypeDescriptor will be {@code java.lang.Integer} * as well. *

Annotation and nested type context will be preserved in the narrowed @@ -388,9 +388,9 @@ public TypeDescriptor getMapKeyTypeDescriptor() { * from the provided map key. *

Narrows the {@link #getMapKeyTypeDescriptor() mapKeyType} property * to the class of the provided map key. For example, if this describes a - * {@code java.util.Map<java.lang.Number, java.lang.String<} and the key + * {@code java.util.Map} and the key * argument is a {@code java.lang.Integer}, the returned TypeDescriptor will be - * {@code java.lang.Integer}. If this describes a {@code java.util.Map<?, ?>} + * {@code java.lang.Integer}. If this describes a {@code java.util.Map} * and the key argument is a {@code java.lang.Integer}, the returned * TypeDescriptor will be {@code java.lang.Integer} as well. *

Annotation and nested type context will be preserved in the narrowed @@ -425,9 +425,9 @@ public TypeDescriptor getMapValueTypeDescriptor() { * from the provided map value. *

Narrows the {@link #getMapValueTypeDescriptor() mapValueType} property * to the class of the provided map value. For example, if this describes a - * {@code java.util.Map<java.lang.String, java.lang.Number<} and the value + * {@code java.util.Map} and the value * argument is a {@code java.lang.Integer}, the returned TypeDescriptor will be - * {@code java.lang.Integer}. If this describes a {@code java.util.Map<?, ?>} + * {@code java.lang.Integer}. If this describes a {@code java.util.Map} * and the value argument is a {@code java.lang.Integer}, the returned * TypeDescriptor will be {@code java.lang.Integer} as well. *

Annotation and nested type context will be preserved in the narrowed diff --git a/spring-core/src/main/java/org/springframework/core/log/LogFormatUtils.java b/spring-core/src/main/java/org/springframework/core/log/LogFormatUtils.java index e3ae033e9f22..51ffd98cf0ab 100644 --- a/spring-core/src/main/java/org/springframework/core/log/LogFormatUtils.java +++ b/spring-core/src/main/java/org/springframework/core/log/LogFormatUtils.java @@ -36,8 +36,9 @@ public abstract class LogFormatUtils { /** - * Variant of {@link #formatValue(Object, int, boolean)} and a convenience - * method that truncates at 100 characters when {@code limitLength} is set. + * Convenience variant of {@link #formatValue(Object, int, boolean)} that + * limits the length of a log message to 100 characters and also replaces + * newline characters if {@code limitLength} is set to "true". * @param value the value to format * @param limitLength whether to truncate the value at a length of 100 * @return the formatted value diff --git a/spring-expression/src/main/java/org/springframework/expression/TypeConverter.java b/spring-expression/src/main/java/org/springframework/expression/TypeConverter.java index f2f67c0965c8..6974d6b8747c 100644 --- a/spring-expression/src/main/java/org/springframework/expression/TypeConverter.java +++ b/spring-expression/src/main/java/org/springframework/expression/TypeConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2021 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. @@ -44,7 +44,7 @@ public interface TypeConverter { * Convert (or coerce) a value from one type to another, for example from a * {@code boolean} to a {@code String}. *

The {@link TypeDescriptor} parameters enable support for typed collections: - * A caller may prefer a {@code List<Integer>}, for example, rather than + * A caller may prefer a {@code List}, for example, rather than * simply any {@code List}. * @param value the value to be converted * @param sourceType a type descriptor that supplies extra information about the diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java index d8af41170d67..821c0b246ae6 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2021 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. @@ -38,6 +38,7 @@ * * @author Andy Clement * @author Juergen Hoeller + * @author Sam Brannen * @since 3.0 */ public abstract class ReflectionHelper { @@ -281,25 +282,32 @@ static boolean convertArguments(TypeConverter converter, Object[] arguments, Exe arguments[i] = converter.convertValue(argument, TypeDescriptor.forObject(argument), targetType); conversionOccurred |= (argument != arguments[i]); } + MethodParameter methodParam = MethodParameter.forExecutable(executable, varargsPosition); + + // If the target is varargs and there is just one more argument, then convert it here. if (varargsPosition == arguments.length - 1) { - // If the target is varargs and there is just one more argument - // then convert it here - TypeDescriptor targetType = new TypeDescriptor(methodParam); Object argument = arguments[varargsPosition]; + TypeDescriptor targetType = new TypeDescriptor(methodParam); TypeDescriptor sourceType = TypeDescriptor.forObject(argument); - arguments[varargsPosition] = converter.convertValue(argument, sourceType, targetType); - // Three outcomes of that previous line: - // 1) the input argument was already compatible (ie. array of valid type) and nothing was done - // 2) the input argument was correct type but not in an array so it was made into an array - // 3) the input argument was the wrong type and got converted and put into an array + // If the argument type is equal to the varargs element type, there is no need + // to convert it or wrap it in an array. For example, using StringToArrayConverter + // to convert a String containing a comma would result in the String being split + // and repackaged in an array when it should be used as-is. + if (!sourceType.equals(targetType.getElementTypeDescriptor())) { + arguments[varargsPosition] = converter.convertValue(argument, sourceType, targetType); + } + // Three outcomes of the above if-block: + // 1) the input argument was correct type but not wrapped in an array, and nothing was done. + // 2) the input argument was already compatible (i.e., array of valid type), and nothing was done. + // 3) the input argument was the wrong type and got converted and wrapped in an array. if (argument != arguments[varargsPosition] && !isFirstEntryInArray(argument, arguments[varargsPosition])) { conversionOccurred = true; // case 3 } } + // Otherwise, convert remaining arguments to the varargs element type. else { - // Convert remaining arguments to the varargs element type TypeDescriptor targetType = new TypeDescriptor(methodParam).getElementTypeDescriptor(); Assert.state(targetType != null, "No element type"); for (int i = varargsPosition; i < arguments.length; i++) { @@ -332,8 +340,8 @@ private static boolean isFirstEntryInArray(Object value, @Nullable Object possib } /** - * Package up the arguments so that they correctly match what is expected in parameterTypes. - * For example, if parameterTypes is {@code (int, String[])} because the second parameter + * Package up the arguments so that they correctly match what is expected in requiredParameterTypes. + *

For example, if requiredParameterTypes is {@code (int, String[])} because the second parameter * was declared {@code String...}, then if arguments is {@code [1,"a","b"]} then it must be * repackaged as {@code [1,new String[]{"a","b"}]} in order to match the expected types. * @param requiredParameterTypes the types of the parameters for the invocation @@ -350,23 +358,24 @@ public static Object[] setupArgumentsForVarargsInvocation(Class[] requiredPar requiredParameterTypes[parameterCount - 1] != (args[argumentCount - 1] != null ? args[argumentCount - 1].getClass() : null)) { - int arraySize = 0; // zero size array if nothing to pass as the varargs parameter - if (argumentCount >= parameterCount) { - arraySize = argumentCount - (parameterCount - 1); - } - - // Create an array for the varargs arguments + // Create an array for the leading arguments plus the varargs array argument. Object[] newArgs = new Object[parameterCount]; + // Copy all leading arguments to the new array, omitting the varargs array argument. System.arraycopy(args, 0, newArgs, 0, newArgs.length - 1); // Now sort out the final argument, which is the varargs one. Before entering this method, // the arguments should have been converted to the box form of the required type. + int varargsArraySize = 0; // zero size array if nothing to pass as the varargs parameter + if (argumentCount >= parameterCount) { + varargsArraySize = argumentCount - (parameterCount - 1); + } Class componentType = requiredParameterTypes[parameterCount - 1].getComponentType(); - Object repackagedArgs = Array.newInstance(componentType, arraySize); - for (int i = 0; i < arraySize; i++) { - Array.set(repackagedArgs, i, args[parameterCount - 1 + i]); + Object varargsArray = Array.newInstance(componentType, varargsArraySize); + for (int i = 0; i < varargsArraySize; i++) { + Array.set(varargsArray, i, args[parameterCount - 1 + i]); } - newArgs[newArgs.length - 1] = repackagedArgs; + // Finally, add the varargs array to the new arguments array. + newArgs[newArgs.length - 1] = varargsArray; return newArgs; } return args; diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java index 0a025acf723f..b2cde1f10ff3 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2021 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. @@ -46,6 +46,7 @@ * * @author Andy Clement * @author Phillip Webb + * @author Sam Brannen */ public class MethodInvocationTests extends AbstractExpressionTests { @@ -233,26 +234,54 @@ public void testAddingMethodResolvers() { @Test public void testVarargsInvocation01() { - // Calling 'public int aVarargsMethod(String... strings)' - //evaluate("aVarargsMethod('a','b','c')", 3, Integer.class); - //evaluate("aVarargsMethod('a')", 1, Integer.class); + // Calling 'public int aVarargsMethod(String... strings)' - returns number of arguments + evaluate("aVarargsMethod('a','b','c')", 3, Integer.class); + evaluate("aVarargsMethod('a')", 1, Integer.class); evaluate("aVarargsMethod()", 0, Integer.class); evaluate("aVarargsMethod(1,2,3)", 3, Integer.class); // all need converting to strings evaluate("aVarargsMethod(1)", 1, Integer.class); // needs string conversion evaluate("aVarargsMethod(1,'a',3.0d)", 3, Integer.class); // first and last need conversion - // evaluate("aVarargsMethod(new String[]{'a','b','c'})", 3, Integer.class); + evaluate("aVarargsMethod(new String[]{'a','b','c'})", 3, Integer.class); } @Test public void testVarargsInvocation02() { - // Calling 'public int aVarargsMethod2(int i, String... strings)' - returns int+length_of_strings + // Calling 'public int aVarargsMethod2(int i, String... strings)' - returns int + length_of_strings evaluate("aVarargsMethod2(5,'a','b','c')", 8, Integer.class); evaluate("aVarargsMethod2(2,'a')", 3, Integer.class); evaluate("aVarargsMethod2(4)", 4, Integer.class); evaluate("aVarargsMethod2(8,2,3)", 10, Integer.class); evaluate("aVarargsMethod2(9)", 9, Integer.class); evaluate("aVarargsMethod2(2,'a',3.0d)", 4, Integer.class); - // evaluate("aVarargsMethod2(8,new String[]{'a','b','c'})", 11, Integer.class); + evaluate("aVarargsMethod2(8,new String[]{'a','b','c'})", 11, Integer.class); + } + + @Test + public void testVarargsInvocation03() { + // Calling 'public int aVarargsMethod3(String str1, String... strings)' - returns all strings concatenated with "-" + + // No conversion necessary + evaluate("aVarargsMethod3('x')", "x", String.class); + evaluate("aVarargsMethod3('x', 'a')", "x-a", String.class); + evaluate("aVarargsMethod3('x', 'a', 'b', 'c')", "x-a-b-c", String.class); + + // Conversion necessary + evaluate("aVarargsMethod3(9)", "9", String.class); + evaluate("aVarargsMethod3(8,2,3)", "8-2-3", String.class); + evaluate("aVarargsMethod3('2','a',3.0d)", "2-a-3.0", String.class); + evaluate("aVarargsMethod3('8',new String[]{'a','b','c'})", "8-a-b-c", String.class); + + // Individual string contains a comma with multiple varargs arguments + evaluate("aVarargsMethod3('foo', ',', 'baz')", "foo-,-baz", String.class); + evaluate("aVarargsMethod3('foo', 'bar', ',baz')", "foo-bar-,baz", String.class); + evaluate("aVarargsMethod3('foo', 'bar,', 'baz')", "foo-bar,-baz", String.class); + + // Individual string contains a comma with single varargs argument. + // Reproduces https://github.com/spring-projects/spring-framework/issues/27582 + evaluate("aVarargsMethod3('foo', ',')", "foo-,", String.class); + evaluate("aVarargsMethod3('foo', ',bar')", "foo-,bar", String.class); + evaluate("aVarargsMethod3('foo', 'bar,')", "foo-bar,", String.class); + evaluate("aVarargsMethod3('foo', 'bar,baz')", "foo-bar,baz", String.class); } @Test diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/VariableAndFunctionTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/VariableAndFunctionTests.java index 0333e20ff961..3271e45ea6d1 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/VariableAndFunctionTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/VariableAndFunctionTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2021 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. @@ -28,6 +28,7 @@ * Tests the evaluation of expressions that access variables and functions (lambda/java). * * @author Andy Clement + * @author Sam Brannen */ public class VariableAndFunctionTests extends AbstractExpressionTests { @@ -58,12 +59,17 @@ public void testFunctionAccess02() { @Test public void testCallVarargsFunction() { + evaluate("#varargsFunctionReverseStringsAndMerge('a,b')", "a,b", String.class); + evaluate("#varargsFunctionReverseStringsAndMerge('a', 'b,c', 'd')", "db,ca", String.class); evaluate("#varargsFunctionReverseStringsAndMerge('a','b','c')", "cba", String.class); evaluate("#varargsFunctionReverseStringsAndMerge('a')", "a", String.class); evaluate("#varargsFunctionReverseStringsAndMerge()", "", String.class); evaluate("#varargsFunctionReverseStringsAndMerge('b',25)", "25b", String.class); evaluate("#varargsFunctionReverseStringsAndMerge(25)", "25", String.class); + + evaluate("#varargsFunctionReverseStringsAndMerge2(1, 'a,b')", "1a,b", String.class); evaluate("#varargsFunctionReverseStringsAndMerge2(1,'a','b','c')", "1cba", String.class); + evaluate("#varargsFunctionReverseStringsAndMerge2(1, 'a', 'b,c', 'd')", "1db,ca", String.class); evaluate("#varargsFunctionReverseStringsAndMerge2(2,'a')", "2a", String.class); evaluate("#varargsFunctionReverseStringsAndMerge2(3)", "3", String.class); evaluate("#varargsFunctionReverseStringsAndMerge2(4,'b',25)", "425b", String.class); diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java b/spring-expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java index bdf6d79c1ddc..34960d982fe3 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2021 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. @@ -28,6 +28,7 @@ ///CLOVER:OFF @SuppressWarnings("unused") public class Inventor { + private String name; public String _name; public String _name_; @@ -202,8 +203,14 @@ public int aVarargsMethod2(int i, String... strings) { return strings.length + i; } - public Inventor(String... strings) { + public String aVarargsMethod3(String str1, String... strings) { + if (ObjectUtils.isEmpty(strings)) { + return str1; + } + return str1 + "-" + String.join("-", strings); + } + public Inventor(String... strings) { } public boolean getSomeProperty() { diff --git a/spring-orm/src/main/java/org/springframework/orm/jpa/ExtendedEntityManagerCreator.java b/spring-orm/src/main/java/org/springframework/orm/jpa/ExtendedEntityManagerCreator.java index 8e9173a94bfd..55431d805f3e 100644 --- a/spring-orm/src/main/java/org/springframework/orm/jpa/ExtendedEntityManagerCreator.java +++ b/spring-orm/src/main/java/org/springframework/orm/jpa/ExtendedEntityManagerCreator.java @@ -230,10 +230,10 @@ private static EntityManager createProxy( if (emIfc != null) { interfaces = cachedEntityManagerInterfaces.computeIfAbsent(emIfc, key -> { - Set> ifcs = new LinkedHashSet<>(4); - ifcs.add(key); - ifcs.add(EntityManagerProxy.class); - return ClassUtils.toClassArray(ifcs); + if (EntityManagerProxy.class.equals(key)) { + return new Class[] {key}; + } + return new Class[] {key, EntityManagerProxy.class}; }); } else { diff --git a/spring-orm/src/main/java/org/springframework/orm/jpa/support/PersistenceAnnotationBeanPostProcessor.java b/spring-orm/src/main/java/org/springframework/orm/jpa/support/PersistenceAnnotationBeanPostProcessor.java index d9ff7afc1414..61582bee1180 100644 --- a/spring-orm/src/main/java/org/springframework/orm/jpa/support/PersistenceAnnotationBeanPostProcessor.java +++ b/spring-orm/src/main/java/org/springframework/orm/jpa/support/PersistenceAnnotationBeanPostProcessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2021 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. @@ -373,7 +373,7 @@ public boolean requiresDestruction(Object bean) { } - private InjectionMetadata findPersistenceMetadata(String beanName, final Class clazz, @Nullable PropertyValues pvs) { + private InjectionMetadata findPersistenceMetadata(String beanName, Class clazz, @Nullable PropertyValues pvs) { // Fall back to class name as cache key, for backwards compatibility with custom callers. String cacheKey = (StringUtils.hasLength(beanName) ? beanName : clazz.getName()); // Quick check on the concurrent map first, with minimal locking. @@ -393,7 +393,7 @@ private InjectionMetadata findPersistenceMetadata(String beanName, final Class clazz) { + private InjectionMetadata buildPersistenceMetadata(Class clazz) { if (!AnnotationUtils.isCandidateClass(clazz, Arrays.asList(PersistenceContext.class, PersistenceUnit.class))) { return InjectionMetadata.EMPTY; } diff --git a/spring-test/src/main/java/org/springframework/test/web/reactive/server/CookieAssertions.java b/spring-test/src/main/java/org/springframework/test/web/reactive/server/CookieAssertions.java index 26b28b486a41..f9804915f72b 100644 --- a/spring-test/src/main/java/org/springframework/test/web/reactive/server/CookieAssertions.java +++ b/spring-test/src/main/java/org/springframework/test/web/reactive/server/CookieAssertions.java @@ -17,6 +17,7 @@ package org.springframework.test.web.reactive.server; import java.time.Duration; +import java.util.Objects; import java.util.function.Consumer; import org.hamcrest.Matcher; @@ -210,10 +211,10 @@ public WebTestClient.ResponseSpec sameSite(String name, String expected) { private ResponseCookie getCookie(String name) { ResponseCookie cookie = this.exchangeResult.getResponseCookies().getFirst(name); if (cookie == null) { - String message = "No cookie with name '" + name + "'"; - this.exchangeResult.assertWithDiagnostics(() -> AssertionErrors.fail(message)); + this.exchangeResult.assertWithDiagnostics(() -> + AssertionErrors.fail("No cookie with name '" + name + "'")); } - return cookie; + return Objects.requireNonNull(cookie); } private String getMessage(String cookie) { diff --git a/spring-test/src/main/java/org/springframework/test/web/reactive/server/HeaderAssertions.java b/spring-test/src/main/java/org/springframework/test/web/reactive/server/HeaderAssertions.java index dfd8ce89c7e4..c3750d27c3c4 100644 --- a/spring-test/src/main/java/org/springframework/test/web/reactive/server/HeaderAssertions.java +++ b/spring-test/src/main/java/org/springframework/test/web/reactive/server/HeaderAssertions.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2021 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. @@ -19,6 +19,7 @@ import java.net.URI; import java.util.Arrays; import java.util.List; +import java.util.Objects; import java.util.function.Consumer; import org.hamcrest.Matcher; @@ -73,7 +74,7 @@ public WebTestClient.ResponseSpec valueEquals(String headerName, long value) { String actual = getHeaders().getFirst(headerName); this.exchangeResult.assertWithDiagnostics(() -> assertTrue("Response does not contain header '" + headerName + "'", actual != null)); - return assertHeader(headerName, value, Long.parseLong(actual)); + return assertHeader(headerName, value, Long.parseLong(Objects.requireNonNull(actual))); } /** @@ -115,7 +116,7 @@ public WebTestClient.ResponseSpec valueMatches(String name, String pattern) { /** * Match all values of the response header with the given regex * patterns which are applied to the values of the header in the - * same order. Note that the number of pattenrs must match the + * same order. Note that the number of patterns must match the * number of actual values. * @param name the header name * @param patterns one or more regex patterns, one per expected value @@ -203,7 +204,7 @@ private List getRequiredValues(String name) { this.exchangeResult.assertWithDiagnostics(() -> AssertionErrors.fail(getMessage(name) + " not found")); } - return values; + return Objects.requireNonNull(values); } /** diff --git a/spring-tx/src/main/java/org/springframework/dao/annotation/PersistenceExceptionTranslationPostProcessor.java b/spring-tx/src/main/java/org/springframework/dao/annotation/PersistenceExceptionTranslationPostProcessor.java index c975e9ec6b05..a612fe808d09 100644 --- a/spring-tx/src/main/java/org/springframework/dao/annotation/PersistenceExceptionTranslationPostProcessor.java +++ b/spring-tx/src/main/java/org/springframework/dao/annotation/PersistenceExceptionTranslationPostProcessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2021 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. @@ -38,7 +38,6 @@ * PersistenceExceptionTranslator} interface, which are subsequently asked to translate * candidate exceptions. * - *

All of Spring's applicable resource factories (e.g. * {@link org.springframework.orm.jpa.LocalContainerEntityManagerFactoryBean}) * implement the {@code PersistenceExceptionTranslator} interface out of the box. @@ -47,6 +46,11 @@ * with the {@code @Repository} annotation, along with defining this post-processor * as a bean in the application context. * + *

As of 5.3, {@code PersistenceExceptionTranslator} beans will be sorted according + * to Spring's dependency ordering rules: see {@link org.springframework.core.Ordered} + * and {@link org.springframework.core.annotation.Order}. Note that such beans will + * get retrieved from any scope, not just singleton scope, as of this 5.3 revision. + * * @author Rod Johnson * @author Juergen Hoeller * @since 2.0 diff --git a/spring-web/src/main/java/org/springframework/http/HttpEntity.java b/spring-web/src/main/java/org/springframework/http/HttpEntity.java index 7b7fe9af6002..9b5be7c625ae 100644 --- a/spring-web/src/main/java/org/springframework/http/HttpEntity.java +++ b/spring-web/src/main/java/org/springframework/http/HttpEntity.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2021 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. @@ -23,12 +23,12 @@ /** * Represents an HTTP request or response entity, consisting of headers and body. * - *

Typically used in combination with the {@link org.springframework.web.client.RestTemplate}, + *

Often used in combination with the {@link org.springframework.web.client.RestTemplate}, * like so: * 

  * HttpHeaders headers = new HttpHeaders();
  * headers.setContentType(MediaType.TEXT_PLAIN);
- * HttpEntity<String> entity = new HttpEntity<String>(helloWorld, headers);
+ * HttpEntity<String> entity = new HttpEntity<>("Hello World", headers);
  * URI location = template.postForLocation("https://example.com", entity);
  *
* or @@ -39,11 +39,11 @@ * * Can also be used in Spring MVC, as a return value from a @Controller method: * 
- * @RequestMapping("/handle")
+ * @GetMapping("/handle")
  * public HttpEntity<String> handle() {
  *   HttpHeaders responseHeaders = new HttpHeaders();
  *   responseHeaders.set("MyResponseHeader", "MyValue");
- *   return new HttpEntity<String>("Hello World", responseHeaders);
+ *   return new HttpEntity<>("Hello World", responseHeaders);
  * }
  *
* diff --git a/spring-web/src/main/java/org/springframework/http/codec/multipart/DefaultPartHttpMessageReader.java b/spring-web/src/main/java/org/springframework/http/codec/multipart/DefaultPartHttpMessageReader.java index d4248c8442d3..6e94678cef11 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/multipart/DefaultPartHttpMessageReader.java +++ b/spring-web/src/main/java/org/springframework/http/codec/multipart/DefaultPartHttpMessageReader.java @@ -63,7 +63,7 @@ public class DefaultPartHttpMessageReader extends LoggingCodecSupport implements private int maxInMemorySize = 256 * 1024; - private int maxHeadersSize = 8 * 1024; + private int maxHeadersSize = 10 * 1024; private long maxDiskUsagePerPart = -1; diff --git a/spring-web/src/main/java/org/springframework/http/codec/multipart/DefaultParts.java b/spring-web/src/main/java/org/springframework/http/codec/multipart/DefaultParts.java index 77044db0a8dc..284c82497b96 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/multipart/DefaultParts.java +++ b/spring-web/src/main/java/org/springframework/http/codec/multipart/DefaultParts.java @@ -16,10 +16,15 @@ package org.springframework.http.codec.multipart; +import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; +import java.util.concurrent.Callable; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; +import reactor.core.scheduler.Scheduler; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DataBufferUtils; @@ -50,17 +55,40 @@ public static FormFieldPart formFieldPart(HttpHeaders headers, String value) { } /** - * Create a new {@link Part} or {@link FilePart} with the given parameters. + * Create a new {@link Part} or {@link FilePart} based on a flux of data + * buffers. Returns {@link FilePart} if the {@code Content-Disposition} of + * the given headers contains a filename, or a "normal" {@link Part} + * otherwise. + * @param headers the part headers + * @param dataBuffers the content of the part + * @return {@link Part} or {@link FilePart}, depending on {@link HttpHeaders#getContentDisposition()} + */ + public static Part part(HttpHeaders headers, Flux dataBuffers) { + Assert.notNull(headers, "Headers must not be null"); + Assert.notNull(dataBuffers, "DataBuffers must not be null"); + + return partInternal(headers, new FluxContent(dataBuffers)); + } + + /** + * Create a new {@link Part} or {@link FilePart} based on the given file. * Returns {@link FilePart} if the {@code Content-Disposition} of the given * headers contains a filename, or a "normal" {@link Part} otherwise * @param headers the part headers - * @param content the content of the part + * @param file the file + * @param scheduler the scheduler used for reading the file * @return {@link Part} or {@link FilePart}, depending on {@link HttpHeaders#getContentDisposition()} */ - public static Part part(HttpHeaders headers, Flux content) { + public static Part part(HttpHeaders headers, Path file, Scheduler scheduler) { Assert.notNull(headers, "Headers must not be null"); - Assert.notNull(content, "Content must not be null"); + Assert.notNull(file, "File must not be null"); + Assert.notNull(scheduler, "Scheduler must not be null"); + + return partInternal(headers, new FileContent(file, scheduler)); + } + + private static Part partInternal(HttpHeaders headers, Content content) { String filename = headers.getContentDisposition().getFilename(); if (filename != null) { return new DefaultFilePart(headers, content); @@ -142,16 +170,22 @@ public String toString() { */ private static class DefaultPart extends AbstractPart { - private final Flux content; + protected final Content content; + - public DefaultPart(HttpHeaders headers, Flux content) { + public DefaultPart(HttpHeaders headers, Content content) { super(headers); this.content = content; } @Override public Flux content() { - return this.content; + return this.content.content(); + } + + @Override + public Mono delete() { + return this.content.delete(); } @Override @@ -171,9 +205,9 @@ public String toString() { /** * Default implementation of {@link FilePart}. */ - private static class DefaultFilePart extends DefaultPart implements FilePart { + private static final class DefaultFilePart extends DefaultPart implements FilePart { - public DefaultFilePart(HttpHeaders headers, Flux content) { + public DefaultFilePart(HttpHeaders headers, Content content) { super(headers, content); } @@ -186,7 +220,7 @@ public String filename() { @Override public Mono transferTo(Path dest) { - return DataBufferUtils.write(content(), dest); + return this.content.transferTo(dest); } @Override @@ -195,7 +229,7 @@ public String toString() { String name = contentDisposition.getName(); String filename = contentDisposition.getFilename(); if (name != null) { - return "DefaultFilePart{" + name() + " (" + filename + ")}"; + return "DefaultFilePart{" + name + " (" + filename + ")}"; } else { return "DefaultFilePart{(" + filename + ")}"; @@ -204,4 +238,100 @@ public String toString() { } + + /** + * Part content abstraction. + */ + private interface Content { + + Flux content(); + + Mono transferTo(Path dest); + + Mono delete(); + + } + + /** + * {@code Content} implementation based on a flux of data buffers. + */ + private static final class FluxContent implements Content { + + private final Flux content; + + + public FluxContent(Flux content) { + this.content = content; + } + + + @Override + public Flux content() { + return this.content; + } + + @Override + public Mono transferTo(Path dest) { + return DataBufferUtils.write(this.content, dest); + } + + @Override + public Mono delete() { + return Mono.empty(); + } + + } + + + /** + * {@code Content} implementation based on a file. + */ + private static final class FileContent implements Content { + + private final Path file; + + private final Scheduler scheduler; + + + public FileContent(Path file, Scheduler scheduler) { + this.file = file; + this.scheduler = scheduler; + } + + + @Override + public Flux content() { + return DataBufferUtils.readByteChannel( + () -> Files.newByteChannel(this.file, StandardOpenOption.READ), + DefaultDataBufferFactory.sharedInstance, 1024) + .subscribeOn(this.scheduler); + } + + @Override + public Mono transferTo(Path dest) { + return blockingOperation(() -> Files.copy(this.file, dest, StandardCopyOption.REPLACE_EXISTING)); + } + + @Override + public Mono delete() { + return blockingOperation(() -> { + Files.delete(this.file); + return null; + }); + } + + private Mono blockingOperation(Callable callable) { + return Mono.create(sink -> { + try { + callable.call(); + sink.success(); + } + catch (Exception ex) { + sink.error(ex); + } + }) + .subscribeOn(this.scheduler); + } + } + } diff --git a/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartParser.java b/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartParser.java index d2057f53d627..d797b99f4b1b 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartParser.java +++ b/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartParser.java @@ -342,33 +342,23 @@ private final class HeadersState implements State { /** * First checks whether the multipart boundary leading to this state - * was the final boundary, or whether {@link #maxHeadersSize} is - * exceeded. Then looks for the header-body boundary - * ({@code CR LF CR LF}) in the given buffer. If found, convert - * all buffers collected so far into a {@link HttpHeaders} object + * was the final boundary. Then looks for the header-body boundary + * ({@code CR LF CR LF}) in the given buffer. If found, checks whether + * the size of all header buffers does not exceed {@link #maxHeadersSize}, + * converts all buffers collected so far into a {@link HttpHeaders} object * and changes to {@link BodyState}, passing the remainder of the - * buffer. If the boundary is not found, the buffer is collected. + * buffer. If the boundary is not found, the buffer is collected if + * its size does not exceed {@link #maxHeadersSize}. */ @Override public void onNext(DataBuffer buf) { - long prevCount = this.byteCount.get(); - long count = this.byteCount.addAndGet(buf.readableByteCount()); - if (prevCount < 2 && count >= 2) { - if (isLastBoundary(buf)) { - if (logger.isTraceEnabled()) { - logger.trace("Last boundary found in " + buf); - } - - if (changeState(this, DisposedState.INSTANCE, buf)) { - emitComplete(); - } - return; + if (isLastBoundary(buf)) { + if (logger.isTraceEnabled()) { + logger.trace("Last boundary found in " + buf); } - } - else if (count > MultipartParser.this.maxHeadersSize) { + if (changeState(this, DisposedState.INSTANCE, buf)) { - emitError(new DataBufferLimitException("Part headers exceeded the memory usage limit of " + - MultipartParser.this.maxHeadersSize + " bytes")); + emitComplete(); } return; } @@ -377,17 +367,23 @@ else if (count > MultipartParser.this.maxHeadersSize) { if (logger.isTraceEnabled()) { logger.trace("End of headers found @" + endIdx + " in " + buf); } - DataBuffer headerBuf = MultipartUtils.sliceTo(buf, endIdx); - this.buffers.add(headerBuf); - DataBuffer bodyBuf = MultipartUtils.sliceFrom(buf, endIdx); - DataBufferUtils.release(buf); - - emitHeaders(parseHeaders()); - changeState(this, new BodyState(), bodyBuf); + long count = this.byteCount.addAndGet(endIdx); + if (belowMaxHeaderSize(count)) { + DataBuffer headerBuf = MultipartUtils.sliceTo(buf, endIdx); + this.buffers.add(headerBuf); + DataBuffer bodyBuf = MultipartUtils.sliceFrom(buf, endIdx); + DataBufferUtils.release(buf); + + emitHeaders(parseHeaders()); + changeState(this, new BodyState(), bodyBuf); + } } else { - this.buffers.add(buf); - requestBuffer(); + long count = this.byteCount.addAndGet(buf.readableByteCount()); + if (belowMaxHeaderSize(count)) { + this.buffers.add(buf); + requestBuffer(); + } } } @@ -407,6 +403,21 @@ private boolean isLastBoundary(DataBuffer buf) { buf.getByte(0) == HYPHEN); } + /** + * Checks whether the given {@code count} is below or equal to {@link #maxHeadersSize} + * and emits a {@link DataBufferLimitException} if not. + */ + private boolean belowMaxHeaderSize(long count) { + if (count <= MultipartParser.this.maxHeadersSize) { + return true; + } + else { + emitError(new DataBufferLimitException("Part headers exceeded the memory usage limit of " + + MultipartParser.this.maxHeadersSize + " bytes")); + return false; + } + } + /** * Parses the list of buffers into a {@link HttpHeaders} instance. * Converts the joined buffers into a string using ISO=8859-1, and parses diff --git a/spring-web/src/main/java/org/springframework/http/codec/multipart/Part.java b/spring-web/src/main/java/org/springframework/http/codec/multipart/Part.java index c611adf22ae6..c39b36ff5faf 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/multipart/Part.java +++ b/spring-web/src/main/java/org/springframework/http/codec/multipart/Part.java @@ -17,6 +17,7 @@ package org.springframework.http.codec.multipart; import reactor.core.publisher.Flux; +import reactor.core.publisher.Mono; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.http.HttpHeaders; @@ -57,4 +58,13 @@ public interface Part { */ Flux content(); + /** + * Return a mono that, when subscribed to, deletes the underlying storage + * for this part. + * @since 5.3.13 + */ + default Mono delete() { + return Mono.empty(); + } + } diff --git a/spring-web/src/main/java/org/springframework/http/codec/multipart/PartGenerator.java b/spring-web/src/main/java/org/springframework/http/codec/multipart/PartGenerator.java index 32a923d8a7d5..88d689d90e9b 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/multipart/PartGenerator.java +++ b/spring-web/src/main/java/org/springframework/http/codec/multipart/PartGenerator.java @@ -674,21 +674,12 @@ public void body(DataBuffer dataBuffer) { @Override public void partComplete(boolean finalPart) { MultipartUtils.closeChannel(this.channel); - Flux content = partContent(); - emitPart(DefaultParts.part(this.headers, content)); + emitPart(DefaultParts.part(this.headers, this.file, PartGenerator.this.blockingOperationScheduler)); if (finalPart) { emitComplete(); } } - private Flux partContent() { - return DataBufferUtils - .readByteChannel( - () -> Files.newByteChannel(this.file, StandardOpenOption.READ), - DefaultDataBufferFactory.sharedInstance, 1024) - .subscribeOn(PartGenerator.this.blockingOperationScheduler); - } - @Override public void dispose() { if (this.closeOnDispose) { diff --git a/spring-web/src/main/java/org/springframework/http/codec/multipart/SynchronossPartHttpMessageReader.java b/spring-web/src/main/java/org/springframework/http/codec/multipart/SynchronossPartHttpMessageReader.java index 032b787d887b..a35c9d5712de 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/multipart/SynchronossPartHttpMessageReader.java +++ b/spring-web/src/main/java/org/springframework/http/codec/multipart/SynchronossPartHttpMessageReader.java @@ -16,7 +16,9 @@ package org.springframework.http.codec.multipart; +import java.io.File; import java.io.IOException; +import java.io.InputStream; import java.nio.channels.Channels; import java.nio.channels.FileChannel; import java.nio.channels.ReadableByteChannel; @@ -40,6 +42,7 @@ import org.synchronoss.cloud.nio.multipart.NioMultipartParser; import org.synchronoss.cloud.nio.multipart.NioMultipartParserListener; import org.synchronoss.cloud.nio.multipart.PartBodyStreamStorageFactory; +import org.synchronoss.cloud.nio.stream.storage.NameAwarePurgableFileInputStream; import org.synchronoss.cloud.nio.stream.storage.StreamStorage; import reactor.core.publisher.BaseSubscriber; import reactor.core.publisher.Flux; @@ -497,6 +500,38 @@ public Flux content() { protected StreamStorage getStorage() { return this.storage; } + + @Override + public Mono delete() { + return Mono.fromRunnable(() -> { + File file = getFile(); + if (file != null) { + file.delete(); + } + }); + } + + @Nullable + private File getFile() { + InputStream inputStream = null; + try { + inputStream = getStorage().getInputStream(); + if (inputStream instanceof NameAwarePurgableFileInputStream) { + NameAwarePurgableFileInputStream stream = (NameAwarePurgableFileInputStream) inputStream; + return stream.getFile(); + } + } + finally { + if (inputStream != null) { + try { + inputStream.close(); + } + catch (IOException ignore) { + } + } + } + return null; + } } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowHeadersAdapter.java b/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowHeadersAdapter.java index fa197aa0c208..8b7c6aed14dd 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowHeadersAdapter.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowHeadersAdapter.java @@ -17,6 +17,7 @@ package org.springframework.http.server.reactive; import java.util.AbstractSet; +import java.util.ArrayList; import java.util.Collection; import java.util.Iterator; import java.util.List; @@ -36,6 +37,7 @@ * {@code MultiValueMap} implementation for wrapping Undertow HTTP headers. * * @author Brian Clozel + * @author Sam Brannen * @since 5.1.1 */ class UndertowHeadersAdapter implements MultiValueMap { @@ -131,7 +133,10 @@ public List put(String key, List value) { @Nullable public List remove(Object key) { if (key instanceof String) { - this.headers.remove((String) key); + Collection removed = this.headers.remove((String) key); + if (removed != null) { + return new ArrayList<>(removed); + } } return null; } diff --git a/spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java b/spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java index e38fdff9b53c..685f376b426a 100644 --- a/spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java +++ b/spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java @@ -405,16 +405,18 @@ else if (index1 == requestUri.length()) { * */ private static String getSanitizedPath(final String path) { - int index = path.indexOf("//"); - if (index >= 0) { - StringBuilder sanitized = new StringBuilder(path); - while (index != -1) { - sanitized.deleteCharAt(index); - index = sanitized.indexOf("//", index); + int start = path.indexOf("//"); + if (start == -1) { + return path; + } + char[] content = path.toCharArray(); + int slowIndex = start; + for (int fastIndex = start + 1; fastIndex < content.length; fastIndex++) { + if (content[fastIndex] != '/' || content[slowIndex] != '/') { + content[++slowIndex] = content[fastIndex]; } - return sanitized.toString(); } - return path; + return new String(content, 0, slowIndex + 1); } /** @@ -532,7 +534,7 @@ public String getOriginatingServletPath(HttpServletRequest request) { */ public String getOriginatingQueryString(HttpServletRequest request) { if ((request.getAttribute(WebUtils.FORWARD_REQUEST_URI_ATTRIBUTE) != null) || - (request.getAttribute(WebUtils.ERROR_REQUEST_URI_ATTRIBUTE) != null)) { + (request.getAttribute(WebUtils.ERROR_REQUEST_URI_ATTRIBUTE) != null)) { return (String) request.getAttribute(WebUtils.FORWARD_QUERY_STRING_ATTRIBUTE); } else { diff --git a/spring-web/src/test/java/org/springframework/http/codec/multipart/DefaultPartHttpMessageReaderTests.java b/spring-web/src/test/java/org/springframework/http/codec/multipart/DefaultPartHttpMessageReaderTests.java index 8e812e720fb0..9179a546820f 100644 --- a/spring-web/src/test/java/org/springframework/http/codec/multipart/DefaultPartHttpMessageReaderTests.java +++ b/spring-web/src/test/java/org/springframework/http/codec/multipart/DefaultPartHttpMessageReaderTests.java @@ -270,6 +270,31 @@ public void utf8Headers(String displayName, DefaultPartHttpMessageReader reader) latch.await(); } + // gh-27612 + @Test + public void exceedHeaderLimit() throws InterruptedException { + Flux body = DataBufferUtils + .readByteChannel((new ClassPathResource("files.multipart", getClass()))::readableChannel, bufferFactory, 282); + + MediaType contentType = new MediaType("multipart", "form-data", singletonMap("boundary", "----WebKitFormBoundaryG8fJ50opQOML0oGD")); + MockServerHttpRequest request = MockServerHttpRequest.post("/") + .contentType(contentType) + .body(body); + + DefaultPartHttpMessageReader reader = new DefaultPartHttpMessageReader(); + + reader.setMaxHeadersSize(230); + + Flux result = reader.read(forClass(Part.class), request, emptyMap()); + + CountDownLatch latch = new CountDownLatch(2); + StepVerifier.create(result) + .consumeNextWith(part -> testPart(part, null, LOREM_IPSUM, latch)) + .consumeNextWith(part -> testPart(part, null, MUSPI_MEROL, latch)) + .verifyComplete(); + + latch.await(); + } private void testBrowser(DefaultPartHttpMessageReader reader, Resource resource, String boundary) throws InterruptedException { diff --git a/spring-web/src/test/java/org/springframework/web/util/UrlPathHelperTests.java b/spring-web/src/test/java/org/springframework/web/util/UrlPathHelperTests.java index e3281e55273f..e7ef05083780 100644 --- a/spring-web/src/test/java/org/springframework/web/util/UrlPathHelperTests.java +++ b/spring-web/src/test/java/org/springframework/web/util/UrlPathHelperTests.java @@ -232,12 +232,12 @@ void removeDuplicateSlashesInPath() { request.setContextPath("/SPR-12372"); request.setPathInfo(null); request.setServletPath("/foo/bar/"); - request.setRequestURI("/SPR-12372/foo//bar/"); + request.setRequestURI("/SPR-12372/foo///bar/"); assertThat(helper.getLookupPathForRequest(request)).isEqualTo("/foo/bar/"); request.setServletPath("/foo/bar/"); - request.setRequestURI("/SPR-12372/foo/bar//"); + request.setRequestURI("////SPR-12372/foo/bar//"); assertThat(helper.getLookupPathForRequest(request)).isEqualTo("/foo/bar/"); @@ -246,6 +246,12 @@ void removeDuplicateSlashesInPath() { request.setRequestURI("/SPR-12372/foo/bar//"); assertThat(helper.getLookupPathForRequest(request)).isEqualTo("/foo/bar//"); + + // "enhance" case + request.setServletPath("/foo/bar//"); + request.setRequestURI("/SPR-12372////////////////////////foo//////////////////bar////////////////////"); + + assertThat(helper.getLookupPathForRequest(request)).isEqualTo("/foo/bar//"); } @Test diff --git a/spring-web/src/test/resources/org/springframework/http/codec/multipart/files.multipart b/spring-web/src/test/resources/org/springframework/http/codec/multipart/files.multipart index 03b41190647e..bd10aea6b0a3 100644 --- a/spring-web/src/test/resources/org/springframework/http/codec/multipart/files.multipart +++ b/spring-web/src/test/resources/org/springframework/http/codec/multipart/files.multipart @@ -3,11 +3,9 @@ Content-Disposition: form-data; name="file2"; filename="a.txt" Content-Type: text/plain Lorem ipsum dolor sit amet, consectetur adipiscing elit. Integer iaculis metus id vestibulum nullam. - ------WebKitFormBoundaryG8fJ50opQOML0oGD Content-Disposition: form-data; name="file2"; filename="b.txt" Content-Type: text/plain .mallun mulubitsev di sutem silucai regetnI .tile gnicsipida rutetcesnoc ,tema tis rolod muspi meroL - ------WebKitFormBoundaryG8fJ50opQOML0oGD-- diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/config/ResourceHandlerRegistration.java b/spring-webflux/src/main/java/org/springframework/web/reactive/config/ResourceHandlerRegistration.java index 8ee0e87eafb1..9734347cc665 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/config/ResourceHandlerRegistration.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/config/ResourceHandlerRegistration.java @@ -54,6 +54,8 @@ public class ResourceHandlerRegistration { private boolean useLastModified = true; + private boolean optimizeLocations = false; + @Nullable private Map mediaTypes; @@ -105,15 +107,33 @@ public ResourceHandlerRegistration setCacheControl(CacheControl cacheControl) { /** * Set whether the {@link Resource#lastModified()} information should be used to drive HTTP responses. *

This configuration is set to {@code true} by default. - * @param useLastModified whether the "last modified" resource information should be used. + * @param useLastModified whether the "last modified" resource information should be used * @return the same {@link ResourceHandlerRegistration} instance, for chained method invocation * @since 5.3 + * @see ResourceWebHandler#setUseLastModified */ public ResourceHandlerRegistration setUseLastModified(boolean useLastModified) { this.useLastModified = useLastModified; return this; } + /** + * Set whether to optimize the specified locations through an existence check on startup, + * filtering non-existing directories upfront so that they do not have to be checked + * on every resource access. + *

The default is {@code false}, for defensiveness against zip files without directory + * entries which are unable to expose the existence of a directory upfront. Switch this flag to + * {@code true} for optimized access in case of a consistent jar layout with directory entries. + * @param optimizeLocations whether to optimize the locations through an existence check on startup + * @return the same {@link ResourceHandlerRegistration} instance, for chained method invocation + * @since 5.3.13 + * @see ResourceWebHandler#setOptimizeLocations + */ + public ResourceHandlerRegistration setOptimizeLocations(boolean optimizeLocations) { + this.optimizeLocations = optimizeLocations; + return this; + } + /** * Configure a chain of resource resolvers and transformers to use. This * can be useful, for example, to apply a version strategy to resource URLs. @@ -181,8 +201,8 @@ protected String[] getPathPatterns() { */ protected ResourceWebHandler getRequestHandler() { ResourceWebHandler handler = new ResourceWebHandler(); - handler.setLocationValues(this.locationValues); handler.setResourceLoader(this.resourceLoader); + handler.setLocationValues(this.locationValues); if (this.resourceChainRegistration != null) { handler.setResourceResolvers(this.resourceChainRegistration.getResourceResolvers()); handler.setResourceTransformers(this.resourceChainRegistration.getResourceTransformers()); @@ -191,6 +211,7 @@ protected ResourceWebHandler getRequestHandler() { handler.setCacheControl(this.cacheControl); } handler.setUseLastModified(this.useLastModified); + handler.setOptimizeLocations(this.optimizeLocations); if (this.mediaTypes != null) { handler.setMediaTypes(this.mediaTypes); } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponse.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponse.java index 0931bf214072..37f14f34ae0b 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponse.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponse.java @@ -195,13 +195,7 @@ public Mono>> toEntityList(ParameterizedTypeReference @Override public Mono createException() { - return DataBufferUtils.join(body(BodyExtractors.toDataBuffers())) - .map(dataBuffer -> { - byte[] bytes = new byte[dataBuffer.readableByteCount()]; - dataBuffer.read(bytes); - DataBufferUtils.release(dataBuffer); - return bytes; - }) + return bodyToMono(byte[].class) .defaultIfEmpty(EMPTY) .onErrorReturn(ex -> !(ex instanceof Error), EMPTY) .map(bodyBytes -> { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java index c2ba1b234bf1..e6f0640a7022 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java @@ -72,8 +72,8 @@ *

This request handler may also be configured with a * {@link #setResourceResolvers(List) resourcesResolver} and * {@link #setResourceTransformers(List) resourceTransformer} chains to support - * arbitrary resolution and transformation of resources being served. By default a - * {@link PathResourceResolver} simply finds resources based on the configured + * arbitrary resolution and transformation of resources being served. By default + * a {@link PathResourceResolver} simply finds resources based on the configured * "locations". An application can configure additional resolvers and * transformers such as the {@link VersionResourceResolver} which can resolve * and prepare URLs for resources with a version in the URL. @@ -85,6 +85,7 @@ * * @author Rossen Stoyanchev * @author Brian Clozel + * @author Juergen Hoeller * @since 5.0 */ public class ResourceWebHandler implements WebHandler, InitializingBean { @@ -94,6 +95,9 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { private static final Log logger = LogFactory.getLog(ResourceWebHandler.class); + @Nullable + private ResourceLoader resourceLoader; + private final List locationValues = new ArrayList<>(4); private final List locationResources = new ArrayList<>(4); @@ -119,11 +123,18 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { @Nullable private Map mediaTypes; - @Nullable - private ResourceLoader resourceLoader; - private boolean useLastModified = true; + private boolean optimizeLocations = false; + + + /** + * Provide the ResourceLoader to load {@link #setLocationValues location values} with. + * @since 5.1 + */ + public void setResourceLoader(ResourceLoader resourceLoader) { + this.resourceLoader = resourceLoader; + } /** * Accepts a list of String-based location values to be resolved into @@ -161,9 +172,9 @@ public void setLocations(@Nullable List locations) { *

Note that if {@link #setLocationValues(List) locationValues} are provided, * instead of loaded Resource-based locations, this method will return * empty until after initialization via {@link #afterPropertiesSet()}. - *

Note: As of 5.3.11 the list of locations is filtered - * to exclude those that don't actually exist and therefore the list returned - * from this method may be a subset of all given locations. + *

Note: As of 5.3.11 the list of locations may be filtered to + * exclude those that don't actually exist and therefore the list returned from this + * method may be a subset of all given locations. See {@link #setOptimizeLocations}. * @see #setLocationValues * @see #setLocations */ @@ -212,6 +223,22 @@ public List getResourceTransformers() { return this.resourceTransformers; } + /** + * Configure the {@link ResourceHttpMessageWriter} to use. + *

By default a {@link ResourceHttpMessageWriter} will be configured. + */ + public void setResourceHttpMessageWriter(@Nullable ResourceHttpMessageWriter httpMessageWriter) { + this.resourceHttpMessageWriter = httpMessageWriter; + } + + /** + * Return the configured resource message writer. + */ + @Nullable + public ResourceHttpMessageWriter getResourceHttpMessageWriter() { + return this.resourceHttpMessageWriter; + } + /** * Set the {@link org.springframework.http.CacheControl} instance to build * the Cache-Control HTTP response header. @@ -230,19 +257,48 @@ public CacheControl getCacheControl() { } /** - * Configure the {@link ResourceHttpMessageWriter} to use. - *

By default a {@link ResourceHttpMessageWriter} will be configured. + * Set whether we should look at the {@link Resource#lastModified()} + * when serving resources and use this information to drive {@code "Last-Modified"} + * HTTP response headers. + *

This option is enabled by default and should be turned off if the metadata of + * the static files should be ignored. + * @since 5.3 */ - public void setResourceHttpMessageWriter(@Nullable ResourceHttpMessageWriter httpMessageWriter) { - this.resourceHttpMessageWriter = httpMessageWriter; + public void setUseLastModified(boolean useLastModified) { + this.useLastModified = useLastModified; } /** - * Return the configured resource message writer. + * Return whether the {@link Resource#lastModified()} information is used + * to drive HTTP responses when serving static resources. + * @since 5.3 */ - @Nullable - public ResourceHttpMessageWriter getResourceHttpMessageWriter() { - return this.resourceHttpMessageWriter; + public boolean isUseLastModified() { + return this.useLastModified; + } + + /** + * Set whether to optimize the specified locations through an existence + * check on startup, filtering non-existing directories upfront so that + * they do not have to be checked on every resource access. + *

The default is {@code false}, for defensiveness against zip files + * without directory entries which are unable to expose the existence of + * a directory upfront. Switch this flag to {@code true} for optimized + * access in case of a consistent jar layout with directory entries. + * @since 5.3.13 + */ + public void setOptimizeLocations(boolean optimizeLocations) { + this.optimizeLocations = optimizeLocations; + } + + /** + * Return whether to optimize the specified locations through an existence + * check on startup, filtering non-existing directories upfront so that + * they do not have to be checked on every resource access. + * @since 5.3.13 + */ + public boolean isOptimizeLocations() { + return this.optimizeLocations; } /** @@ -269,36 +325,6 @@ public Map getMediaTypes() { return (this.mediaTypes != null ? this.mediaTypes : Collections.emptyMap()); } - /** - * Provide the ResourceLoader to load {@link #setLocationValues(List) - * location values} with. - * @since 5.1 - */ - public void setResourceLoader(ResourceLoader resourceLoader) { - this.resourceLoader = resourceLoader; - } - - /** - * Return whether the {@link Resource#lastModified()} information is used - * to drive HTTP responses when serving static resources. - * @since 5.3 - */ - public boolean isUseLastModified() { - return this.useLastModified; - } - - /** - * Set whether we should look at the {@link Resource#lastModified()} - * when serving resources and use this information to drive {@code "Last-Modified"} - * HTTP response headers. - *

This option is enabled by default and should be turned off if the metadata of - * the static files should be ignored. - * @param useLastModified whether to use the resource last-modified information. - * @since 5.3 - */ - public void setUseLastModified(boolean useLastModified) { - this.useLastModified = useLastModified; - } @Override public void afterPropertiesSet() throws Exception { @@ -332,7 +358,9 @@ private void resolveResourceLocations() { } } - result = result.stream().filter(Resource::exists).collect(Collectors.toList()); + if (isOptimizeLocations()) { + result = result.stream().filter(Resource::exists).collect(Collectors.toList()); + } this.locationsToUse.clear(); this.locationsToUse.addAll(result); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientResponseTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientResponseTests.java index 0b21511feab1..f9b626a60c79 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientResponseTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientResponseTests.java @@ -30,6 +30,7 @@ import reactor.core.publisher.Mono; import org.springframework.core.ParameterizedTypeReference; +import org.springframework.core.codec.ByteArrayDecoder; import org.springframework.core.codec.StringDecoder; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DefaultDataBuffer; @@ -48,6 +49,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.assertj.core.api.Assertions.entry; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; import static org.springframework.web.reactive.function.BodyExtractors.toMono; @@ -328,6 +330,29 @@ public void toEntityListTypeReference() { assertThat(result.getHeaders().getContentType()).isEqualTo(MediaType.TEXT_PLAIN); } + @Test + public void createException() { + byte[] bytes = "foo".getBytes(StandardCharsets.UTF_8); + DefaultDataBuffer dataBuffer = DefaultDataBufferFactory.sharedInstance.wrap(ByteBuffer.wrap(bytes)); + Flux body = Flux.just(dataBuffer); + httpHeaders.setContentType(MediaType.TEXT_PLAIN); + given(mockResponse.getStatusCode()).willReturn(HttpStatus.NOT_FOUND); + given(mockResponse.getRawStatusCode()).willReturn(HttpStatus.NOT_FOUND.value()); + given(mockResponse.getBody()).willReturn(body); + + List> messageReaders = Collections.singletonList( + new DecoderHttpMessageReader<>(new ByteArrayDecoder())); + given(mockExchangeStrategies.messageReaders()).willReturn(messageReaders); + + Mono resultMono = defaultClientResponse.createException(); + WebClientResponseException exception = resultMono.block(); + assertThat(exception.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND); + assertThat(exception.getMessage()).isEqualTo("404 Not Found"); + assertThat(exception.getHeaders()).containsExactly(entry("Content-Type", + Collections.singletonList("text/plain"))); + assertThat(exception.getResponseBodyAsByteArray()).isEqualTo(bytes); + } + private void mockTextPlainResponse(Flux body) { httpHeaders.setContentType(MediaType.TEXT_PLAIN); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultWebClientTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultWebClientTests.java index 37928d2fb1b7..a33a24c01647 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultWebClientTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultWebClientTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2021 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. @@ -133,6 +133,7 @@ public void requestHeaderAndCookie() { } @Test + @SuppressWarnings("deprecation") public void contextFromThreadLocal() { WebClient client = this.builder .filter((request, next) -> diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java index b668172197d6..c213b1672ac4 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java @@ -74,6 +74,7 @@ public class ResourceWebHandlerTests { private ResourceWebHandler handler; + @BeforeEach public void setup() throws Exception { List locations = new ArrayList<>(2); @@ -253,7 +254,7 @@ public void getResourceFromFileSystem() throws Exception { assertResponseBody(exchange, "h1 { color:red; }"); } - @Test // gh-27538 + @Test // gh-27538, gh-27624 public void filterNonExistingLocations() throws Exception { List inputLocations = Arrays.asList( new ClassPathResource("test/", getClass()), @@ -262,6 +263,7 @@ public void filterNonExistingLocations() throws Exception { ResourceWebHandler handler = new ResourceWebHandler(); handler.setLocations(inputLocations); + handler.setOptimizeLocations(true); handler.afterPropertiesSet(); List actual = handler.getLocations(); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ResourceHandlerRegistration.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ResourceHandlerRegistration.java index fd822bedd55b..c173f602455e 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ResourceHandlerRegistration.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/ResourceHandlerRegistration.java @@ -55,6 +55,8 @@ public class ResourceHandlerRegistration { private boolean useLastModified = true; + private boolean optimizeLocations = false; + /** * Create a {@link ResourceHandlerRegistration} instance. @@ -130,15 +132,33 @@ public ResourceHandlerRegistration setCacheControl(CacheControl cacheControl) { /** * Set whether the {@link Resource#lastModified()} information should be used to drive HTTP responses. *

This configuration is set to {@code true} by default. - * @param useLastModified whether the "last modified" resource information should be used. + * @param useLastModified whether the "last modified" resource information should be used * @return the same {@link ResourceHandlerRegistration} instance, for chained method invocation * @since 5.3 + * @see ResourceHttpRequestHandler#setUseLastModified */ public ResourceHandlerRegistration setUseLastModified(boolean useLastModified) { this.useLastModified = useLastModified; return this; } + /** + * Set whether to optimize the specified locations through an existence check on startup, + * filtering non-existing directories upfront so that they do not have to be checked + * on every resource access. + *

The default is {@code false}, for defensiveness against zip files without directory + * entries which are unable to expose the existence of a directory upfront. Switch this flag to + * {@code true} for optimized access in case of a consistent jar layout with directory entries. + * @param optimizeLocations whether to optimize the locations through an existence check on startup + * @return the same {@link ResourceHandlerRegistration} instance, for chained method invocation + * @since 5.3.13 + * @see ResourceHttpRequestHandler#setOptimizeLocations + */ + public ResourceHandlerRegistration setOptimizeLocations(boolean optimizeLocations) { + this.optimizeLocations = optimizeLocations; + return this; + } + /** * Configure a chain of resource resolvers and transformers to use. This * can be useful, for example, to apply a version strategy to resource URLs. @@ -204,6 +224,7 @@ else if (this.cachePeriod != null) { handler.setCacheSeconds(this.cachePeriod); } handler.setUseLastModified(this.useLastModified); + handler.setOptimizeLocations(this.optimizeLocations); return handler; } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java index 22f06ccf58d7..7855a1171cb3 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java @@ -140,11 +140,13 @@ public class ResourceHttpRequestHandler extends WebContentGenerator @Nullable private UrlPathHelper urlPathHelper; + private boolean useLastModified = true; + + private boolean optimizeLocations = false; + @Nullable private StringValueResolver embeddedValueResolver; - private boolean useLastModified = true; - public ResourceHttpRequestHandler() { super(HttpMethod.GET.name(), HttpMethod.HEAD.name()); @@ -185,13 +187,13 @@ public void setLocations(List locations) { /** * Return the configured {@code List} of {@code Resource} locations including * both String-based locations provided via - * {@link #setLocationValues(List) setLocationValues} and pre-resolved {@code Resource} - * locations provided via {@link #setLocations(List) setLocations}. + * {@link #setLocationValues(List) setLocationValues} and pre-resolved + * {@code Resource} locations provided via {@link #setLocations(List) setLocations}. *

Note that the returned list is fully initialized only after * initialization via {@link #afterPropertiesSet()}. - *

Note: As of 5.3.11 the list of locations is filtered - * to exclude those that don't actually exist and therefore the list returned - * from this method may be a subset of all given locations. + *

Note: As of 5.3.11 the list of locations may be filtered to + * exclude those that don't actually exist and therefore the list returned from this + * method may be a subset of all given locations. See {@link #setOptimizeLocations}. * @see #setLocationValues * @see #setLocations */ @@ -293,7 +295,7 @@ public void setContentNegotiationManager(@Nullable ContentNegotiationManager con /** * Return the configured content negotiation manager. * @since 4.3 - * @deprecated as of 5.2.4. + * @deprecated as of 5.2.4 */ @Nullable @Deprecated @@ -303,7 +305,7 @@ public ContentNegotiationManager getContentNegotiationManager() { /** * Add mappings between file extensions, extracted from the filename of a - * static {@link Resource}, and corresponding media type to set on the + * static {@link Resource}, and corresponding media type to set on the * response. *

Use of this method is typically not necessary since mappings are * otherwise determined via @@ -361,9 +363,16 @@ public UrlPathHelper getUrlPathHelper() { return this.urlPathHelper; } - @Override - public void setEmbeddedValueResolver(StringValueResolver resolver) { - this.embeddedValueResolver = resolver; + /** + * Set whether we should look at the {@link Resource#lastModified()} when + * serving resources and use this information to drive {@code "Last-Modified"} + * HTTP response headers. + *

This option is enabled by default and should be turned off if the metadata + * of the static files should be ignored. + * @since 5.3 + */ + public void setUseLastModified(boolean useLastModified) { + this.useLastModified = useLastModified; } /** @@ -376,18 +385,35 @@ public boolean isUseLastModified() { } /** - * Set whether we should look at the {@link Resource#lastModified()} - * when serving resources and use this information to drive {@code "Last-Modified"} - * HTTP response headers. - *

This option is enabled by default and should be turned off if the metadata of - * the static files should be ignored. - * @param useLastModified whether to use the resource last-modified information. - * @since 5.3 + * Set whether to optimize the specified locations through an existence + * check on startup, filtering non-existing directories upfront so that + * they do not have to be checked on every resource access. + *

The default is {@code false}, for defensiveness against zip files + * without directory entries which are unable to expose the existence of + * a directory upfront. Switch this flag to {@code true} for optimized + * access in case of a consistent jar layout with directory entries. + * @since 5.3.13 */ - public void setUseLastModified(boolean useLastModified) { - this.useLastModified = useLastModified; + public void setOptimizeLocations(boolean optimizeLocations) { + this.optimizeLocations = optimizeLocations; + } + + /** + * Return whether to optimize the specified locations through an existence + * check on startup, filtering non-existing directories upfront so that + * they do not have to be checked on every resource access. + * @since 5.3.13 + */ + public boolean isOptimizeLocations() { + return this.optimizeLocations; } + @Override + public void setEmbeddedValueResolver(StringValueResolver resolver) { + this.embeddedValueResolver = resolver; + } + + @Override public void afterPropertiesSet() throws Exception { resolveResourceLocations(); @@ -449,8 +475,8 @@ private void resolveResourceLocations() { if (location.equals("/") && !(resource instanceof ServletContextResource)) { throw new IllegalStateException( "The String-based location \"/\" should be relative to the web application root " + - "but resolved to a Resource of type: " + resource.getClass() + ". " + - "If this is intentional, please pass it as a pre-configured Resource via setLocations."); + "but resolved to a Resource of type: " + resource.getClass() + ". " + + "If this is intentional, please pass it as a pre-configured Resource via setLocations."); } result.add(resource); if (charset != null) { @@ -463,7 +489,9 @@ private void resolveResourceLocations() { } result.addAll(this.locationResources); - result = result.stream().filter(Resource::exists).collect(Collectors.toList()); + if (isOptimizeLocations()) { + result = result.stream().filter(Resource::exists).collect(Collectors.toList()); + } this.locationsToUse.clear(); this.locationsToUse.addAll(result); @@ -508,6 +536,7 @@ protected org.springframework.web.accept.PathExtensionContentNegotiationStrategy return null; } + /** * Processes a resource request. *

Checks for the existence of the requested resource in the configured list of locations. diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java index 9aefd7f9d07b..834aab2694a6 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java @@ -311,7 +311,7 @@ public String getMimeType(String filePath) { assertThat(this.response.getContentAsString()).isEqualTo("h1 { color:red; }"); } - @Test // gh-27538 + @Test // gh-27538, gh-27624 public void filterNonExistingLocations() throws Exception { List inputLocations = Arrays.asList( new ClassPathResource("test/", getClass()), @@ -321,6 +321,7 @@ public void filterNonExistingLocations() throws Exception { ResourceHttpRequestHandler handler = new ResourceHttpRequestHandler(); handler.setServletContext(new MockServletContext()); handler.setLocations(inputLocations); + handler.setOptimizeLocations(true); handler.afterPropertiesSet(); List actual = handler.getLocations(); diff --git a/src/docs/asciidoc/core/core-beans.adoc b/src/docs/asciidoc/core/core-beans.adoc index 5ba741761f28..8d701befcbcc 100644 --- a/src/docs/asciidoc/core/core-beans.adoc +++ b/src/docs/asciidoc/core/core-beans.adoc @@ -3039,7 +3039,7 @@ constructor or setter argument or autowired field) as `ObjectFactory`, which delivers +As an extended variant, you may declare `ObjectProvider` which delivers several additional access variants, including `getIfAvailable` and `getIfUnique`. The JSR-330 variant of this is called `Provider` and is used with a `Provider` @@ -6675,9 +6675,11 @@ factory method and other bean definition properties, such as a qualifier value t the `@Qualifier` annotation. Other method-level annotations that can be specified are `@Scope`, `@Lazy`, and custom qualifier annotations. -TIP: In addition to its role for component initialization, you can also place the `@Lazy` annotation -on injection points marked with `@Autowired` or `@Inject`. In this context, it -leads to the injection of a lazy-resolution proxy. +TIP: In addition to its role for component initialization, you can also place the `@Lazy` +annotation on injection points marked with `@Autowired` or `@Inject`. In this context, +it leads to the injection of a lazy-resolution proxy. However, such a proxy approach +is rather limited. For sophisticated lazy interactions, in particular in combination +with optional dependencies, we recommend `ObjectProvider` instead. Autowired fields and methods are supported, as previously discussed, with additional support for autowiring of `@Bean` methods. The following example shows how to do so: diff --git a/src/docs/asciidoc/web/webflux-webclient.adoc b/src/docs/asciidoc/web/webflux-webclient.adoc index 22e0d265136e..3099ecb02d26 100644 --- a/src/docs/asciidoc/web/webflux-webclient.adoc +++ b/src/docs/asciidoc/web/webflux-webclient.adoc @@ -89,7 +89,7 @@ modified copy as follows: === MaxInMemorySize Codecs have <> for buffering data in -memory to avoid application memory issues. By the default those are set to 256KB. +memory to avoid application memory issues. By default those are set to 256KB. If that's not enough you'll get the following error: ---- diff --git a/src/docs/asciidoc/web/webflux.adoc b/src/docs/asciidoc/web/webflux.adoc index 3b601ea4c885..1cc302102126 100644 --- a/src/docs/asciidoc/web/webflux.adoc +++ b/src/docs/asciidoc/web/webflux.adoc @@ -4170,8 +4170,8 @@ the example: @Override public void addResourceHandlers(ResourceHandlerRegistry registry) { registry.addResourceHandler("/resources/**") - .addResourceLocations("/public", "classpath:/static/") - .setCacheControl(CacheControl.maxAge(365, TimeUnit.DAYS)); + .addResourceLocations("/public", "classpath:/static/") + .setCacheControl(CacheControl.maxAge(365, TimeUnit.DAYS)); } } @@ -4259,6 +4259,9 @@ re-write URLs to include the version of the jar and can also match against incom without versions -- for example, from `/jquery/jquery.min.js` to `/jquery/1.2.0/jquery.min.js`. +TIP: The Java configuration based on `ResourceHandlerRegistry` provides further options +for fine-grained control, e.g. last-modified behavior and optimized resource resolution. + [[webflux-config-path-matching]] diff --git a/src/docs/asciidoc/web/webmvc.adoc b/src/docs/asciidoc/web/webmvc.adoc index b49666303e98..33b49bef56a5 100644 --- a/src/docs/asciidoc/web/webmvc.adoc +++ b/src/docs/asciidoc/web/webmvc.adoc @@ -5738,8 +5738,8 @@ The following listing shows how to do so with Java configuration: @Override public void addResourceHandlers(ResourceHandlerRegistry registry) { registry.addResourceHandler("/resources/**") - .addResourceLocations("/public", "classpath:/static/") - .setCacheControl(CacheControl.maxAge(Duration.ofDays(365))); + .addResourceLocations("/public", "classpath:/static/") + .setCacheControl(CacheControl.maxAge(Duration.ofDays(365))); } } ---- @@ -5846,6 +5846,9 @@ re-write URLs to include the version of the jar and can also match against incom without versions -- for example, from `/jquery/jquery.min.js` to `/jquery/1.2.0/jquery.min.js`. +TIP: The Java configuration based on `ResourceHandlerRegistry` provides further options +for fine-grained control, e.g. last-modified behavior and optimized resource resolution. + [[mvc-default-servlet-handler]]