Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ResourceHandlers cannot resolve static resources without wildcard patterns #29739

Closed
mufasa1976 opened this issue Dec 25, 2022 · 3 comments
Closed
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@mufasa1976
Copy link

The following Code-Snippet works correctly with Spring MVC:

@Configuration
@EnableWebMvc
public class WebMvcConfiguration implements WebMvcConfigurer {
  private static final String[] ANGULAR_RESOURCES = {
      "/favicon.ico",
      "/main.*.js",
      "/polyfills.*.js",
      "/runtime.*.js",
      "/styles.*.css",
      "/deeppurple-amber.css",
      "/indigo-pink.css",
      "/pink-bluegrey.css",
      "/purple-green.css",
      "/3rdpartylicenses.txt"
  };
  private static final List<Locale> SUPPORTED_LANGUAGES = List.of(Locale.GERMAN, Locale.ENGLISH);
  private static final Locale DEFAULT_LOCALE = Locale.ENGLISH;

  private final String prefix;
  private final Collection<HttpMessageConverter<?>> messageConverters;
  private final AsyncTaskExecutor asyncTaskExecutor;

  public WebMvcConfiguration(
      @Value("${spring.thymeleaf.prefix:" + ThymeleafProperties.DEFAULT_PREFIX + "}") String prefix,
      Collection<HttpMessageConverter<?>> messageConverters,
      AsyncTaskExecutor asyncTaskExecutor) {
    this.prefix = StringUtils.appendIfMissing(prefix, "/");
    this.messageConverters = messageConverters;
    this.asyncTaskExecutor = asyncTaskExecutor;
  }

  @Override
  public void addResourceHandlers(ResourceHandlerRegistry registry) {
    registry.setOrder(1);
    registry.addResourceHandler("/webjars/**")
            .addResourceLocations("classpath:/META-INF/resources/webjars/")
            .resourceChain(true);
    SUPPORTED_LANGUAGES.forEach(registerLocalizedAngularResourcesTo(registry));
  }

  private Consumer<Locale> registerLocalizedAngularResourcesTo(ResourceHandlerRegistry registry) {
    return language -> {
      final var relativeAngularResources = Stream.of(ANGULAR_RESOURCES)
                                                 .filter(resource -> StringUtils.contains(resource, "*"))
                                                 .map(resource -> "/" + language.getLanguage() + resource)
                                                 .toArray(String[]::new);
      registry.addResourceHandler(relativeAngularResources)
              .addResourceLocations(prefix + language.getLanguage() + "/");

      final var fixedAngularResources = Stream.of(ANGULAR_RESOURCES)
                                              .filter(resource -> !StringUtils.contains(resource, "*"))
                                              .map(resource -> "/" + language.getLanguage() + resource)
                                              .toArray(String[]::new);
      registry.addResourceHandler(fixedAngularResources)
              .addResourceLocations(prefix);

      registry.addResourceHandler("/" + language.getLanguage() + "/assets/**")
              .addResourceLocations(prefix + language.getLanguage() + "/assets/");
    };
  }

  @Override
  public void addViewControllers(ViewControllerRegistry registry) {
    registry.setOrder(2);
    SUPPORTED_LANGUAGES.forEach(language -> registry.addViewController("/" + language.getLanguage() + "/**").setViewName(language.getLanguage() + "/index"));
  }

  @Bean
  public RouterFunction<ServerResponse> routerFunction() {
    return route(GET("/"), this::defaultLandingPage);
  }

  private ServerResponse defaultLandingPage(ServerRequest request) {
    final var locale = Optional.ofNullable(Locale.lookup(request.headers().acceptLanguage(), SUPPORTED_LANGUAGES))
                               .orElse(DEFAULT_LOCALE);
    return ServerResponse.status(HttpStatus.TEMPORARY_REDIRECT).render("redirect:/" + locale.getLanguage());
  }

  @Override
  public void configureContentNegotiation(ContentNegotiationConfigurer configurer) {
    configurer.defaultContentType(APPLICATION_JSON);
  }

  @Override
  public void configureMessageConverters(List<HttpMessageConverter<?>> converters) {
    converters.addAll(messageConverters);
  }

  @Override
  public void configureAsyncSupport(AsyncSupportConfigurer configurer) {
    configurer.setDefaultTimeout(5000).setTaskExecutor(asyncTaskExecutor);
  }
}

I convert the Application to use Spring Web-Flux and rewrite the Configuration:

