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.

Sign up for GitHub

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

Jump to bottom

Fix NullPointerException with empty X-Forwarded-For header #16046

Closed
wants to merge 1 commit into from
Closed

Fix NullPointerException with empty X-Forwarded-For header #16046

wants to merge 1 commit into from

Conversation

nosan
Copy link
Contributor

@nosan nosan commented Feb 27, 2019

gh-16044

@snicoll
I have not found a way how to test this, if you have an idea how to test, please let me know.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 27, 2019
wilkinsona
wilkinsona requested changes Feb 27, 2019
View reviewed changes
...a/org/springframework/boot/actuate/web/trace/reactive/ServerWebExchangeTraceableRequest.java Outdated
@@ -45,8 +48,9 @@
this.method = request.getMethodValue();
this.headers = request.getHeaders();
this.uri = request.getURI();
this.remoteAddress = (request.getRemoteAddress() != null)
? request.getRemoteAddress().getAddress().toString() : null;
this.remoteAddress = Optional.ofNullable(request.getRemoteAddress())
Copy link
Member

Choose a reason for hiding this comment

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

We prefer not to use Optional for internal null checks such as this. If the readability of the ternary is a problem it could be pulled out into a method that's something like this instead:

private static String asString(InetSocketAddress socketAddress) {
	return (socketAddress != null && socketAddress.getAddress() != null)
			? socketAddress.getAddress().toString() : null;
}

@wilkinsona
Copy link
Member

It should be possible to test it using an InetSocketAddress created with an unresolvable host.

@wilkinsona wilkinsona added the status: waiting-for-feedback We need additional information before we can continue label Feb 27, 2019
@nosan
Copy link
Contributor Author

nosan commented Feb 27, 2019

@wilkinsona Thank you so much!
I have updated this PR.

@wilkinsona
Copy link
Member

Thanks for the updates, particularly the two new tests. Did you consider adding a new test class for ServerWebExchangeTraceableRequest rather than testing things at the level of the filter? I think that might be better as it would allow the test to focus specifically on the handling of an unresolved remote address.

@nosan
Copy link
Contributor Author

nosan commented Feb 27, 2019

@wilkinsona sure, I need some time. I let you know.

@nosan nosan force-pushed the gh-16044 branch 2 times, most recently from 8b7167b to d1e2df7 Compare February 27, 2019 13:42
@nosan
Copy link
Contributor Author

nosan commented Feb 27, 2019

@wilkinsona
updated

@nosan nosan changed the title fix a NullPointerException. fixes a NullPointerException when a remote address is unresolved. Feb 28, 2019
@nosan
This commit fixes a NullPointerException when InetAddress is not reso…
e9cd313 
…lved,

and add a class for testing ServerWebExchangeTraceableRequest.

closes spring-projectsgh-16044
@snicoll snicoll self-assigned this Feb 28, 2019
@snicoll snicoll mentioned this pull request Feb 28, 2019
Closed
@snicoll snicoll added type: bug A general bug and removed status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged labels Feb 28, 2019
@snicoll snicoll added this to the 2.1.4 milestone Feb 28, 2019
@snicoll snicoll changed the title fixes a NullPointerException when a remote address is unresolved. Fix NullPointerException with empty X-Forwarded-For header Feb 28, 2019
snicoll pushed a commit that referenced this pull request Feb 28, 2019
@nosan @snicoll
Fix NullPointerException with empty X-Forwarded-For header
64a74c3 
See gh-16046
snicoll added a commit that referenced this pull request Feb 28, 2019
@snicoll
Merge pull request #16046 from nosan
8c18bc1 
* pr/16046:
  Polish "Fix NullPointerException with empty X-Forwarded-For header"
  Fix NullPointerException with empty X-Forwarded-For header
@snicoll snicoll closed this in c224eeb Feb 28, 2019
@snicoll
Copy link
Member

snicoll commented Feb 28, 2019

Thank you @nosan. Please review the polish commit when you get a chance. We prefer to use BDDMockito FYI.

@nosan
Copy link
Contributor Author

nosan commented Feb 28, 2019 •
edited

@snicoll I did not know about this, I will keep in mind.
Thanks

@nosan nosan deleted the gh-16044 branch February 28, 2019 14:07
snicoll pushed a commit that referenced this pull request Apr 3, 2019
@izeye @snicoll
Check for Reactor Netty disconnected client errors
9fae1e5 
Closes gh-16046
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wilkinsona wilkinsona wilkinsona requested changes

Assignees

@snicoll snicoll

Labels
type: bug A general bug
Projects
None yet
Milestone
2.1.4
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@nosan @wilkinsona @snicoll @spring-projects-issues