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

Add support for empty path prefixes #3201

Open
NadChel opened this issue Dec 29, 2023 · 5 comments
Open

Add support for empty path prefixes #3201

NadChel opened this issue Dec 29, 2023 · 5 comments

Comments

@NadChel
Copy link
Contributor

NadChel commented Dec 29, 2023

If I set path prefixes dynamically using PrefixPathGatewayFilterFactory, and one of them happens to be an empty string, I get an exception

Caused by: java.lang.IllegalArgumentException: 'uriTemplate' must not be null

It's because its apply() method calls the UriTemplate constructor

	@Override
	public GatewayFilter apply(Config config) {
		return new GatewayFilter() {
			final UriTemplate uriTemplate = new UriTemplate(config.prefix);

which in turn checks the prefix for emptiness (not just nullness, as the exception message suggests)

	public UriTemplate(String uriTemplate) {
		Assert.hasText(uriTemplate, "'uriTemplate' must not be null");

It's super-easy to fix: simply replace

Assert.hasText(uriTemplate, "'uriTemplate' must not be null");

with

Assert.notNull(uriTemplate, "'uriTemplate' must not be null");

I want to point out that RewritePathGatewayFilterFactory does support empty replacements, for example

My Spring Cloud version is 2023.0.0, Spring Boot is at 3.2.1

@kimmking
Copy link

use / instead of empty path?

@spencergibb
Copy link
Member

PRs welcome

@bsgrd
Copy link

bsgrd commented Mar 13, 2024

@spencergibb
I created my first PR here:
spring-projects/spring-framework#32432

@jhoeller
Copy link

@spencergibb we consider addressing this in UriTemplate itself for tomorrow's 6.1.5 release, and possibly also for a backport to the 6.0.18 and 5.3.33 releases.

@spencergibb
Copy link
Member

Thanks @jhoeller !

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.

5 participants