Spring-cloud-netflix: NullPointerException thrown if illegal characters are used in URI

[russTbaker]

If illegal characters are used in the @FeignClient("<service_name>") annotation, then the org.springframework.cloud.netflix.feign.ribbon.LoadBalancerFeignClient will throw a NullPointerException.
The execute method uses the following code to create the URI which is then passed down to the cleanUri method:

    URI asUri = URI.create(request.url());
    String clientName = asUri.getHost();
    URI uriWithoutHost = cleanUrl(request.url(), clientName);

static URI cleanUrl(String originalUrl, String host) {
    return URI.create(originalUrl.replaceFirst(host, ""));
}

if the URI has an illegal character, then the "host" is null, which results in a NullPointerException.

If there was a check using a regular expression prior to creating the URI, then the user can be notified that the URL that is being passed in is invalid instead of throwing an NPE.

[ryanjbaxter]

I assume when you say "illegal characters" you mean what is illegal in a URI? When I include ^ in my @FeignClient name, for example @FeignClient(name="f^o"), I get an exception when registering the FeignClient and my application fails to start. Are you doing something different?

java.lang.IllegalStateException: Service id not legal hostname (f^o)

[russTbaker]

Illegal according to the URI specification. I found this bug when I put in an underscore in my service name, which resulted in a null value for the host. So I didn’t want to narrow it down to just one character, rather to have all characters that might result in the host being null.

Hope that helps.

Russ Baker

On Aug 16, 2016, at 14:35, Ryan Baxter [email protected] wrote:

I assume when you say "illegal characters" you mean what is illegal in a URI? When I include ^ in my @FeignClient name, for example @FeignClient(name="f^o"), I get an exception when registering the FeignClient and my application fails to start. Are you doing something different?

java.lang.IllegalStateException: Service id not legal hostname (f^o)

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub https://github.com/spring-cloud/spring-cloud-netflix/issues/1257#issuecomment-240229836, or mute the thread https://github.com/notifications/unsubscribe-auth/ABnfrTLZAF6EFYNOsdxQ7H18sXRmFwAgks5qgh8lgaJpZM4JjmCa.

[ryanjbaxter]

I got the same error using an underscore, maybe you can provide a simple app that demonstrates what you are doing?

[russTbaker]

It sounds like you reproduced the error. What I did to fix it was to rename my service that I was calling via the FeignClient to something that did not have an underscore in it, and the host value came back with a real value instead of null.

If you like I can create two services and have one call the other with FeignClient. You’ll obviously have to have a Eureka service running and then configure each of the apps to use Eureka.

On Aug 16, 2016, at 14:47, Ryan Baxter [email protected] wrote:

I got the same error using an underscore, maybe you can provide a simple app that demonstrates what you are doing?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub https://github.com/spring-cloud/spring-cloud-netflix/issues/1257#issuecomment-240233429, or mute the thread https://github.com/notifications/unsubscribe-auth/ABnfrZRflBOT9zGrV7-ENrhew9LySlWlks5qgiHwgaJpZM4JjmCa.

[ryanjbaxter]

@russTbaker doesnt sound like I reproduced it, the app never even starts for me, and I don't see an NPE being thrown

[russTbaker]

I’ll go ahead and create a sample app to repro then submit it. I’ll be able to do that tomorrow night.

On Aug 16, 2016, at 17:32, Ryan Baxter [email protected] wrote:

@russTbaker https://github.com/russTbaker doesnt sound like I reproduced it, the app never even starts for me, and I don't see an NPE being thrown

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub https://github.com/spring-cloud/spring-cloud-netflix/issues/1257#issuecomment-240271249, or mute the thread https://github.com/notifications/unsubscribe-auth/ABnfrXVWZ9xBnsjruTHXJ4RjdTANikA8ks5qgkiogaJpZM4JjmCa.

[russTbaker]

@ryanbaxter I was able to repro with this app. Let me know if you have any questions.

On Aug 16, 2016, at 17:32, Ryan Baxter [email protected] wrote:

@russTbaker https://github.com/russTbaker doesnt sound like I reproduced it, the app never even starts for me, and I don't see an NPE being thrown

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub https://github.com/spring-cloud/spring-cloud-netflix/issues/1257#issuecomment-240271249, or mute the thread https://github.com/notifications/unsubscribe-auth/ABnfrXVWZ9xBnsjruTHXJ4RjdTANikA8ks5qgkiogaJpZM4JjmCa.

[spencergibb]

@russTbaker I think you missed "this app"

[russTbaker]

It was an attachment to the email. If that didn't come through, is there another way to get it to you?

On Aug 18, 2016, at 12:21 PM, Spencer Gibb [email protected] wrote:

@russTbaker I think you missed "this app"

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[spencergibb]

You can attach zip files to this issue, put a link to a github project or public dropbox.

[russTbaker]

When I get home later tonight I'll attach the file to the issue. Sorry for the delay.

On Aug 18, 2016, at 1:04 PM, Spencer Gibb [email protected] wrote:

You can attach zip files to this issue, put a link to a github project or public dropbox.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[russTbaker]

Here is the app. Let me know if you have any questions.
feign-bug.tar.gz

[spencergibb]

Why do you have @FeignClient("http://underscore_server")?

Changing it to @FeignClient("underscore_server") it gets properly validated. The problem is that it tries to create a URL object of http://http://underscore_server which returns a valid URI with a host of http. so FeignClientsRegistrar.getName() should only prepend http:// if it doesn't already have it.

[ryanjbaxter]

@spencergibb would doing @FeignClient(url="http://underscore_server") solve the issue?

[russTbaker]

That makes sense because previously when I passed in just the service name with the underscore it said that no valid host found.

[spencergibb]

@ryanjbaxter no, using url skips discovery.

[ryanjbaxter]

True, sorry i was confused about what @russTbaker was doing

[russTbaker]

I'm curious though, when I do use correct service name with http:// pre-appended to it, it works fine. Shouldn't it fail?

On Aug 19, 2016, at 2:16 PM, Spencer Gibb [email protected] wrote:

@ryanjbaxter no, using url skips discovery.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[spencergibb]

I should. That's why I relabled it a bug.