Fix NullPointerException in getPortFromGetInfo when getInfo() fails - #197
Open
munzzyy wants to merge 1 commit into
Open
Fix NullPointerException in getPortFromGetInfo when getInfo() fails#197munzzyy wants to merge 1 commit into
munzzyy wants to merge 1 commit into
Conversation
getInfo() is documented to return null on error (IOException talking to the control port), and its implementation does exactly that. But getPortFromGetInfo() called value.trim() without checking for null first, so any getInfo() failure while reading net/listeners/socks or net/listeners/httptunnel threw an uncaught NullPointerException on controlPortThread. That thread's catch block only lists IOException, ArrayIndexOutOfBoundsException and InterruptedException, so the NPE would propagate past it. Treat a null value the same way an empty value is already treated: the port is unavailable, return 0.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
getPortFromGetInfo()can throw an uncaught NullPointerException.getInfo(String)is documented as returning "the value or null on error", and it does return null whenevertorControlConnection.getInfo(key)throws an IOException talking to the control port (it catches, logs, and falls through toreturn null). ButgetPortFromGetInfo()calls it and goes straight tovalue.trim()with no null check:It is called from
controlPortThreadright afterauthenticate():and that thread only catches
IOException | ArrayIndexOutOfBoundsException | InterruptedException, not NPE. So any control-port hiccup during those two GETINFO calls (a dropped connection, a short read) makesgetInfo()return null, and the nullvalue.trim()becomes an uncaught NPE on that thread instead of the IOException path the method is clearly built to handle. An uncaught exception on any thread takes down the app process by default on Android.The fix treats null the same as the empty case: the port just isn't available, return 0.
I verified it with a small standalone Java harness that runs the exact method body with
value = null(whatgetInfo()returns on failure): before, it throws the NPE onString.trim(); after, same input returns 0, no exception. I don't have the Android SDK/NDK here, so I have not built the AAR or reproduced it on a device, this is a static read of the call chain plus a harness proof of the method logic. If you can confirm against a real control-port failure it would be worth a look.