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

HeaderRoutePredicateFactory fails to handle multiple header values correctly #3446

Closed
ohprettyhak opened this issue Jul 1, 2024 · 2 comments

Comments

@ohprettyhak
Copy link
Contributor

Describe the bug
In the public Predicate<ServerWebExchange> org.springframework.cloud.gateway.handler.predicate.HeaderRoutePredicateFactory.test(Config) method, the anonymous GatewayPredicate class should use List<String> HttpHeaders.getValuesAsList(String) instead of List<String> HttpHeaders.getOrDefault(String, List<String>).

The current implementation using List<String> HttpHeaders.getOrDefault(String, List<String>) provides multiple values as a single String connected by ','. To find "a match among multiple values" using a regexp, one would need to provide an input like ^(.*,\s?)*exact_match(,\s.*)$. If parsing headers containing spaces or ',' is required, it becomes even more complex as they need to be wrapped in quotation marks.

As explained in the annotation of the List<String> HttpHeaders.getValuesAsList(String) method, RFC 9110, section 5.5 defines ',' as a delimiter between members.

The current implementation is valid for 'singleton fields' but not for 'list-based fields'. Since the results may vary depending on the request client or the proxy used, we propose this change.

This issue affects Spring Cloud Gateway, current version.

Sample
Here's a sample application that reproduces the problem:

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.cloud.gateway.route.RouteLocator;
import org.springframework.cloud.gateway.route.builder.RouteLocatorBuilder;
import org.springframework.context.annotation.Bean;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RestController;
import reactor.core.publisher.Mono;

@SpringBootApplication
public class GatewayApplication {

    public static void main(String[] args) {
        SpringApplication.run(GatewayApplication.class, args);
    }

    @Bean
    public RouteLocator customRouteLocator(RouteLocatorBuilder builder) {
        return builder.routes()
            .route("test-header-route", r -> r
                .path("/test")
                .and()
                .header("X-Test-Header", "^test-value$")
                .uri("forward:/endpoint"))
            .route("catch-all", r -> r
                .path("/**")
                .uri("forward:/fallback"))
            .build();
    }

    @RestController
    public class TestController {

        @GetMapping("/endpoint")
        public Mono<String> endpoint() {
            return Mono.just("Route matched");
        }

        @GetMapping("/fallback")
        public Mono<String> fallback() {
            return Mono.just("Route not matched, fallback triggered");
        }
    }
}

Test calls:

# This works as expected
curl http://localhost:8080/test -H "X-Test-Header: test-value"
# Output: Route matched

# This fails to match the intended route and triggers the fallback
curl http://localhost:8080/test -H "X-Test-Header: test-value, another-value"
# Output: Route not matched, fallback triggered

# This works as expected
curl http://localhost:8080/test -H "X-Test-Header: test-value" -H "X-Test-Header: another-value"
# Output: Route matched

The second request should match the "test-header-route" and return "Route matched", just like the first and third requests. However, it fails to match the intended route due to the current implementation of HeaderRoutePredicateFactory, causing the request to be handled by the catch-all route.

@freeyob
Copy link

freeyob commented Jul 21, 2024

well done

@spencergibb
Copy link
Member

Closing in favor of #3447

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants