spring-projects / spring-framework

Spring Framework
https://spring.io/projects/spring-framework
Apache License 2.0
56.21k stars 37.97k forks source link

Provide and document a way to handle single-page application redirects #27257

Closed gschjetne closed 7 months ago

gschjetne commented 3 years ago

Affects: 5.3.8


The problem was previously discussed in #21328, but as @danbim pointed out, it's not a reliable solution as it overrides other routes in the application.

I tried to annotate the static resource bean with @Order(Ordered.LOWEST_PRECEDENCE), but that seems to only make it the lowest of the annotated beans, not lowest of all beans like it would need to be for this to work. I checked the documentation for ways to specify a custom error page instead of the white label one, and the only thing I could find was a static 404.html file, but as far as I can tell there is no way to change the status to 200.

This has been asked for countless of times on StackOverflow and similar places, and the answers range from completely wrong to completely ridiculous, like using regexes or explicit routes to catch everything that the front end might conceivably want to handle.

I'm suggesting that Spring as a back-end for SPAs is a common enough use case to warrant a reliable solution.

gschjetne commented 3 years ago

For reference, here's the best I could come up with, which if not wrong is still ridiculous, throwing loose coupling from dependency injection out the window, since you always have to remember which path prefixes to keep out:

import org.springframework.context.annotation.Bean
import org.springframework.core.io.ClassPathResource
import org.springframework.stereotype.Component
import org.springframework.web.reactive.function.server.RouterFunctions
import org.springframework.web.reactive.function.server.RouterFunctions.resources
import reactor.core.publisher.Mono

@Component
class Client {
    @Bean
    fun client() = resources { request ->
        val path = request.uri().path
        if (path.startsWith("/graphql") || path.startsWith("/auth")) {
            Mono.empty()
        } else {
            resourceLookup.apply(request)
        }
    }

    private val resourceLookup = RouterFunctions
        .resourceLookupFunction("/**", ClassPathResource("public/"))
        .andThen {
            it.switchIfEmpty(Mono.just(ClassPathResource("public/index.html")))
        }
}
goatfryed commented 1 year ago

I have a similar issue as described here, so I wanted to document my solution for everyone searching for the same issue.

Note that resourceLookupFunctions didn't solve the issue for me, because I wanted to be able to serve resources from outside the classpath in development mode via spring.web.resources.static-locations = path/to/frontend/dist

I ended up implementing a WebExceptionHandler. It works well for me so far.

@Component
@Order(-2)
public class HtmlRequestNotFoundHandler implements WebExceptionHandler {

    private final DispatcherHandler dispatcherHandler;

    private final RequestPredicate PREDICATE = RequestPredicates.accept(MediaType.TEXT_HTML);

    public HtmlRequestNotFoundHandler(DispatcherHandler dispatcherHandler) {
        this.dispatcherHandler = dispatcherHandler;
    }

    @Override
    public Mono<Void> handle(ServerWebExchange exchange, Throwable throwable) {
        if (
            isNotFoundAndShouldBeForwarded(exchange, throwable)
        ) {
            var forwardRequest = exchange.mutate().request(it -> it.path("/index.html"));
            return dispatcherHandler.handle(forwardRequest.build());
        }
        return Mono.error(throwable);
    }

    private boolean isNotFoundAndShouldBeForwarded(ServerWebExchange exchange, Throwable throwable) {
        if (throwable instanceof ResponseStatusException
            && ((ResponseStatusException) throwable).getStatusCode() == HttpStatus.NOT_FOUND
        ) {

            var serverRequest = ServerRequest.create(exchange, Collections.emptyList());
            return PREDICATE.test(serverRequest);
        }

        return false;
    }

}
bclozel commented 8 months ago

