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

Fixed NullPointerException Bug #2304

Merged
merged 1 commit into from
Jan 24, 2024
Merged

Fixed NullPointerException Bug #2304

merged 1 commit into from
Jan 24, 2024

Conversation

pingpingy1
Copy link
Contributor

RequestTemplate.header(name, values) method has two overloaded implementations.
In the case where values has type Iterable<String>, values is guarded against null.
This does not happen when values has type String..., which is fixed by this commit.

Concrete testcase:

import feign.RequestTemplate;

public class Test{
	public static void main(String args[]) throws Exception {
		RequestTemplate rt = new RequestTemplate();
		String[] values = null;
		String name = "";
		rt.header(name, values); // NPE
	}
}

Result:

Exception in thread "main" java.lang.NullPointerException
        at java.base/java.util.Objects.requireNonNull(Objects.java:233)
        at java.base/java.util.Arrays$ArrayList.<init>(Arrays.java:4238)
        at java.base/java.util.Arrays.asList(Arrays.java:4223)
        at feign.RequestTemplate.header(RequestTemplate.java:706)
        at Test.main(Test.java:8)

Copy link
Member

@velo velo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, appreciated the effort you put into fixing this problem.

Seems your change includes some git syntax instructions, please fix that.

Also, It looks like nothing what changed on JacksonCodecTest, would you please revert the formatting changes?

Last, but also a blocked, please create a new test case inside RequestTemplateTest.java that reproduced the issue you found.

Thanks for the assistance

@pingpingy1
Copy link
Contributor Author

@velo Thank you for the kind instructions.
I apologize for not fully obliging by the guidelines; this is my first public PR, and I was quite nervous.
Will promptly get on with it.

`RequestTemplate.header(name, values)` method has two overloaded implementations.
In the case where `values` has type `Iterable<String>`, `values` is guarded against `null`.
This does not happen when `values` has type `String...`, which is fixed by this commit.
@velo
Copy link
Member

velo commented Jan 24, 2024

this is my first public PR, and I was quite nervous

All good, you probably did far better than I did on my first one

@velo velo merged commit 32eacdc into OpenFeign:master Jan 24, 2024
3 checks passed
@velo
Copy link
Member

velo commented Jan 24, 2024

I will be releasing feign next Friday, this will be included

@pingpingy1
Copy link
Contributor Author

Thank you for the encouraging messages sir!!

@velo
Copy link
Member

velo commented Jan 24, 2024

Hey, I'm not going to get more help by being nasty

Look forward your next PR

velo pushed a commit that referenced this pull request Oct 7, 2024
`RequestTemplate.header(name, values)` method has two overloaded implementations.
In the case where `values` has type `Iterable<String>`, `values` is guarded against `null`.
This does not happen when `values` has type `String...`, which is fixed by this commit.
velo pushed a commit that referenced this pull request Oct 8, 2024
`RequestTemplate.header(name, values)` method has two overloaded implementations.
In the case where `values` has type `Iterable<String>`, `values` is guarded against `null`.
This does not happen when `values` has type `String...`, which is fixed by this commit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants