Avoid NPEs in JavaNetAuthenticator#9100
Conversation
DanielThomas
force-pushed
the
dannyt/proxy-authenticate-fixes
branch
from
54448bb to
415ab5c
Compare
okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/authenticator/JavaNetAuthenticator.kt
Show resolved
Hide resolved
| else -> { | ||
| val socketAddress = address() as InetSocketAddress | ||
| if (socketAddress.isUnresolved) { | ||
| dns.lookup(socketAddress.hostString).first() |
There was a problem hiding this comment.
Why is this resolving the DNS address? I don’t think we guarantee that we’ll use this same resolved address later when we connect
There was a problem hiding this comment.
Ahhh, I see from your stack trace. Hmmmmm.....
There was a problem hiding this comment.
Let’s just change this function to return InetAddress? ?
There was a problem hiding this comment.
Yeah, that’s what this should do. Authenticator.requestPasswordAuthentication is even documented as such.
addr – The InetAddress of the site requesting authorization, or null if not known.
| else -> { | ||
| val socketAddress = address() as InetSocketAddress | ||
| if (socketAddress.isUnresolved) { | ||
| dns.lookup(socketAddress.hostString).first() |
There was a problem hiding this comment.
Let’s just change this function to return InetAddress? ?
| else -> { | ||
| val socketAddress = address() as InetSocketAddress | ||
| if (socketAddress.isUnresolved) { | ||
| dns.lookup(socketAddress.hostString).first() |
There was a problem hiding this comment.
Yeah, that’s what this should do. Authenticator.requestPasswordAuthentication is even documented as such.
addr – The InetAddress of the site requesting authorization, or null if not known.
3 participants
Configuring
JavaNetAuthenticatoras theproxyAuthenticatorwhen using the default ProxySelector causes NPEs to be thrown because the socket addresses returned are unresolved:The implementation could also NPE on a 407 from an non-proxy host, so I've handled this too.