Fix an issue with loading bundles with multi-byte character (#2820)

Author: wu-hui

firebase-firestore/CHANGELOG.md

@@ -1,10 +1,18 @@
-# Unreleased
-- [changed] Increases the aggressiveness of network retires when an app's
+# 23.0.3
+- [fixed] Fixed an issue when loading a data bundle with
+  multi-byte Unicode characters leads to failures.
+
+# 23.0.2
+- [changed] Increases the aggressiveness of network retries when an app's
   foreground status changes.
 
 # 23.0.1
 - [changed] The SDK now tries to immediately establish a connection to the
   backend when the app enters the foreground.
+  
+# 23.0.0
+ - [changed] Internal infrastructure improvements.
+ - [changed] Internal changes to support dynamic feature modules.
 
 # 22.1.1
 - [fixed] Fixed an issue that dropped the limit for queries loaded from

firebase-firestore/src/androidTest/java/com/google/firebase/firestore/BundleTest.java

@@ -32,6 +32,7 @@
 import java.io.InputStream;
 import java.io.UnsupportedEncodingException;
 import java.nio.ByteBuffer;
+import java.nio.charset.Charset;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.ExecutionException;
@@ -43,6 +44,7 @@
 
 @RunWith(AndroidJUnit4.class)
 public class BundleTest {
+  private static Charset UTF8_CHARSET = Charset.forName("UTF-8");
   private static String[] BUNDLE_TEMPLATES =
       new String[] {
         "{\"metadata\":{\"id\":\"test-bundle\",\"createTime\":{\"seconds\":1001,\"nanos\":9999},"
@@ -71,7 +73,7 @@ public class BundleTest {
             + "{\"seconds\":1000,\"nanos\":9999},\"exists\":true}}",
         "{\"document\":{\"name\":\"projects/{projectId}/databases/(default)/documents/coll-1/b\","
             + "\"createTime\":{\"seconds\":1,\"nanos\":9},\"updateTime\":{\"seconds\":1,"
-            + "\"nanos\":9},\"fields\":{\"k\":{\"stringValue\":\"b\"},\"bar\":"
+            + "\"nanos\":9},\"fields\":{\"k\":{\"stringValue\":\"\uD83D\uDE0A\"},\"bar\":"
             + "{\"integerValue\":2}}}}"
       };
 
@@ -238,18 +240,22 @@ private void verifyQueryResults() throws ExecutionException, InterruptedExceptio
     CollectionReference collectionQuery = db.collection("coll-1");
     QuerySnapshot collectionSnapshot = Tasks.await(collectionQuery.get(Source.CACHE));
     assertEquals(
-        asList(map("bar", 1L, "k", "a"), map("bar", 2L, "k", "b")),
+        asList(map("bar", 1L, "k", "a"), map("bar", 2L, "k", "\uD83D\uDE0A")),
         querySnapshotToValues(collectionSnapshot));
 
     Query limitQuery = Tasks.await(db.getNamedQuery("limit"));
     QuerySnapshot limitSnapshot = Tasks.await(limitQuery.get(Source.CACHE));
-    assertEquals(asList(map("bar", 2L, "k", "b")), querySnapshotToValues(limitSnapshot));
+    assertEquals(asList(map("bar", 2L, "k", "\uD83D\uDE0A")), querySnapshotToValues(limitSnapshot));
 
     Query limitToLastQuery = Tasks.await(db.getNamedQuery("limit-to-last"));
     QuerySnapshot limitToLastSnapshot = Tasks.await(limitToLastQuery.get(Source.CACHE));
     assertEquals(asList(map("bar", 1L, "k", "a")), querySnapshotToValues(limitToLastSnapshot));
   }
 
+  private int getUTF8ByteCount(String s) {
+    return s.getBytes(UTF8_CHARSET).length;
+  }
+
   /**
    * Returns a valid bundle by replacing project id in `BUNDLE_TEMPLATES` with the given db project
    * id (also recalculates length prefixes).
@@ -262,7 +268,7 @@ private byte[] createBundle(String projectId) throws UnsupportedEncodingExceptio
     for (int i = 1; i < BUNDLE_TEMPLATES.length; ++i) {
       // Extract elements from BUNDLE_TEMPLATE and replace the project ID.
       String element = BUNDLE_TEMPLATES[i].replaceAll("\\{projectId\\}", projectId);
-      bundle.append(element.length());
+      bundle.append(getUTF8ByteCount(element));
       bundle.append(element);
     }
 
@@ -273,7 +279,8 @@ private byte[] createBundle(String projectId) throws UnsupportedEncodingExceptio
         BUNDLE_TEMPLATES[0].replace(
             "{totalBytes}", Integer.toString(bundleString.getBytes("UTF-8").length));
 
-    return (metadata.length() + metadata + bundleString).getBytes("UTF-8");
+    String fullBundle = getUTF8ByteCount(metadata) + metadata + bundleString;
+    return fullBundle.getBytes("UTF-8");
   }
 
   /**

firebase-firestore/src/main/java/com/google/firebase/firestore/bundle/BundleReader.java

@@ -16,10 +16,11 @@
 
 import androidx.annotation.Nullable;
 import com.google.firebase.firestore.util.Logger;
+import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
-import java.nio.CharBuffer;
+import java.nio.ByteBuffer;
 import java.nio.charset.Charset;
 import org.json.JSONException;
 import org.json.JSONObject;
@@ -34,18 +35,21 @@ public class BundleReader {
   /** The capacity for the internal char buffer. */
   protected static final int BUFFER_CAPACITY = 1024;
 
+  private static final Charset UTF8_CHARSET = Charset.forName("UTF-8");
+
   private final BundleSerializer serializer;
+  private final InputStream bundleInputStream;
   private final InputStreamReader dataReader;
-  private final Charset charset = Charset.forName("UTF-8");
 
   @Nullable BundleMetadata metadata;
-  private CharBuffer buffer;
+  private ByteBuffer buffer;
   long bytesRead;
 
-  public BundleReader(BundleSerializer serializer, InputStream data) {
+  public BundleReader(BundleSerializer serializer, InputStream bundleInputStream) {
     this.serializer = serializer;
-    dataReader = new InputStreamReader(data, charset);
-    buffer = CharBuffer.allocate(BUFFER_CAPACITY);
+    this.bundleInputStream = bundleInputStream;
+    this.dataReader = new InputStreamReader(this.bundleInputStream);
+    buffer = ByteBuffer.allocate(BUFFER_CAPACITY);
 
     buffer.flip(); // Start the buffer in "reading mode"
   }
@@ -83,7 +87,7 @@ public long getBytesRead() {
   }
 
   public void close() throws IOException {
-    dataReader.close();
+    bundleInputStream.close();
   }
 
   /**
@@ -101,8 +105,9 @@ private BundleElement readNextElement() throws IOException, JSONException {
       return null;
     }
 
-    String json = readJsonString(Integer.parseInt(lengthPrefix));
-    bytesRead += lengthPrefix.length() + json.getBytes(charset).length;
+    int jsonStringByteCount = Integer.parseInt(lengthPrefix);
+    String json = readJsonString(jsonStringByteCount);
+    bytesRead += lengthPrefix.getBytes(UTF8_CHARSET).length + jsonStringByteCount;
     return decodeBundleElement(json);
   }
 
@@ -133,9 +138,9 @@ private BundleElement readNextElement() throws IOException, JSONException {
       throw abort("Reached the end of bundle when a length string is expected.");
     }
 
-    char[] c = new char[nextOpenBracket];
-    buffer.get(c);
-    return new String(c);
+    byte[] b = new byte[nextOpenBracket];
+    buffer.get(b);
+    return UTF8_CHARSET.decode(ByteBuffer.wrap(b)).toString();
   }
 
   /** Returns the index of the first open bracket, or -1 if none is found. */
@@ -157,25 +162,30 @@ private int indexOfOpenBracket() {
    * Reads from a specified position from the internal buffer, for a specified number of bytes,
    * pulling more data from the underlying stream if needed.
    *
-   * <p>Returns a string decoded from the read bytes.
+   * <p>Returns an object containing the Json string and its UTF8 byte count.
    */
-  private String readJsonString(int length) throws IOException {
-    StringBuilder json = new StringBuilder(length);
+  private String readJsonString(int bytesToRead) throws IOException {
+    ByteArrayOutputStream jsonBytes = new ByteArrayOutputStream();
 
-    int remaining = length;
+    // Read at least `bytesToRead` number of bytes from the bundle into `this.buffer`, pulling more
+    // data if necessary.
+    // Exactly `bytesToRead` number of bytes will be put in `jsonBytes` after the loop completes.
+    int remaining = bytesToRead;
     while (remaining > 0) {
       if (buffer.remaining() == 0 && !pullMoreData()) {
         throw abort("Reached the end of bundle when more data was expected.");
       }
 
+      // `read` is the number of bytes guaranteed to exist in `this.buffer` after the above
+      // call to `pullMoreData`. Copy them to `jsonBytes` and advance `this.buffer`'s position.
       int read = Math.min(remaining, buffer.remaining());
-      json.append(buffer, 0, read);
+      jsonBytes.write(buffer.array(), buffer.arrayOffset() + buffer.position(), read);
       buffer.position(buffer.position() + read);
 
       remaining -= read;
     }
 
-    return json.toString();
+    return jsonBytes.toString(UTF8_CHARSET.name());
   }
 
   /**
@@ -185,9 +195,17 @@ private String readJsonString(int length) throws IOException {
    */
   private boolean pullMoreData() throws IOException {
     buffer.compact();
-    int read = dataReader.read(buffer);
+
+    int bytesRead =
+        bundleInputStream.read(
+            buffer.array(), buffer.arrayOffset() + buffer.position(), buffer.remaining());
+    boolean readSuccess = bytesRead > 0;
+    if (readSuccess) {
+      buffer.position(buffer.position() + bytesRead);
+    }
+
     buffer.flip();
-    return read > 0;
+    return readSuccess;
   }
 
   /** Converts a JSON-encoded bundle element into its model class. */

firebase-firestore/src/main/java/com/google/firebase/firestore/util/ByteBufferInputStream.java

@@ -27,6 +27,11 @@ public ByteBufferInputStream(ByteBuffer buf) {
     this.buffer = buf;
   }
 
+  @Override
+  public int available() throws IOException {
+    return this.buffer.remaining();
+  }
+
   @Override
   public int read() {
     if (!buffer.hasRemaining()) {