From 86f38e15f00910328df5f8b751c2d48b5532e57a Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 22 Aug 2022 20:00:06 -0700 Subject: [PATCH] Polish 'Align MimeMappings with Tomcat's defaults' Refine `MimeMappings` so that common default mappings are included by default and the complete set is only loaded when needed. The `TomcatServletWebServerFactory` has been updated so that if `TomcatEmbeddedContext` is in use the mime mapping are used directly rather than being copied to another Map. The `AbstractServletWebServerFactory` class has also been changed to use a lazy copy of the mappings. This should mean that the complete set of properties is only loaded if the user mutates the mappings. See gh-32101 --- .../tomcat/TomcatEmbeddedContext.java | 27 ++ .../tomcat/TomcatServletWebServerFactory.java | 14 +- .../boot/web/server/MimeMappings.java | 288 ++++++++++++++---- .../AbstractServletWebServerFactory.java | 3 +- .../resources/META-INF/spring/aot.factories | 5 +- .../boot/web/server/MimeMappingsTests.java | 73 +++++ 6 files changed, 351 insertions(+), 59 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatEmbeddedContext.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatEmbeddedContext.java index 633e6df3d92..48f37a1bb93 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatEmbeddedContext.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatEmbeddedContext.java @@ -17,6 +17,7 @@ package org.springframework.boot.web.embedded.tomcat; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.TreeMap; @@ -30,6 +31,7 @@ import org.apache.catalina.core.StandardContext; import org.apache.catalina.core.StandardWrapper; import org.apache.catalina.session.ManagerBase; +import org.springframework.boot.web.server.MimeMappings; import org.springframework.boot.web.server.WebServerException; import org.springframework.util.ClassUtils; @@ -44,6 +46,8 @@ class TomcatEmbeddedContext extends StandardContext { private TomcatStarter starter; + private MimeMappings mimeMappings; + @Override public boolean loadOnStartup(Container[] children) { // deferred until later (see deferredLoadOnStartup) @@ -118,4 +122,27 @@ class TomcatEmbeddedContext extends StandardContext { return this.starter; } + void setMimeMappings(MimeMappings mimeMappings) { + this.mimeMappings = mimeMappings; + } + + @Override + public String[] findMimeMappings() { + List mappings = new ArrayList<>(); + mappings.addAll(Arrays.asList(super.findMimeMappings())); + if (this.mimeMappings != null) { + this.mimeMappings.forEach((mapping) -> mappings.add(mapping.getExtension())); + } + return mappings.toArray(String[]::new); + } + + @Override + public String findMimeMapping(String extension) { + String mimeMapping = super.findMimeMapping(extension); + if (mimeMapping != null) { + return mimeMapping; + } + return (this.mimeMappings != null) ? this.mimeMappings.get(extension) : null; + } + } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactory.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactory.java index 5d3086143c8..9420c7696d7 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactory.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactory.java @@ -389,9 +389,7 @@ public class TomcatServletWebServerFactory extends AbstractServletWebServerFacto tomcatErrorPage.setExceptionType(errorPage.getExceptionName()); context.addErrorPage(tomcatErrorPage); } - for (MimeMappings.Mapping mapping : getMimeMappings()) { - context.addMimeMapping(mapping.getExtension(), mapping.getMimeType()); - } + setMimeMappings(context); configureSession(context); configureCookieProcessor(context); new DisableReferenceClearingContextCustomizer().customize(context); @@ -423,6 +421,16 @@ public class TomcatServletWebServerFactory extends AbstractServletWebServerFacto } } + private void setMimeMappings(Context context) { + if (context instanceof TomcatEmbeddedContext embeddedContext) { + embeddedContext.setMimeMappings(getMimeMappings()); + return; + } + for (MimeMappings.Mapping mapping : getMimeMappings()) { + context.addMimeMapping(mapping.getExtension(), mapping.getMimeType()); + } + } + private void configureCookieProcessor(Context context) { SameSite sessionSameSite = getSession().getCookie().getSameSite(); List suppliers = new ArrayList<>(); diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/server/MimeMappings.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/server/MimeMappings.java index 3c92df87c0a..b731532830c 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/server/MimeMappings.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/server/MimeMappings.java @@ -21,9 +21,13 @@ import java.util.Collection; import java.util.Collections; import java.util.Iterator; import java.util.LinkedHashMap; +import java.util.Locale; import java.util.Map; -import java.util.Properties; +import java.util.Map.Entry; +import java.util.concurrent.atomic.AtomicBoolean; +import org.springframework.aot.hint.RuntimeHints; +import org.springframework.aot.hint.RuntimeHintsRegistrar; import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.support.PropertiesLoaderUtils; import org.springframework.util.Assert; @@ -33,29 +37,15 @@ import org.springframework.util.Assert; * {@literal <mime-mapping>} element traditionally found in web.xml. * * @author Phillip Webb + * @author Guirong Hu * @since 2.0.0 */ -public final class MimeMappings implements Iterable { +public sealed class MimeMappings implements Iterable { /** * Default mime mapping commonly used. */ - public static final MimeMappings DEFAULT; - - static { - MimeMappings mappings = new MimeMappings(); - try { - Properties defaultMimeMappings = PropertiesLoaderUtils - .loadProperties(new ClassPathResource("mime-mappings.properties", MimeMappings.class)); - for (String extension : defaultMimeMappings.stringPropertyNames()) { - mappings.add(extension, defaultMimeMappings.getProperty(extension)); - } - } - catch (IOException ex) { - throw new IllegalArgumentException("Unable to load the default MIME types", ex); - } - DEFAULT = unmodifiableMappings(mappings); - } + public static final MimeMappings DEFAULT = new DefaultMimeMappings(); private final Map map; @@ -90,14 +80,44 @@ public final class MimeMappings implements Iterable { * @param mappings source mappings * @param mutable if the new object should be mutable. */ - private MimeMappings(MimeMappings mappings, boolean mutable) { + MimeMappings(MimeMappings mappings, boolean mutable) { Assert.notNull(mappings, "Mappings must not be null"); this.map = (mutable ? new LinkedHashMap<>(mappings.map) : Collections.unmodifiableMap(mappings.map)); } - @Override - public Iterator iterator() { - return getAll().iterator(); + /** + * Add a new mime mapping. + * @param extension the file extension (excluding '.') + * @param mimeType the mime type to map + * @return any previous mapping or {@code null} + */ + public String add(String extension, String mimeType) { + Assert.notNull(extension, "Extension must not be null"); + Assert.notNull(mimeType, "MimeType must not be null"); + Mapping previous = this.map.put(extension.toLowerCase(Locale.ENGLISH), new Mapping(extension, mimeType)); + return (previous != null) ? previous.getMimeType() : null; + } + + /** + * Remove an existing mapping. + * @param extension the file extension (excluding '.') + * @return the removed mime mapping or {@code null} if no item was removed + */ + public String remove(String extension) { + Assert.notNull(extension, "Extension must not be null"); + Mapping previous = this.map.remove(extension.toLowerCase(Locale.ENGLISH)); + return (previous != null) ? previous.getMimeType() : null; + } + + /** + * Get a mime mapping for the given extension. + * @param extension the file extension (excluding '.') + * @return a mime mapping or {@code null} + */ + public String get(String extension) { + Assert.notNull(extension, "Extension must not be null"); + Mapping mapping = this.map.get(extension.toLowerCase(Locale.ENGLISH)); + return (mapping != null) ? mapping.getMimeType() : null; } /** @@ -108,35 +128,9 @@ public final class MimeMappings implements Iterable { return this.map.values(); } - /** - * Add a new mime mapping. - * @param extension the file extension (excluding '.') - * @param mimeType the mime type to map - * @return any previous mapping or {@code null} - */ - public String add(String extension, String mimeType) { - Mapping previous = this.map.put(extension, new Mapping(extension, mimeType)); - return (previous != null) ? previous.getMimeType() : null; - } - - /** - * Get a mime mapping for the given extension. - * @param extension the file extension (excluding '.') - * @return a mime mapping or {@code null} - */ - public String get(String extension) { - Mapping mapping = this.map.get(extension); - return (mapping != null) ? mapping.getMimeType() : null; - } - - /** - * Remove an existing mapping. - * @param extension the file extension (excluding '.') - * @return the removed mime mapping or {@code null} if no item was removed - */ - public String remove(String extension) { - Mapping previous = this.map.remove(extension); - return (previous != null) ? previous.getMimeType() : null; + @Override + public final Iterator iterator() { + return getAll().iterator(); } @Override @@ -148,14 +142,18 @@ public final class MimeMappings implements Iterable { return true; } if (obj instanceof MimeMappings other) { - return this.map.equals(other.map); + return getMap().equals(other.map); } return false; } @Override public int hashCode() { - return this.map.hashCode(); + return getMap().hashCode(); + } + + Map getMap() { + return this.map; } /** @@ -165,9 +163,22 @@ public final class MimeMappings implements Iterable { * @return an unmodifiable view of the specified mappings. */ public static MimeMappings unmodifiableMappings(MimeMappings mappings) { + Assert.notNull(mappings, "Mappings must not be null"); return new MimeMappings(mappings, false); } + /** + * Crate a new lazy copy of the given mappings will only copy entries if the mappings + * are mutated. + * @param mappings the source mappings + * @return a new mappings instance + * @since 3.0.0 + */ + public static MimeMappings lazyCopy(MimeMappings mappings) { + Assert.notNull(mappings, "Mappings must not be null"); + return new LazyMimeMappingsCopy(mappings); + } + /** * A single mime mapping. */ @@ -218,4 +229,175 @@ public final class MimeMappings implements Iterable { } + /** + * {@link MimeMappings} implementation used for {@link MimeMappings#DEFAULT}. Provides + * in-memory access for common mappings and lazily loads the complete set when + * necessary. + */ + static final class DefaultMimeMappings extends MimeMappings { + + static final String MIME_MAPPINGS_PROPERTIES = "mime-mappings.properties"; + + private static final MimeMappings COMMON; + + static { + MimeMappings mappings = new MimeMappings(); + mappings.add("avi", "video/x-msvideo"); + mappings.add("bin", "application/octet-stream"); + mappings.add("body", "text/html"); + mappings.add("class", "application/java"); + mappings.add("css", "text/css"); + mappings.add("dtd", "application/xml-dtd"); + mappings.add("gif", "image/gif"); + mappings.add("gtar", "application/x-gtar"); + mappings.add("gz", "application/x-gzip"); + mappings.add("htm", "text/html"); + mappings.add("html", "text/html"); + mappings.add("jar", "application/java-archive"); + mappings.add("java", "text/x-java-source"); + mappings.add("jnlp", "application/x-java-jnlp-file"); + mappings.add("jpe", "image/jpeg"); + mappings.add("jpeg", "image/jpeg"); + mappings.add("jpg", "image/jpeg"); + mappings.add("js", "application/javascript"); + mappings.add("json", "application/json"); + mappings.add("otf", "application/x-font-opentype"); + mappings.add("pdf", "application/pdf"); + mappings.add("png", "image/png"); + mappings.add("ps", "application/postscript"); + mappings.add("tar", "application/x-tar"); + mappings.add("tif", "image/tiff"); + mappings.add("tiff", "image/tiff"); + mappings.add("ttf", "application/x-font-ttf"); + mappings.add("txt", "text/plain"); + mappings.add("xht", "application/xhtml+xml"); + mappings.add("xhtml", "application/xhtml+xml"); + mappings.add("xls", "application/vnd.ms-excel"); + mappings.add("xml", "application/xml"); + mappings.add("xsl", "application/xml"); + mappings.add("xslt", "application/xslt+xml"); + mappings.add("wasm", "application/wasm"); + mappings.add("zip", "application/zip"); + COMMON = unmodifiableMappings(mappings); + } + + private volatile Map loaded; + + DefaultMimeMappings() { + super(new MimeMappings(), false); + } + + @Override + public Collection getAll() { + return load().values(); + } + + @Override + public String get(String extension) { + Assert.notNull(extension, "Extension must not be null"); + extension = extension.toLowerCase(Locale.ENGLISH); + Map loaded = this.loaded; + if (loaded != null) { + return get(loaded, extension); + } + String commonMimeType = COMMON.get(extension); + if (commonMimeType != null) { + return commonMimeType; + } + loaded = load(); + return get(loaded, extension); + } + + private String get(Map mappings, String extension) { + Mapping mapping = mappings.get(extension); + return (mapping != null) ? mapping.getMimeType() : null; + } + + @Override + Map getMap() { + return load(); + } + + private Map load() { + Map loaded = this.loaded; + if (loaded != null) { + return loaded; + } + try { + loaded = new LinkedHashMap<>(); + for (Entry entry : PropertiesLoaderUtils + .loadProperties(new ClassPathResource(MIME_MAPPINGS_PROPERTIES, getClass())).entrySet()) { + loaded.put((String) entry.getKey(), + new Mapping((String) entry.getKey(), (String) entry.getValue())); + } + loaded = Collections.unmodifiableMap(loaded); + this.loaded = loaded; + return loaded; + } + catch (IOException ex) { + throw new IllegalArgumentException("Unable to load the default MIME types", ex); + } + } + + } + + /** + * {@link MimeMappings} implementation used to create a lazy copy only when the + * mappings are mutated. + */ + static final class LazyMimeMappingsCopy extends MimeMappings { + + private final MimeMappings source; + + private AtomicBoolean copied = new AtomicBoolean(); + + LazyMimeMappingsCopy(MimeMappings source) { + this.source = source; + } + + @Override + public String add(String extension, String mimeType) { + copyIfNecessary(); + return super.add(extension, mimeType); + } + + @Override + public String remove(String extension) { + copyIfNecessary(); + return super.remove(extension); + } + + private void copyIfNecessary() { + if (this.copied.compareAndSet(false, true)) { + this.source.forEach((mapping) -> add(mapping.getExtension(), mapping.getMimeType())); + } + } + + @Override + public String get(String extension) { + return !this.copied.get() ? this.source.get(extension) : super.get(extension); + } + + @Override + public Collection getAll() { + return !this.copied.get() ? this.source.getAll() : super.getAll(); + } + + @Override + Map getMap() { + return !this.copied.get() ? this.source.getMap() : super.getMap(); + } + + } + + static class MimeMappingsRuntimeHints implements RuntimeHintsRegistrar { + + @Override + public void registerHints(RuntimeHints hints, ClassLoader classLoader) { + hints.resources().registerPattern( + "org/springframework/boot/web/server/" + DefaultMimeMappings.MIME_MAPPINGS_PROPERTIES); + } + + } + } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactory.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactory.java index bc9e2d7b3e3..75d4b1f2fda 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactory.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactory.java @@ -69,7 +69,7 @@ public abstract class AbstractServletWebServerFactory extends AbstractConfigurab private boolean registerDefaultServlet = false; - private MimeMappings mimeMappings = new MimeMappings(MimeMappings.DEFAULT); + private MimeMappings mimeMappings = MimeMappings.lazyCopy(MimeMappings.DEFAULT); private List initializers = new ArrayList<>(); @@ -173,6 +173,7 @@ public abstract class AbstractServletWebServerFactory extends AbstractConfigurab @Override public void setMimeMappings(MimeMappings mimeMappings) { + Assert.notNull(mimeMappings, "MimeMappings must not be null"); this.mimeMappings = new MimeMappings(mimeMappings); } diff --git a/spring-boot-project/spring-boot/src/main/resources/META-INF/spring/aot.factories b/spring-boot-project/spring-boot/src/main/resources/META-INF/spring/aot.factories index 7f5bbaa03ef..557835873c7 100644 --- a/spring-boot-project/spring-boot/src/main/resources/META-INF/spring/aot.factories +++ b/spring-boot-project/spring-boot/src/main/resources/META-INF/spring/aot.factories @@ -1,11 +1,12 @@ org.springframework.aot.hint.RuntimeHintsRegistrar=\ +org.springframework.boot.SpringApplication.SpringApplicationRuntimeHints,\ +org.springframework.boot.WebApplicationType.WebApplicationTypeRuntimeHints,\ org.springframework.boot.context.config.ConfigDataLocationRuntimeHints,\ org.springframework.boot.env.PropertySourceRuntimeHints,\ org.springframework.boot.json.JacksonRuntimeHints,\ org.springframework.boot.logging.java.JavaLoggingSystemRuntimeHints,\ org.springframework.boot.logging.logback.LogbackRuntimeHints,\ -org.springframework.boot.SpringApplication.SpringApplicationRuntimeHints,\ -org.springframework.boot.WebApplicationType.WebApplicationTypeRuntimeHints +org.springframework.boot.web.server.MimeMappings.MimeMappingsRuntimeHints org.springframework.beans.factory.aot.BeanFactoryInitializationAotProcessor=\ org.springframework.boot.context.properties.ConfigurationPropertiesBeanFactoryInitializationAotProcessor diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/server/MimeMappingsTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/server/MimeMappingsTests.java index b28a8e5dcc7..0d84f76e80a 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/server/MimeMappingsTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/server/MimeMappingsTests.java @@ -24,6 +24,13 @@ import java.util.regex.Pattern; import org.junit.jupiter.api.Test; +import org.springframework.aot.hint.RuntimeHints; +import org.springframework.aot.hint.predicate.RuntimeHintsPredicates; +import org.springframework.boot.web.server.MimeMappings.DefaultMimeMappings; +import org.springframework.boot.web.server.MimeMappings.Mapping; +import org.springframework.boot.web.server.MimeMappings.MimeMappingsRuntimeHints; +import org.springframework.test.util.ReflectionTestUtils; + import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; @@ -31,6 +38,7 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType; * Tests for {@link MimeMappings}. * * @author Phillip Webb + * @author Guirong Hu */ class MimeMappingsTests { @@ -143,4 +151,69 @@ class MimeMappingsTests { assertThat(MimeMappings.DEFAULT).allSatisfy((mapping) -> assertThat(mapping.getMimeType()).matches(pattern)); } + @Test + void getCommonTypeOnDefaultMimeMappingsDoesNotLoadMappings() { + DefaultMimeMappings mappings = new DefaultMimeMappings(); + assertThat(mappings.get("json")).isEqualTo("application/json"); + assertThat((Object) mappings).extracting("loaded").isNull(); + } + + @Test + void getExoticTypeOnDefaultMimeMappingsLoadsMappings() { + DefaultMimeMappings mappings = new DefaultMimeMappings(); + assertThat(mappings.get("123")).isEqualTo("application/vnd.lotus-1-2-3"); + assertThat((Object) mappings).extracting("loaded").isNotNull(); + } + + @Test + void iterateOnDefaultMimeMappingsLoadsMappings() { + DefaultMimeMappings mappings = new DefaultMimeMappings(); + assertThat(mappings).isNotEmpty(); + assertThat((Object) mappings).extracting("loaded").isNotNull(); + } + + @Test + void commonMappingsAreSubsetOfAllMappings() { + MimeMappings commonMappings = (MimeMappings) ReflectionTestUtils.getField(DefaultMimeMappings.class, "COMMON"); + for (Mapping commonMapping : commonMappings) { + assertThat(MimeMappings.DEFAULT.get(commonMapping.getExtension())).isEqualTo(commonMapping.getMimeType()); + } + } + + @Test + void lazyCopyWhenNotMutatedDelegates() { + DefaultMimeMappings mappings = new DefaultMimeMappings(); + MimeMappings lazyCopy = MimeMappings.lazyCopy(mappings); + assertThat(lazyCopy.get("json")).isEqualTo("application/json"); + assertThat((Object) mappings).extracting("loaded").isNull(); + } + + @Test + void lazyCopyWhenMutatedCreatesCopy() { + DefaultMimeMappings mappings = new DefaultMimeMappings(); + MimeMappings lazyCopy = MimeMappings.lazyCopy(mappings); + lazyCopy.add("json", "other/json"); + assertThat(lazyCopy.get("json")).isEqualTo("other/json"); + assertThat((Object) mappings).extracting("loaded").isNotNull(); + } + + @Test + void lazyCopyWhenMutatedCreatesCopyOnlyOnce() { + MimeMappings mappings = new MimeMappings(); + mappings.add("json", "one/json"); + MimeMappings lazyCopy = MimeMappings.lazyCopy(mappings); + assertThat(lazyCopy.get("json")).isEqualTo("one/json"); + mappings.add("json", "two/json"); + lazyCopy.add("json", "other/json"); + assertThat(lazyCopy.get("json")).isEqualTo("other/json"); + } + + @Test + void shouldRegisterHints() { + RuntimeHints runtimeHints = new RuntimeHints(); + new MimeMappingsRuntimeHints().registerHints(runtimeHints, getClass().getClassLoader()); + assertThat(RuntimeHintsPredicates.resource() + .forResource("org/springframework/boot/web/server/mime-mappings.properties")).accepts(runtimeHints); + } + }