400 error throwing FileNotFoundException

[nynymike]

We recently had a complaint from a client about a 400 response from the token endpoint that was causing AppAuth to throw a FileNotFoundException.

com.android.okhttp.internal.huc.HttpURLConnectionImpl.getInputStream(HttpURLConnectionImpl.java:238)  at 
com.android.okhttp.internal.huc.DelegatingHttpsURLConnection.getInputStream(DelegatingHttpsURLConnection.java:210)

It looks like it came from this library:
https://android.googlesource.com/platform/external/okhttp/+/781c9c2/okhttp-urlconnection/src/main/java/com/squareup/okhttp/internal/huc/HttpURLConnectionImpl.java

@Override public final InputStream getInputStream() throws IOException {
if (getResponseCode() >= HTTP_BAD_REQUEST) {
      throw new FileNotFoundException(url.toString());
    }

So instead of returning the 400, and other details about the message, it just throws a very confusing FileNotFoundException... which you would think would be a 404 if it really couldn't find a resource.

It makes me wonder about the quality of the underlying HTTP library. Is okhttp really good enough? Wouldn't you want to use at least goodhttp or even greathttp?

[iainmcgin]

Terrible jokes about the HTTP library aside (:smile:), AppAuth actually uses the HttpURLConnection API provided by the Android OS, which in the context of the device / app is being implemented by okhttp. The semantics of HttpURLConnection are such that any response code >= 400 results in a FileNotFoundException being thrown if you ask for the input stream (rather than error stream) of the connection. See the implementation from okhttp2 (embedded in Android itself) here:

https://android.googlesource.com/platform/external/okhttp/+/781c9c2/okhttp-urlconnection/src/main/java/com/squareup/okhttp/internal/huc/HttpURLConnectionImpl.java#238

It was a bug in AppAuth that we were not checking the status code to determine which stream to use, and was recently fixed in #144. This fix will be included in the 0.5.0 release, which I hope to put out today.

[iainmcgin]

Closing due to lack of activity.