-
-
Save franmontiel/ed12a2295566b7076161 to your computer and use it in GitHub Desktop.
Hi @theyann, thanks for your code review. Here are some answers:
when instantiating a collection, no need to specify the generic argument (List cookies = new ArrayList<>() is good enough :) ).
I originally wrote this in a Java 6 project and that is why I didn't use the diamond operator.
line 146 of the PersistentCookieStore file: targetCookies can never null, so no need to check for it
Changed in the last revision. Now an empty check is done.
line 73 and 74 doing a remove and an add seems strange to me, is it really more efficient than checking wether cookie is already in the map ?
If a previously stored cookie is received again the old one should be overwritten by the most recent cookie (in order to maintain the most recent expire date). To do that is necessary to remove and then add the new cookie to the set.
line 161 of the PersistenCookieStore file: you return a new ArrayList<>(targetCookies) ... why not return targetCookies that was just instantiated earlier and never reassigned to anything. Is there a reason for copying all the objects again? EDIT: never mind this either, still using sets instead of lists. In this case though, I believe you could just create a list for targetCookies as the values already come from a set, could you have a possibility of redundance?
Changed in the last revision to directly use a list.
Thanks for sharing, excelent work !!
I'm having a problem though, I'm getting NPE when I enable ProGuard. I followed @thintsa advice but it doesn't solve my problem.
This is part of the stack trace:
Caused by: java.lang.NullPointerException
at com.ati.Utils.PersistentCookieStore.getValidCookies(PersistentCookieStore.java:167)
at com.ati.Utils.PersistentCookieStore.get(PersistentCookieStore.java:134)
at java.net.CookieManager.get(CookieManager.java:112)
at com.squareup.okhttp.internal.http.HttpEngine.networkRequest(HttpEngine.java:719)
at com.squareup.okhttp.internal.http.HttpEngine.sendRequest(HttpEngine.java:225)
line 167 is when it calls if (currentCookie.hasExpired())
Tried adding all kinds of stuff to proguard-rules (even including -dontobfuscate) with no luck. Works great with proguard off
Any help will be much appreciated!
OK, please disregard my previous comment.
I found I was calling CookieHandler.setDefault(..) with a new PersistentCookieStore every time I made a call to my server API. Fixed problem by making sure setDefault() is called only once in the app.
Not sure how Proguard got into the middle of this, but I'm happy the issue is solved!
thank you very much , it help me so much. awesome code.
thank all very much, very useful solution. i use with OkHttp3
Thank you very much it works great.
May I suggest that you will upload this to maven and allow us to use gradle. I hate that I have external code in my project as a raw java file.
Thanks!
@lgalant Proguard obfuscates your SerializableHttpCookie objects.
Add this to your proguard config to avoid this obfuscation problem:
-keepnames class * implements java.io.Serializable
-keepclassmembers class * implements java.io.Serializable {
static final long serialVersionUID;
private static final java.io.ObjectStreamField[] serialPersistentFields;
!static !transient ;
private void writeObject(java.io.ObjectOutputStream);
private void readObject(java.io.ObjectInputStream);
java.lang.Object writeReplace();
java.lang.Object readResolve();
}
@franmontiel If Proguard is not configured correctly your PersistentCookieStore will fail in a silent and confusing manner:
HttpCookie cookie
will be null here https://gist.github.com/franmontiel/ed12a2295566b7076161#file-persistentcookiestore-java-L47 because an InvalidClassException is being catched in https://gist.github.com/franmontiel/ed12a2295566b7076161#file-serializablehttpcookie-java-L64. Further down the line this will result in a null object being added to the allCookies Map, which in turn will cause a NPE in https://gist.github.com/franmontiel/ed12a2295566b7076161#file-persistentcookiestore-java-L150 as described by @lgalant
Why the stream object in encode/decode methods does not require close?
Why do you use your own checkDomainsMatch(String, String)
and not the function of java.net.HttpCookie
HttpCookie.domainMatches(String, String)
?
@hintsa : thanks man saved me from great deal of headache
the solution you have offered does not respect the expiry of cookies, i have created a nice kotlin version of it which does
https://gist.github.com/leviyehonatan/0c53e89864a0890c2e524d87c6c70c2a
Should i have a single instance of the ClearableCookieJar? or else the PersistentCookieJar is taking care of it ? Sorry if my question sounds stupid.
@manikantagarikipati it might be possible that your are asking about the PersistentCookieJar library?
In any case you should add the CookieStore(this gist) or the CookieJar to the OkHttpClient and you should be using one single OkHttp client.
I've fixed few issues:
- added a workaround for the hasExpire bug (https://code.google.com/p/android/issues/detail?id=191981);
- added support for the session cookies;
- fixed the NPE for the null cookie during deserialization (in the case of missing Proguard rules).
Please feel free to grab the changes from my fork: https://gist.github.com/nuald/ad776c9f7f52d3f6865142bda58c6d3f
If by HttpCookie you mean java.net.HttpCookie then there is a huge mistake in this code.
The java.net.HttpCookie has a private final field called "whenCreated" that is set at consturction time and is used to calculate hasExpired().
Your code is not serializing that value hence after reloading cookies from the persistence all of them get new extended lifetime.
EDIT: I just saw the comment above by nuald and seems someone else has already detected this bug
EDIT2: I was looking for a way to implement persistent cookie store in standard java api not android. The bug that I explained refers to that of the standard java api.
@mrmaffen build fail with yours, i change to below:
-keepnames class * implements java.io.Serializable
-keepclassmembers class * implements java.io.Serializable {
static final long serialVersionUID;
private static final java.io.ObjectStreamField[] serialPersistentFields;
!static !transient ;
private void writeObject(java.io.ObjectOutputStream);
private void readObject(java.io.ObjectInputStream);
java.lang.Object writeReplace();
java.lang.Object readResolve();
}
FYI, isHttpOnly() and setHttpOnly() methods were added to 24 API version.
I found strange situation in getValidCookies() method.
storedUri may match with uri but allCookies.get(storedUri) return null value.
It leads to crash in:
HttpCookie currentCookie = it.next();
if (currentCookie.hasExpired()) {
I have no Proguard option and I set CookieStore only once..
@agent10
java.lang.NullPointerException: Attempt to invoke virtual method 'boolean java.net.HttpCookie.hasExpired()' on a null object reference
device:NEXUS 4
client code:
in application onCreate:
CookieManager manager = new CookieManager(
new PersistentCookieStore(this),
CookiePolicy.ACCEPT_ALL);
CookieHandler.setDefault(manager);
if I comment SerializableHttpCookie.java npe bug will not appear;
line 127 writeObject method out.writeBoolean(getHttpOnly());
line 144 readObject method setHttpOnly(in.readBoolean());
Hi,
While uploading a file using okHttp, facing the following issue. Pls help me to sort it out.
Mentioned the issues:
SerializableHttpCookie: java.lang.NoSuchFieldException: httpOnly
at java.lang.Class.getDeclaredField(Class.java:546)
at in.xxxx.xxxx.SerializableHttpCookie.initFieldHttpOnly(SerializableHttpCookie.java:98)
at in.xxxx.xxxx.SerializableHttpCookie.setHttpOnly(SerializableHttpCookie.java:88)
at in.xxxx.xxxx.SerializableHttpCookie.readObject(SerializableHttpCookie.java:131)
at java.lang.reflect.Method.invokeNative(Native Method)
at java.lang.reflect.Method.invoke(Method.java:525)
at java.io.ObjectInputStream.readObjectForClass(ObjectInputStream.java:1357)
at java.io.ObjectInputStream.readHierarchy(ObjectInputStream.java:1269)
at java.io.ObjectInputStream.readNewObject(ObjectInputStream.java:1858)
at java.io.ObjectInputStream.readNonPrimitiveContent(ObjectInputStream.java:787)
at java.io.ObjectInputStream.readObject(ObjectInputStream.java:2006)
at java.io.ObjectInputStream.readObject(ObjectInputStream.java:1963)
at in.xxxx.xxxx.SerializableHttpCookie.decode(SerializableHttpCookie.java:62)
at in.xxxx.xxxx.PersistentCookieStore.loadAllFromPersistence(PersistentCookieStore.java:53)
at in.xxxx.xxxx.PersistentCookieStore.(PersistentCookieStore.java:39)
System.err: java.lang.NullPointerException
System.err: at in.xxxx.xxxx.PersistentCookieStore.(PersistentCookieStore.java:37)
System.err: at in.xxxx.xxxx.RequestManager.upload(RequestManager.java:204)
@mrmaffen @torv I also get a build error with these Proguard rules, on the static transient line:
Warning: Exception while processing task java.io.IOException: proguard.ParseException: Expecting java type before ';' in line 32 of file '/Users/julian/AndroidStudioProjects/Twinkle/app/proguard-rules.pro'
It seems like both of you have this line.
Update: I think it's because the real line keeps getting filtered: try quoting it:
!static !transient <fields>;
Been trying to fix the httpOnly problem on API level 18. Thoughts?
08-11 15:36:11.243 3613-3644/me.shreyasr.chatse W/SerializableHttpCookie: java.lang.NullPointerException
at me.shreyasr.chatse.network.cookie.SerializableHttpCookie.getHttpOnly(SerializableHttpCookie.java:100)
at me.shreyasr.chatse.network.cookie.SerializableHttpCookie.writeObject(SerializableHttpCookie.java:142)
at java.lang.reflect.Method.invokeNative(Native Method)
at java.lang.reflect.Method.invoke(Method.java:525)
at java.io.ObjectOutputStream.writeHierarchy(ObjectOutputStream.java:1055)
at java.io.ObjectOutputStream.writeNewObject(ObjectOutputStream.java:1406)
at java.io.ObjectOutputStream.writeObjectInternal(ObjectOutputStream.java:1673)
at java.io.ObjectOutputStream.writeObject(ObjectOutputStream.java:1519)
at java.io.ObjectOutputStream.writeObject(ObjectOutputStream.java:1483)
at me.shreyasr.chatse.network.cookie.SerializableHttpCookie.encode(SerializableHttpCookie.java:69)
at me.shreyasr.chatse.network.cookie.PersistentCookieStore.saveToPersistence(PersistentCookieStore.java:139)
at me.shreyasr.chatse.network.cookie.PersistentCookieStore.add(PersistentCookieStore.java:132)
at java.net.CookieManager.put(CookieManager.java:188)
at com.squareup.okhttp.internal.http.HttpEngine.receiveHeaders(HttpEngine.java:1054)
at com.squareup.okhttp.internal.http.HttpEngine.readResponse(HttpEngine.java:796)
at com.squareup.okhttp.Call.getResponse(Call.java:274)
at com.squareup.okhttp.Call$ApplicationInterceptorChain.proceed(Call.java:230)
at com.squareup.okhttp.Call.getResponseWithInterceptorChain(Call.java:201)
at com.squareup.okhttp.Call.execute(Call.java:81)
at me.shreyasr.chatse.chat.service.IncomingEventService.loadRoom$app_debug(IncomingEventService.kt:65)
at me.shreyasr.chatse.chat.service.IncomingEventServiceBinder.loadRoom(IncomingEventServiceBinder.kt:27)
at me.shreyasr.chatse.chat.ChatActivity$rejoinFavoriteRooms$1.invoke(ChatActivity.kt:481)
at me.shreyasr.chatse.chat.ChatActivity$rejoinFavoriteRooms$1.invoke(ChatActivity.kt:56)
at org.jetbrains.anko.AsyncKt$doAsync$1.invoke(Async.kt:140)
at org.jetbrains.anko.AsyncKt$doAsync$1.invoke(Async.kt)
at org.jetbrains.anko.AsyncKt$sam$Callable$761a5578.call(Async.kt)
at java.util.concurrent.FutureTask.run(FutureTask.java:234)
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$201(ScheduledThreadPoolExecutor.java:153)
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:267)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1080)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:573)
at java.lang.Thread.run(Thread.java:841)
Hi
This Class In Debug Mode Is true
But When Release App Not Work This Class
This Class Has Error :
HttpCookie.hasExpired() is Null Of object Refrence ????
Can anyone tell me how to use this? When I tried to use it, the cookies would not persist application restarts.
Hello @franmontiel and thank you so very much for this.
I don't have much to say to improve this as it seems to be already pretty well done.
Just a very few minor stuff:
Questions:
Thanks in advance if you can answer these silly questions :)
Again, this work is very appreciated !