There are various solutions described out there and I agree they are all far from perfect. Most of them involve some kind of heuristics that try to describe front-end application routes in general (like @RequestMapping(value = "/**/{path:[^.]*}")) or by elimination (everything but /api/**). This can be achieved with filters, controller endpoints or functional routes - usually ordering is an issue as those route definitions match lots of URLs.

I think the main problem here is that the web framework has no way of knowing which routes are declared by the front-end application. Ideally, the back-end app should get a list of routes and patterns and forward those to the index page automatically. Without that, all other solutions will remain imperfect: false positives or maintenance issues.

As far as I know, other web frameworks from other languages have the same issue. Some front-end libraries send the Sec-Fetch-Mode: navigate header when navigating routes in the app but I don't think this would be covered by initial browser requests.

In summary, I don't think there's anything actionable right now for Spring.

sdeleuze commented 8 months ago

Given the traffic on SO for this kind of issues, I am wondering if we could at least try to provide a bit more guidance in our reference doc. I am also wondering if we could make some features already supported more discoverable.

See for example this SO answer from @dsyer, where this kind of configuration is proposed and voted massively:

public void addViewControllers(ViewControllerRegistry registry) {
    registry.addViewController("/").setViewName("forward:/index.html");
}

I think that's pretty useful for the need raised here, but:

Maybe we could:

We can maybe discuss that during our next team meeting.

dsyer commented 8 months ago

All of those suggestions would be welcome (to me and to many I expect), but none of them actually nails the central requirement of being able to configure a fallback handler for requests that do not match other mappings (expect in a limited way possibly hinted at with the "patterns are supported" comment). I know why that is an unpopular idea, but it serves a purpose, and maybe we need to solve that problem as well? One suggestion I have that would reduce the unpopularity is to provide a fallback for only requests that can be identified in some way as browser-based and expecting HTML (content negotiation, agent identification, or something).

sdeleuze commented 8 months ago

I can see the initial appeal of the fallback approach, but in practice you will still have to define patterns, so not sure we would introduce such dedicated mechanism + forwarding transparently is pretty specific and hard to translate in term of programming model.

As pointed out by @bclozel, the right solution for that would probably be the frontend framework sharing client-side routes with the backend, but that's hard to provide as a builtin feature, would create a lot of coupling. And the headers set by the frontend framework will indeed not cover the initial request, which is IMO a blocker.

After a team related discussion, we are going to introduce a dedicated documentation section that will provide related guidance for both Spring MVC and WebFlux, leveraging the fact that function router gives a great deal of flexibility to deal with that kind of concern. We will also list alternatives like view controllers (Spring MVC only) and filters.

Let see how we much we can help fullstack developers with proper guidance on what we have today, and see how it goes.

sdeleuze commented 8 months ago

While working on the documentation, I found we probably miss a feature in the functional router API to be able to support the most common use case, so I may turn this documentation issue to an enhancement one (with related documentation).

Functional router seems the sweet spot to support such feature on both Spring MVC and Spring WebFlux in a consistent way. But I think we miss a related API since in RouterFunctions.Builder#resources(java.lang.String, org.springframework.core.io.Resource) the provided resource represent a directory where files to serve can be found, not the file to serve. You can see a great example of twisted usage for that use case here.

As a consequence, my proposal is to introduce a new RouterFunctions.Builder#resource(java.lang.String, org.springframework.core.io.Resource) (singular variant without a trailing s) that would redirect the pattern parameter to the specified file resource. This feature looks useful for the use case of that issue, but also for others, and would be much easier to leverage than a custom implementation using RouterFunctions#resourceLookupFunction.

In practice, users could specify:

@Bean
RouterFunction<ServerResponse> router() {
    var index = new ClassPathResource("static/index.html");
    return RouterFunctions.route()
            .resource("*.html", index)
            .resource("admin/*.html", index)
            .build();
}

Looks cleaner, simpler and more consistent than filter or view controller variants. Custom predicates can of course be created for more advanced use cases.

Any thoughts @poutsma @bclozel @jnizet @dsyer ?

jnizet commented 8 months ago

Custom predicates can of course be created for more advanced use cases

I think using this syntax to support SPAs will almost always require the usage of custom predicates. In my experience, paths of SPA routes don't end with .html, and what distinguishes them from the other resources is that

So I think using a resource path to forward all the SPA routes to index.html won't be doable, and a custom predicate will be necessary. That's fine by me, but I just wanted to make it clear.

sdeleuze commented 8 months ago

Thanks for your feedback @jnizet, maybe we can provide something more flexible with RequestPredicate instead of String pattern which allows to negate predicates. Let me try to explore what we can do for such need.

sdeleuze commented 8 months ago

The more flexible variant with a RequestPredicate parameter would look like:

import static org.springframework.web.servlet.function.RequestPredicates.*;
// ...

@Bean
RouterFunction<ServerResponse> frontendRouter() {
    var extensions = Arrays.asList("js", "css", "ico", "png", "jpg", "gif", "eot", "woff", "woff2", "ttf", "json")
    return RouterFunctions.route()
        .resource(path("/api/**").negate().and(pathExtension(extensions::contains).negate()),
            new ClassPathResource("static/index.html"))
        .build();
}

And in Kotlin, that would lead to something like:

@Bean
fun frontendRouter() = router {
    val extensions = arrayOf("js", "css", "ico", "png", "jpg", "gif", "eot", "woff", "woff2", "ttf", "json")
    resource(!path("/api/**") and !pathExtension(extensions::contains)),
        ClassPathResource("static/index.html"))
}

So we would add to RouterFunctions and related builder those 2 methods:

public static RouterFunction<ServerResponse> resource(String pattern, Resource fileLocation)
public static RouterFunction<ServerResponse> resource(RequestPredicate predicate, Resource fileLocation)

Any thoughts?

poutsma commented 7 months ago

My €0.02: I can see the reason for adding support for these use cases, and am I favor of doing so.

I am not entirely sure, however, whether this support needs to be implemented in terms of new methods on RouterFunctions and especially RouterFunctions.Builder, next to the existing resources methods. The difference between the method name resource and resources is rather small, so I think adding the former will be confusing. Moreover, users might want to customise the resulting resource mapping, which could add more overloaded versions.

Instead, can't we implement the required functionality in terms of a Function<ServerRequest, Mono<Resource>> (or Function<ServerRequest, Optional<Resource>> for Servlet)? Such a function can then be plugged into the existing RouterFunctions::resources(Function<ServerRequest, Mono<Resource>> lookupFunction) method (and corresponding builder variant), and can therefore be easily customized and composed upon.

sdeleuze commented 7 months ago

I can see how resource versus resources could potentially be confusing, but I am also concerned about the lack of discoverability, the extra verbosity and the fact that providing let say a ResourceRouterFunction won't be less confusing in term of naming for the proposed alternative.

Moreover, users might want to customise the resulting resource mapping, which could add more overloaded versions.

Could please elaborate? I am not sure to understand what you mean.

poutsma commented 7 months ago

I can see how resource versus resources could potentially be confusing, but I am also concerned about the lack of discoverability, the extra verbosity and the fact that providing let say a ResourceRouterFunction won't be less confusing in term of naming for the proposed alternative.

The problem with discoverability is that if you make everything discoverable; nothing is discoverable. If I understand correctly, the suggested change would introduce two new methods on RouterFunctions as well as RouterFunctions.Builder. If there is a need to customize headers, which there probably is (see below), then we're looking at 3 methods on each type (6 in total), which almost doubles the amount of resource-related methods on each (from 4 to 7).

So it's not really the name resource that bothers me; it's about striking the right balance between introducing this feature and not ending up with a maze of twisty little resource-methods, all alike.

Would you consider introducing only the generic, predicate based resource method, but not the string variant? @jnizet has indicated that the string-base variant would be of little use to them. That way we would end up with the following on RouterFunctions:

public static RouterFunction<ServerResponse> resource(RequestPredicate predicate, Resource fileLocation)
public static RouterFunction<ServerResponse> resource(RequestPredicate predicate, Resource fileLocation, BiConsumer<Resource, HttpHeaders> headersConsumer)

Moreover, users might want to customise the resulting resource mapping, which could add more overloaded versions.

Could please elaborate? I am not sure to understand what you mean.

For instance, uses might want to customize headers of the response served, such as Cache-Control. See #29985.

sdeleuze commented 7 months ago

Thanks for the extra details.

Would you consider introducing only the generic, predicate based resource method, but not the string variant?

Sure, that's would be perfectly fine and IMO a great tradeoff. Would you be ok if I just introduce this one?

poutsma commented 7 months ago

Sure, that's would be perfectly fine and IMO a great tradeoff. Would you be ok if I just introduce this one?

After a review, sure!

sdeleuze commented 7 months ago

@poutsma Please review https://github.com/sdeleuze/spring-framework/tree/functional-resource-redirection.

sdeleuze commented 7 months ago

@dsyer When Spring Boot 3.2.3 will be released, would be great if you could update your amazingly popular SO answer with links to the documentation of the proposed solution Spring MVC and Spring WebFlux, maybe showing an example with @Bean like:

Java

import static org.springframework.web.reactive.function.server.RequestPredicates.path;
import static org.springframework.web.reactive.function.server.RequestPredicates.pathExtension;
import static org.springframework.web.reactive.function.server.RouterFunctions.route;

// ...

@Bean
RouterFunction<ServerResponse> spaRouter() {
    ClassPathResource index = new ClassPathResource("static/index.html");
    List<String> extensions = Arrays.asList("js", "css", "ico", "png", "jpg", "gif");
    RequestPredicate spaPredicate = path("/api/**").or(path("/error")).or(pathExtension(extensions::contains)).negate();
    return route().resource(spaPredicate, index).build();;
}

Kotlin

import org.springframework.web.reactive.function.server.router

// ...

@Bean
fun spaRouter() = router {
    val index = ClassPathResource("static/index.html")
    val extensions = listOf("js", "css", "ico", "png", "jpg", "gif")
    val spaPredicate = !(path("/api/**") or path("/error") or
            pathExtension(extensions::contains))
    resource(spaPredicate, index)
}

To be clear, we recommend using that regardless of if users are using primarily annotation or functional web programming model, WebMVC or WebFlux. This is possible since functional router are now consistently evaluated before annotation-based controllers. Probably also worth to mention the Spring Boot 3.2.3+ requirement.