Audit code to ensure printf style templates are compile time constants. (#15)

When the format string is not, it could be influenced by error messages
or user input, and could possibly include illegal format specifiers
(such as %p). While not as dangerous as in C/C++, this can cause
exceptions to be thrown, which is especially bad, since the reason we're
printf'ing is typically because we're in the middle of error handling.

As part of this, I've audited usages of:
- f.f.util.Assert.hardAssert
- f.f.util.Assert.fail
- f.f.util.Logger.warn
- f.f.util.Logger.debug
- java.lang.String.format

Author: rsgowman

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

@@ -636,7 +636,7 @@ public void testQuerySnapshotEventsForAdd() {
                   assertFalse(doc2.getMetadata().hasPendingWrites());
                   break;
                 default:
-                  fail("unexpected call to onSnapshot:" + snapshot);
+                  fail("unexpected call to onSnapshot: " + snapshot);
               }
             });
     waitFor(emptyLatch);

firebase-firestore/src/androidTest/java/com/google/firebase/firestore/testutil/EventAccumulator.java

@@ -39,7 +39,7 @@ public EventAccumulator() {
   public EventListener<T> listener() {
     return (value, error) -> {
       synchronized (EventAccumulator.this) {
-        hardAssert(error == null, "Unexpected error: " + error);
+        hardAssert(error == null, "Unexpected error: %s", error);
         Log.i("EventAccumulator", "Received new event: " + value);
         events.add(value);
         checkFulfilled();

firebase-firestore/src/main/java/com/google/firebase/firestore/DocumentSnapshot.java

@@ -505,15 +505,14 @@ private Object convertValue(FieldValue value, FieldValueOptions options) {
         // TODO: Somehow support foreign references.
         Logger.warn(
             "DocumentSnapshot",
-            String.format(
-                "Document %s contains a document reference within a different database "
-                    + "(%s/%s) which is not supported. It will be treated as a reference in "
-                    + "the current database (%s/%s) instead.",
-                key.getPath(),
-                refDatabase.getProjectId(),
-                refDatabase.getDatabaseId(),
-                database.getProjectId(),
-                database.getDatabaseId()));
+            "Document %s contains a document reference within a different database "
+                + "(%s/%s) which is not supported. It will be treated as a reference in "
+                + "the current database (%s/%s) instead.",
+            key.getPath(),
+            refDatabase.getProjectId(),
+            refDatabase.getDatabaseId(),
+            database.getProjectId(),
+            database.getDatabaseId());
       }
       return new DocumentReference(key, firestore);
     } else {

firebase-firestore/src/main/java/com/google/firebase/firestore/core/RelationFilter.java

@@ -94,7 +94,7 @@ private boolean matchesComparison(int comp) {
       case GREATER_THAN_OR_EQUAL:
         return comp >= 0;
       default:
-        throw Assert.fail("Unknown operator: ", operator);
+        throw Assert.fail("Unknown operator: %s", operator);
     }
   }
 

firebase-firestore/src/main/java/com/google/firebase/firestore/core/SyncEngine.java

@@ -376,7 +376,7 @@ public void handleRejectedListen(int targetId, Status error) {
       handleRemoteEvent(event);
     } else {
       QueryView queryView = queryViewsByTarget.get(targetId);
-      hardAssert(queryView != null, "Unknown target: " + targetId);
+      hardAssert(queryView != null, "Unknown target: %s", targetId);
       localStore.releaseQuery(queryView.getQuery());
       removeAndCleanup(queryView);
       callback.onError(queryView.getQuery(), error);

firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java

@@ -143,7 +143,7 @@ private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollection
         } else if (mutatedDoc instanceof Document) {
           results = results.insert(key, (Document) mutatedDoc);
         } else {
-          throw fail("Unknown document type: " + mutatedDoc);
+          throw fail("Unknown document type: %s", mutatedDoc);
         }
       }
     }

firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalSerializer.java

@@ -68,7 +68,7 @@ MaybeDocument decodeMaybeDocument(com.google.firebase.firestore.proto.MaybeDocum
         return decodeNoDocument(proto.getNoDocument());
 
       default:
-        throw fail("Unknown MaybeDocument " + proto);
+        throw fail("Unknown MaybeDocument %s", proto);
     }
   }
 

firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationBatchResult.java

@@ -55,10 +55,9 @@ public static MutationBatchResult create(
       ByteString streamToken) {
     Assert.hardAssert(
         batch.getMutations().size() == mutationResults.size(),
-        "Mutations sent "
-            + batch.getMutations().size()
-            + " must equal results received "
-            + mutationResults.size());
+        "Mutations sent %d must equal results received %d",
+        batch.getMutations().size(),
+        mutationResults.size());
     ImmutableSortedMap<DocumentKey, SnapshotVersion> docVersions =
         DocumentCollections.emptyVersionMap();
     List<Mutation> mutations = batch.getMutations();

firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/TransformMutation.java