@Configuration
@EnableWebFlux
public class WebFluxConfiguration implements WebFluxConfigurer {
  private static final String[] ANGULAR_RESOURCES = {
      "/favicon.ico",
      "/main.*.js",
      "/polyfills.*.js",
      "/runtime.*.js",
      "/styles.*.css",
      "/deeppurple-amber.css",
      "/indigo-pink.css",
      "/pink-bluegrey.css",
      "/purple-green.css",
      "/3rdpartylicenses.txt"
  };
  private static final List<Locale> SUPPORTED_LANGUAGES = List.of(Locale.GERMAN, Locale.ENGLISH);
  private static final Locale DEFAULT_LANGUAGE = Locale.ENGLISH;

  private final String prefix;
  private final ThymeleafReactiveViewResolver thymeleafReactiveViewResolver;

  public WebFluxConfiguration(@Value("${spring.thymeleaf.prefix:" + ThymeleafProperties.DEFAULT_PREFIX + "}") String prefix, ThymeleafReactiveViewResolver thymeleafReactiveViewResolver) {
    this.prefix = prefix;
    this.thymeleafReactiveViewResolver = thymeleafReactiveViewResolver;
  }

  @Override
  public void addResourceHandlers(ResourceHandlerRegistry registry) {
    registry.setOrder(1);
    registry.addResourceHandler("/webjars/**")
            .addResourceLocations("classpath:/META-INF/resources/webjars/")
            .resourceChain(true);
    SUPPORTED_LANGUAGES.forEach(addLocalizedAngularResourcesTo(registry));
  }

  private Consumer<Locale> addLocalizedAngularResourcesTo(ResourceHandlerRegistry registry) {
    return language -> {
      final var relativeAngularResources = Stream.of(ANGULAR_RESOURCES)
                                                 .filter(resource -> StringUtils.contains(resource, "*"))
                                                 .map(resource -> "/" + language.getLanguage() + resource)
                                                 .toArray(String[]::new);
      registry.addResourceHandler(relativeAngularResources)
              .addResourceLocations(prefix + language.getLanguage() + "/");

      final var fixedAngularResources = Stream.of(ANGULAR_RESOURCES)
                                              .filter(resource -> !StringUtils.contains(resource, "*"))
                                              .map(resource -> "/" + language.getLanguage() + resource)
                                              .toArray(String[]::new);
      registry.addResourceHandler(fixedAngularResources)
              .addResourceLocations(prefix);

      registry.addResourceHandler("/" + language.getLanguage() + "/assets/**")
              .addResourceLocations(prefix + language.getLanguage() + "/assets/");
    };
  }

  @Override
  public void configureViewResolvers(ViewResolverRegistry registry) {
    registry.viewResolver(thymeleafReactiveViewResolver);
  }

  @Bean
  public RouterFunction<ServerResponse> routerFunction() {
    final var routerFunctionBuilder = route().GET("/", this::defaultLandingPage);
    SUPPORTED_LANGUAGES.forEach(addLocalizedLandingPageTo(routerFunctionBuilder));
    return routerFunctionBuilder.build();
  }

  private Mono<ServerResponse> defaultLandingPage(ServerRequest request) {
    final var locale = Optional.ofNullable(Locale.lookup(request.headers().acceptLanguage(), SUPPORTED_LANGUAGES))
                               .orElse(DEFAULT_LANGUAGE);
    return ServerResponse.temporaryRedirect(request.uriBuilder().path(locale.getLanguage()).build()).build();
  }

  private Consumer<Locale> addLocalizedLandingPageTo(RouterFunctions.Builder routerFunctionBuilder) {
    return language -> {
      var requestPredicate = Stream.of(ANGULAR_RESOURCES)
                                   .map(angularResource -> "/" + language.getLanguage() + angularResource)
                                   .reduce(GET("/" + language.getLanguage() + "/**"), (requestPredicte, route) -> requestPredicte.and(GET(route).negate()), RequestPredicate::and);
      requestPredicate = requestPredicate.and(GET("/" + language.getLanguage() + "/assets/**").negate());
      routerFunctionBuilder.route(requestPredicate, request -> ServerResponse.ok().contentType(MediaType.TEXT_HTML).render(language.getLanguage() + "/index"));
    };
  }
}

The rewritten Application is available via Github

When trying to access the Resource /en/favicon.ico I will receive a HTTP 404 Error while accessing the Resource /en/main.6187e65880c98290.js will give me a HTTP 200 with the correct Content.

  @Test
  @DisplayName("GET /en/main.6187e65880c98290.js and expect the Content of en/main.6187e65880c98290.js")
  void relativeResource() {
    web.get()
       .uri("/en/main.6187e65880c98290.js")
       .exchange()
       .expectStatus().isOk()
       .expectBody().consumeWith(response -> assertThat(response.getResponseBody())
           .asString()
           .isNotEmpty()
           .isEqualTo(getContent("/en/main.6187e65880c98290.js")));
  }

  private String getContent(String location) {
    final var resource = resourceLoader.getResource(String.join("/", thymeleafPrefix, StringUtils.stripStart(location, "/")));
    if (!resource.exists() || !resource.isFile() || !resource.isReadable()) {
      return null;
    }
    try (final Reader reader = new InputStreamReader(resource.getInputStream(), UTF_8)) {
      return FileCopyUtils.copyToString(reader);
    } catch (IOException e) {
      log.error("Error while reading Test-Resource {}", location, e);
      return null;
    }
  }

  @Test
  @DisplayName("GET /en/favicon.ico and expect the Content of en/favicon.ico")
  void staticResource() {
    web.get()
       .uri("/en/favicon.ico")
       .exchange()
       .expectStatus().isOk()
       .expectBody().consumeWith(response -> assertThat(response.getResponseBody())
           .asString()
           .isNotEmpty()
           .isEqualTo(getContent("/en/favicon.ico")));
  }
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 25, 2022
@drgnchan
Copy link
Contributor

drgnchan commented Dec 26, 2022

image

As the pic shows,the reason is that the attribute of org.springframework.web.reactive.HandlerMapping.pathWithinHandlerMapping is blank.I think there should be checking another attribute org.springframework.web.reactive.HandlerMapping.bestMatchingPattern or org.springframework.web.reactive.HandlerMapping.bestMatchingHandler is existing.

@sbrannen sbrannen added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Jan 4, 2023
@sbrannen sbrannen added this to the Triage Queue milestone Jan 4, 2023
@rstoyanchev rstoyanchev removed this from the Triage Queue milestone Jan 20, 2023
@bclozel bclozel self-assigned this Jan 20, 2023
@bclozel bclozel added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 20, 2023
@bclozel bclozel added this to the 6.0.5 milestone Jan 20, 2023
@bclozel
Copy link
Member

bclozel commented Jan 20, 2023

This case can be reduced to a favicon file located in src/main/resources/static/en/favicon.ico and the following configuration:

@Configuration
public class WebConfiguration implements WebFluxConfigurer {

	@Override
	public void addResourceHandlers(ResourceHandlerRegistry registry) {
		registry.addResourceHandler("/en/favicon.ico").addResourceLocations("classpath:/static/");
	}
}

The ResourceWebHandler is only using the path within the handler mapping to resolve the resource in the configured locations. In this case, the pattern has no dynamic part (no *, no regexp elements) and will always have an empty path within handler mapping value.

This is not a problem on the MVC side, where the HTTP handling itself is slightly different and can use the full path.

We're going to handle this as an enhancement, as backporting this change to 5.3.x has not been a problem so far and we want to avoid serving static resources that were ignored in the past.

@mufasa1976 Note that your resource handling setup is quite inefficient, as it's registering many handlers for few resources; this might cause performance inefficiencies when handling HTTP requests. I'm not familiar with your constraints, but I would try in general to use a front-end build tool to pack and clean those resources, only leaving the ones you're expecting to be public.

You could then try and reduce registrations just for supported languages:

@Configuration
public class WebConfiguration implements WebFluxConfigurer {

	@Override
	public void addResourceHandlers(ResourceHandlerRegistry registry) {
		registry.addResourceHandler("/en/**").addResourceLocations("classpath:/static/en/");
		registry.addResourceHandler("/de/**").addResourceLocations("classpath:/static/de/");
	}
}

@mufasa1976
Copy link
Author

@bclozel: you are completely right. I've copied the Block from the WebMvcConfiguration and reworked it to work as WebFluxConfiguration. But in this case it is best to reduce to

SUPPORTED_LANGUAGE.forEach(locale -> registry.addResourceHandler("/" + locale.getLanguage() + "/**")
                                             .addResourceLocations(prefix + locale.getLanguage() + "/"));

Thank you for your Tip!

mdeinum pushed a commit to mdeinum/spring-framework that referenced this issue Jun 29, 2023
Prior to this commit, the reactive `ResourceWebHandler` would only look
at the path within the current mapping when resolving static resources
to be served. This means that when registering a handler at
`"/resources/**"` with a `"classpath:/static/"` location, the handler
would process a `"GET /resources/file.txt"` as the `"/static/file.txt"`
classpath location.

When a developer registers a fixed pattern like `"/resources/file.txt"`
with the same location, the path within the handler mapping is empty as
there is no dynamic part in the given pattern. While the typical use
case for this feature is to register multiple resources at once with a
pattern, we should support a single registration like this.

This commit ensures that if the matching `PathPattern` for the current
request does not have a pattern syntax (i.e. no regexp, no wildcard), we
can use it to match the resource directly. Otherwise, we can use the
path within the handler mapping to resolve the resource as before.

Closes spring-projectsgh-29739
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

6 participants