spring-security icon indicating copy to clipboard operation
spring-security copied to clipboard

Fix NullPointerException

Open dsbecker opened this issue 2 years ago • 6 comments

Fix NullPointerException when the remoteAddress is unresolved by using remoteAddress.getHostString() or remoteAddress.getAddress().getHostAddress() when it is. The ipAddressMatcher.matches(String) call will attempt to re-parse and resolve the address anyway.

Closes gh-11888

dsbecker avatar Sep 21 '22 16:09 dsbecker

Hi, @dsbecker. Thanks for putting this PR together. Could you please also add a unit test that confirms the code works as intended?

jzheaux avatar Sep 23 '22 19:09 jzheaux

@jzheaux I will try to get to that next week. I don’t actually have a full runnable project, I found and tested this fix by hand using class masking in my full work project where I found the problem.

dsbecker avatar Sep 23 '22 22:09 dsbecker

@jzheaux Unit tests added. Thank you for your patience!

New tests failing with existing code: Screen Shot 2022-09-26 at 11 16 00 AM

All tests passing with new code: Screen Shot 2022-09-26 at 11 16 28 AM

dsbecker avatar Sep 26 '22 18:09 dsbecker

@jzheaux I see the build of my PR is failing, but I don't understand how to find the reason why... I don't use gradle and it's not jumping out at me. Is there something broken I need to fix?

dsbecker avatar Oct 07 '22 16:10 dsbecker

Hi, @dsbecker. I think this is the error message:

* What went wrong:
Execution failed for task ':spring-security-web:checkFormatTest'.
> Formatting violations found in the following files:
   * /home/runner/work/spring-security/spring-security/web/src/test/java/org/springframework/security/web/server/util/matcher/IpAddressServerWebExchangeMatcherTests.java

...
  
  Run `format` to fix.

I don't use gradle

Gradle ships with the repository. Please run from the command line to ensure that the PR is ready:

// Linux/Mac
./gradlew format check

// Windows
gradlew.bat format check

jzheaux avatar Oct 10 '22 18:10 jzheaux

@jzheaux Sorry, that's not working for me. After adding various env opts to work around my corp proxy, having a self-signed root cert, I get the following:

% ./gradlew format check             

> Task :buildSrc:test

JavadocApiPluginITest > multiModuleApi() FAILED
    java.io.FileNotFoundException at JavadocApiPluginITest.java:33

ShowcaseITest > build() FAILED
    org.gradle.testkit.runner.UnexpectedBuildFailure at ShowcaseITest.java:30

110 tests completed, 2 failed, 3 skipped

> Task :buildSrc:test FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':buildSrc:test'.
> There were failing tests. See the report at: file:///Users/dbecker/Documents/intellij/spring-security/buildSrc/build/reports/tests/test/index.html

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 9s
13 actionable tasks: 1 executed, 12 up-to-date

At least one of these seems to be caused by:

Exception in thread "main" java.lang.reflect.InvocationTargetException
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at java.instrument/sun.instrument.InstrumentationImpl.loadClassAndStartAgent(InstrumentationImpl.java:491)
	at java.instrument/sun.instrument.InstrumentationImpl.loadClassAndCallPremain(InstrumentationImpl.java:503)
Caused by: java.lang.RuntimeException: Class java/lang/UnknownError could not be instrumented.
	at org.jacoco.agent.rt.internal_28bab1d.core.runtime.ModifiedSystemClassRuntime.createFor(ModifiedSystemClassRuntime.java:140)
	at org.jacoco.agent.rt.internal_28bab1d.core.runtime.ModifiedSystemClassRuntime.createFor(ModifiedSystemClassRuntime.java:101)
	at org.jacoco.agent.rt.internal_28bab1d.PreMain.createRuntime(PreMain.java:55)
FATAL ERROR in native method: processing of -javaagent failed, processJavaStart failed
	at org.jacoco.agent.rt.internal_28bab1d.PreMain.premain(PreMain.java:47)
	... 6 more
Caused by: java.lang.NoSuchFieldException: $jacocoAccess
	at java.base/java.lang.Class.getField(Class.java:2117)
	at org.jacoco.agent.rt.internal_28bab1d.core.runtime.ModifiedSystemClassRuntime.createFor(ModifiedSystemClassRuntime.java:138)
	... 9 more
*** java.lang.instrument ASSERTION FAILED ***: "result" with message agent load/premain call failed at src/java.instrument/share/native/libinstrument/JPLISAgent.c line: 422

It could be this is caused by our use of jdk17. I'm sorry to put this back on you, but is there any chance you could tell me what changes need to be made? Or just make them yourself? While I do want to play nice with your process, this is proving quite difficult for such a simple bugfix.

Please advise.

dsbecker avatar Oct 10 '22 20:10 dsbecker

@jzheaux Thank you for carrying that across the finish line for me. Sorry I couldn’t get it to build locally.

dsbecker avatar Oct 24 '22 23:10 dsbecker