@@ -126,7 +126,7 @@ public MaybeDocument applyToLocalView(
    * method is guaranteed to be safe.
    */
   private Document requireDocument(@Nullable MaybeDocument maybeDoc) {
-    hardAssert(maybeDoc instanceof Document, "Unknown MaybeDocument type " + maybeDoc);
+    hardAssert(maybeDoc instanceof Document, "Unknown MaybeDocument type %s", maybeDoc);
     Document doc = (Document) maybeDoc;
     hardAssert(doc.getKey().equals(getKey()), "Can only transform a document with the same key");
     return doc;

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/OnlineStateTracker.java

@@ -187,10 +187,10 @@ private void logClientOfflineWarningIfNecessary(String reason) {
             reason);
 
     if (shouldWarnClientIsOffline) {
-      Logger.warn(LOG_TAG, message);
+      Logger.warn(LOG_TAG, "%s", message);
       shouldWarnClientIsOffline = false;
     } else {
-      Logger.debug(LOG_TAG, message);
+      Logger.debug(LOG_TAG, "%s", message);
     }
   }
 

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/RemoteSerializer.java

@@ -287,7 +287,7 @@ public com.google.firestore.v1beta1.Value encodeValue(FieldValue value) {
       DocumentKey key = (DocumentKey) encodedValue;
       builder.setReferenceValue(encodeResourceName(id, key.getPath()));
     } else {
-      throw fail("Can't serialize " + value);
+      throw fail("Can't serialize %s", value);
     }
 
     return builder.build();
@@ -330,7 +330,7 @@ public FieldValue decodeValue(com.google.firestore.v1beta1.Value proto) {
       case MAP_VALUE:
         return decodeMapValue(proto.getMapValue());
       default:
-        throw fail("Unknown value " + proto);
+        throw fail("Unknown value %s", proto);
     }
   }
 
@@ -446,7 +446,7 @@ public com.google.firestore.v1beta1.Write encodeMutation(Mutation mutation) {
     } else if (mutation instanceof DeleteMutation) {
       builder.setDelete(encodeKey(mutation.getKey()));
     } else {
-      throw fail("unknown mutation type ", mutation.getClass());
+      throw fail("unknown mutation type %s", mutation.getClass());
     }
 
     if (!mutation.getPrecondition().isNone()) {
@@ -579,7 +579,8 @@ private FieldTransform decodeFieldTransform(DocumentTransform.FieldTransform fie
         hardAssert(
             fieldTransform.getSetToServerValue()
                 == DocumentTransform.FieldTransform.ServerValue.REQUEST_TIME,
-            "Unknown transform setToServerValue: " + fieldTransform.getSetToServerValue());
+            "Unknown transform setToServerValue: %s",
+            fieldTransform.getSetToServerValue());
         return new FieldTransform(
             FieldPath.fromServerFormat(fieldTransform.getFieldPath()),
             ServerTimestampOperation.getInstance());
@@ -857,7 +858,7 @@ private StructuredQuery.Filter encodeUnaryFilter(Filter filter) {
     } else if (filter instanceof NullFilter) {
       proto.setOp(UnaryFilter.Operator.IS_NULL);
     } else {
-      throw fail("Unrecognized filter: " + filter.getCanonicalId());
+      throw fail("Unrecognized filter: %s", filter.getCanonicalId());
     }
     return StructuredQuery.Filter.newBuilder().setUnaryFilter(proto).build();
   }

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/TargetState.java

@@ -106,7 +106,7 @@ TargetChange toTargetChange() {
           removedDocuments = removedDocuments.insert(key);
           break;
         default:
-          throw fail("Encountered invalid change type: " + changeType);
+          throw fail("Encountered invalid change type: %s", changeType);
       }
     }
 

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/WatchChangeAggregator.java

@@ -148,7 +148,7 @@ public void handleTargetChange(WatchTargetChange targetChange) {
           }
           break;
         default:
-          throw fail("Unknown target watch change state: " + targetChange.getChangeType());
+          throw fail("Unknown target watch change state: %s", targetChange.getChangeType());
       }
     }
   }
@@ -187,7 +187,7 @@ public void handleExistenceFilter(ExistenceFilterWatchChange watchChange) {
           removeDocumentFromTarget(targetId, key, new NoDocument(key, SnapshotVersion.NONE));
         } else {
           hardAssert(
-              expectedCount == 1, "Single document existence filter with count: " + expectedCount);
+              expectedCount == 1, "Single document existence filter with count: %d", expectedCount);
         }
       } else {
         long currentSize = getCurrentDocumentCountForTarget(targetId);

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

@@ -718,7 +718,7 @@ T deserialize(
           if (throwOnUnknownProperties) {
             throw new RuntimeException(message);
           } else if (warnOnUnknownProperties) {
-            Logger.warn(CustomClassMapper.class.getSimpleName(), message);
+            Logger.warn(CustomClassMapper.class.getSimpleName(), "%s", message);
           }
         }
       }

firebase-firestore/src/test/java/com/google/firebase/firestore/spec/SpecTestCase.java

@@ -344,7 +344,7 @@ private Query parseQuery(Object querySpec) throws JSONException {
       }
       return query;
     } else {
-      throw Assert.fail("Invalid query: " + querySpec);
+      throw Assert.fail("Invalid query: %s", querySpec);
     }
   }
 
@@ -761,7 +761,7 @@ private void doStep(JSONObject step) throws Exception {
       throw Assert.fail(
           "'applyClientState' is not supported on Android and should only be used in multi-client tests");
     } else {
-      throw Assert.fail("Unknown step: " + step);
+      throw Assert.fail("Unknown step: %s", step);
     }
   }