android: Fix CanonicalCookie same_site field

chrome/android/java/src/org/chromium/chrome/browser/cookies/CanonicalCookie.java

Index: chrome/android/java/src/org/chromium/chrome/browser/cookies/CanonicalCookie.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/cookies/CanonicalCookie.java b/chrome/android/java/src/org/chromium/chrome/browser/cookies/CanonicalCookie.java
index eead2a8143cd2a0b0c936f61b537e87c74846f5f..8cda73f44888d593c4f493f9e59820913416f9bb 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/cookies/CanonicalCookie.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/cookies/CanonicalCookie.java
@@ -4,12 +4,24 @@
 
 package org.chromium.chrome.browser.cookies;
 
+import java.io.ByteArrayOutputStream;
 import java.io.DataInputStream;
 import java.io.DataOutputStream;
+import java.io.EOFException;
+import java.io.File;
+import java.io.FileInputStream;
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+import javax.crypto.Cipher;
+import javax.crypto.CipherInputStream;
+import javax.crypto.CipherOutputStream;
 
 /**
  * Java representation of net/cookies/canonical_cookie.h.
+ *
+ * Also has static methods to write and restore list of CanonicalCookies to and from disk.
  */
 class CanonicalCookie {
     private final String mUrl;
@@ -22,13 +34,13 @@ class CanonicalCookie {
     private final long mLastAccess;
     private final boolean mSecure;
     private final boolean mHttpOnly;
-    private final boolean mSameSite;
+    private final int mSameSite;
     private final int mPriority;
 
     /** Constructs a CanonicalCookie */
     CanonicalCookie(String url, String name, String value, String domain, String path,
             long creation, long expiration, long lastAccess, boolean secure, boolean httpOnly,
-            boolean sameSite, int priority) {
+            int sameSite, int priority) {
         mUrl = url;
         mName = name;
         mValue = value;
@@ -53,8 +65,8 @@ class CanonicalCookie {
         return mHttpOnly;
     }
 
-    /** @return True if the cookie is Same-Site. */
-    boolean isSameSite() {
+    /** @return SameSite enum */
+    int getSameSite() {
         return mSameSite;
     }
 
@@ -103,13 +115,61 @@ class CanonicalCookie {
         return mValue;
     }
 
-    /**
-     * Serializes for saving to disk. Does not close the stream.
-     * It is up to the caller to do so.
-     *
-     * @param out Stream to write the cookie to.
-     */
-    void saveToStream(DataOutputStream out) throws IOException {
+    private static final int SERIALIZATION_VERSION = 20160426;
+
+    // Not private for tests.
+    static void saveToStream(DataOutputStream out, CanonicalCookie[] cookies) throws IOException {
+        if (out == null) {
+            throw new IllegalArgumentException("out arg is null");
+        }
+        if (cookies == null) {
+            throw new IllegalArgumentException("cookies arg is null");
+        }
+        for (CanonicalCookie cookie : cookies) {
+            if (cookie == null) {
+                throw new IllegalArgumentException("cookies arg contains null value");
+            }
+        }
+
+        int length = cookies.length;
+        out.writeInt(SERIALIZATION_VERSION);
+        out.writeInt(length);
+        for (int i = 0; i < length; ++i) {
+            cookies[i].saveToStream(out);
+        }
+    }
+
+    // Not private for tests.
+    static class UnexpectedFormatException extends Exception {
+        public UnexpectedFormatException(String message) {
+            super(message);
+        }
+    }
+
+    // Not private for tests.
+    static List<CanonicalCookie> readFromStream(DataInputStream in)
+            throws IOException, UnexpectedFormatException {
+        if (in == null) {
+            throw new IllegalArgumentException("in arg is null");
+        }
+
+        final int version = in.readInt();
+        if (version != SERIALIZATION_VERSION) {
+            throw new UnexpectedFormatException("Unexpected version");
+        }
+        final int length = in.readInt();
+        if (length < 0) {
+            throw new UnexpectedFormatException("Negative length: " + length);
+        }
+
+        ArrayList<CanonicalCookie> cookies = new ArrayList<>(length);
+        for (int i = 0; i < length; ++i) {
+            cookies.add(createFromStream(in));
+        }
+        return cookies;
+    }
+
+    private void saveToStream(DataOutputStream out) throws IOException {
         out.writeUTF(mUrl);
         out.writeUTF(mName);
         out.writeUTF(mValue);
@@ -120,20 +180,92 @@ class CanonicalCookie {
         out.writeLong(mLastAccess);
         out.writeBoolean(mSecure);
         out.writeBoolean(mHttpOnly);
-        out.writeBoolean(mSameSite);
+        out.writeInt(mSameSite);
         out.writeInt(mPriority);
     }
 
-    /**
-     * Constructs a cookie by deserializing a single entry from the
-     * input stream.
-     *
-     * @param in Stream to read a cookie entry from.
-     */
-    static CanonicalCookie createFromStream(DataInputStream in)
-            throws IOException {
+    private static CanonicalCookie createFromStream(DataInputStream in) throws IOException {
+        return new CanonicalCookie(in.readUTF(), in.readUTF(), in.readUTF(), in.readUTF(),
+                in.readUTF(), in.readLong(), in.readLong(), in.readLong(), in.readBoolean(),
+                in.readBoolean(), in.readInt(), in.readInt());
+    }
+
+    // Not private for tests.
+    static final String LEGACY_FORMAT_MAGIC_STRING = "c0Ok135";
+
+    // If this raises UnexpectedFormatException, then caller should rewind the stream, and try
+    // readFromStream instead.
+    // Not private for tests.
+    static ArrayList<CanonicalCookie> readFromLegacyStream(DataInputStream in)
+            throws IOException, UnexpectedFormatException {
+        // Identify legacy format by the magic string.
+        try {
+            String check = in.readUTF();
+            if (!LEGACY_FORMAT_MAGIC_STRING.equals(check)) {
+                throw new UnexpectedFormatException("No magic string");
+            }
+        } catch (IOException e) {
+            throw new UnexpectedFormatException("IOException while reading magic string");
+        }
+
+        ArrayList<CanonicalCookie> cookies = new ArrayList<>();
+        try {
+            while (true) {
+                CanonicalCookie cookie = createFromLegacyStream(in);
+                cookies.add(cookie);
+            }
+        } catch (EOFException ignored) {
+            // We are done.
+        }
+        return cookies;
+    }
+
+    // Legacy format stored sameSite as a boolean.
+    private static CanonicalCookie createFromLegacyStream(DataInputStream in) throws IOException {
         return new CanonicalCookie(in.readUTF(), in.readUTF(), in.readUTF(), in.readUTF(),
                 in.readUTF(), in.readLong(), in.readLong(), in.readLong(), in.readBoolean(),
-                in.readBoolean(), in.readBoolean(), in.readInt());
+                in.readBoolean(), in.readBoolean() ? 1 : 0, in.readInt());
+    }
+
+    // This leaks implementation details from CookiesFetcher. But CookiesFetcher can't be unit
+    // tested.
+    public static List<CanonicalCookie> readFromFileUnknownFormat(File fileIn, Cipher cipher)

[Yaron, 2016/04/28 03:43:10]

Couldn't you at least omit "public" and leave this package-protected?

[boliu, 2016/04/28 17:49:26]

I guess .cookies is its own package, so there actually is value to distinguish
between public and package protected. Ok..

+            throws IOException, UnexpectedFormatException {
+        DataInputStream in = null;
+        try {
+            FileInputStream streamIn = new FileInputStream(fileIn);
+            in = new DataInputStream(new CipherInputStream(streamIn, cipher));
+            return readFromLegacyStream(in);
+        } catch (UnexpectedFormatException ignored) {
+            // Maybe in new format.
+        } finally {
+            if (in != null) {
+                in.close();
+            }
+            in = null;

[Yaron, 2016/04/28 03:43:10]

Do you really need to null this out?

[boliu, 2016/04/28 17:49:26]

I want to make sure that the "in" here and the "in" below are definitely
different objects. Used different var names instead to make that clear, so can
remove setting it to null.

+        }
+
+        try {
+            FileInputStream streamIn = new FileInputStream(fileIn);
+            in = new DataInputStream(new CipherInputStream(streamIn, cipher));
+            return CanonicalCookie.readFromStream(in);
+        } finally {
+            if (in != null) {
+                in.close();
+            }
+            in = null;

[Yaron, 2016/04/28 03:43:10]

same here

[boliu, 2016/04/28 17:49:25]

ditto

+        }
+    }
+
+    // This leaks implementation details from CookiesFetcher. But CookiesFetcher can't be unit
+    // tested.
+    public static byte[] writeToByteArray(Cipher cipher, CanonicalCookie cookies[])
+            throws IOException {
+        ByteArrayOutputStream byteOut = new ByteArrayOutputStream();
+        CipherOutputStream cipherOut = new CipherOutputStream(byteOut, cipher);
+        DataOutputStream out = new DataOutputStream(cipherOut);
+        CanonicalCookie.saveToStream(out, cookies);
+        out.close();
+        return byteOut.toByteArray();
     }
 }