Skip to content

Commit

Permalink
This ensures no illegal cookies are send to okhttp
Browse files Browse the repository at this point in the history
When a website in a ReactNative WebView sets a cookie with an illegal
character, this cookie will automatically be added to any request to the
same domain.

This happens through:
BridgeInterceptor.java (l.84)
ReactCookieJarContainer.java (l.44)
JavaNetCookieJar.java (l.59)
ForwardingCookieHandler.java (l.57)
ForwardingCookieHandler.java (l.168)
CookieManager.java (l.39)

The BridgeInterceptor.java then tries to set a Cookie header, which
validates both keys and values, and then crashes.
okhttp3.6.0 Headers.java (l.320)

I would have expected that the IllegalArgumentException would have been
caught by JavaMethodRapper.java (l.376), because that's where the
NetworkingModule is triggered to build the okhttp request, but my guess
is that this is not caught, because IllegalArgumentException is not an
IOException. Interceptor can only throw IOExceptions.
  • Loading branch information
Erik Poort committed Jan 11, 2018
1 parent 368f3b4 commit d0ea19f
Showing 1 changed file with 13 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
package com.facebook.react.modules.network;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

import javax.annotation.Nullable;

import okhttp3.Cookie;
import okhttp3.CookieJar;
import okhttp3.Headers;
import okhttp3.HttpUrl;

/**
Expand Down Expand Up @@ -37,7 +39,17 @@ public void saveFromResponse(HttpUrl url, List<Cookie> cookies) {
@Override
public List<Cookie> loadForRequest(HttpUrl url) {
if (cookieJar != null) {
return cookieJar.loadForRequest(url);
List<Cookie> cookies = cookieJar.loadForRequest(url);
ArrayList<Cookie> validatedCookies = new ArrayList<>();
for (Cookie cookie : cookies) {
try {
Headers.Builder cookieChecker = new Headers.Builder();
cookieChecker.add(cookie.name(), cookie.value());
validatedCookies.add(cookie);
} catch (IllegalArgumentException ignored) {
}
}
return validatedCookies;
}
return Collections.emptyList();
}
Expand Down

1 comment on commit d0ea19f

@erikpoort
Copy link

Choose a reason for hiding this comment

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

Catching the IllegalArgumentException in okhttp and transforming it to an IOException doesn't work either. The issue is that JavaMethodRapper / NetworkingModule only adds a call to a queue.
Sometime later the call is triggered asynchronous by okhttp, and only exceptions in the response are being forwarded.
See: square/okhttp#3774

Please sign in to comment.