Fixed issue where Storage reported the wrong SDK version (#5353)

Author: maneesht

README.md

@@ -255,6 +255,11 @@ To run formatting on your entire project you can run
 ./gradlew :<firebase-project>:googleJavaFormat
 ```
 
+To auto-format, just run
+```bash
+./gradlew :<firebase-project>:gJf
+```
+
 #### Kotlin
 
 Kotlin code in this repo is formatted with the `ktfmt` tool. You can enable

firebase-storage/CHANGELOG.md

@@ -1,4 +1,5 @@
 # Unreleased
+* [fixed] Fixed an issue where the wrong SDK version was being reported to the backend.
 
 # 20.2.1
 * [changed] Migrated `firebase-storage` SDK to use standard Firebase executors.

firebase-storage/src/androidTest/java/com/google/firebase/storage/IntegrationTest.java

@@ -18,9 +18,7 @@
 import static com.google.common.truth.Truth.assertThat;
 
 import android.net.Uri;
-import android.os.Environment;
 import androidx.annotation.NonNull;
-import androidx.test.InstrumentationRegistry;
 import androidx.test.rule.GrantPermissionRule;
 import androidx.test.runner.AndroidJUnit4;
 import com.google.android.gms.tasks.Task;
@@ -40,23 +38,23 @@
 /** Integration tests for {@link FirebaseStorage}. */
 @RunWith(AndroidJUnit4.class)
 public class IntegrationTest {
+  // The file size in bytes of "1.1mb.dat"
+  private static final int LARGE_FILE_SIZE_BYTES = 10 * 1024;
+  private final String randomPrefix = UUID.randomUUID().toString();
+  private final String unicodePrefix = "prefix/\\%:😊 ";
+
   @Rule
   public GrantPermissionRule grantPermissionRule =
       GrantPermissionRule.grant(WRITE_EXTERNAL_STORAGE);
 
-  // The file size in bytes of "1.1mb.dat"
-  private static final int LARGE_FILE_SIZE_BYTES = 10 * 1024;
-
   private FirebaseStorage storageClient;
 
-  private final String randomPrefix = UUID.randomUUID().toString();
-
-  private final String unicodePrefix = "prefix/\\%:😊 ";
-
   @Before
   public void before() throws ExecutionException, InterruptedException {
     if (storageClient == null) {
-      FirebaseApp app = FirebaseApp.initializeApp(InstrumentationRegistry.getContext());
+      FirebaseApp app =
+          FirebaseApp.initializeApp(
+              androidx.test.platform.app.InstrumentationRegistry.getInstrumentation().getContext());
       storageClient = FirebaseStorage.getInstance(app);
 
       Tasks.await(getReference("metadata.dat").putBytes(new byte[0]));
@@ -66,12 +64,20 @@ public void before() throws ExecutionException, InterruptedException {
     }
   }
 
+  public File createFile(String fileName) {
+    return new File(
+        androidx.test.platform.app.InstrumentationRegistry.getInstrumentation()
+            .getContext()
+            .getFilesDir(),
+        fileName);
+  }
+
   @Test
   public void downloadFile() throws ExecutionException, InterruptedException, IOException {
-    File tempFile = new File(Environment.getExternalStorageDirectory(), "download.dat");
+    String fileName = "download.dat";
+    File tempFile = createFile(fileName);
 
-    FileDownloadTask.TaskSnapshot fileTask =
-        Tasks.await(getReference("download.dat").getFile(tempFile));
+    FileDownloadTask.TaskSnapshot fileTask = Tasks.await(getReference(fileName).getFile(tempFile));
 
     assertThat(tempFile.exists()).isTrue();
     assertThat(tempFile.length()).isEqualTo(LARGE_FILE_SIZE_BYTES);
@@ -80,9 +86,10 @@ public void downloadFile() throws ExecutionException, InterruptedException, IOEx
 
   @Test
   public void downloadUnicodeFile() throws ExecutionException, InterruptedException, IOException {
-    File tempFile = new File(Environment.getExternalStorageDirectory(), "empty.dat");
+    String fileName = "empty.dat";
+    File tempFile = createFile(fileName);
 
-    Tasks.await(getReference(unicodePrefix + "/empty.dat").getFile(tempFile));
+    Tasks.await(getReference(unicodePrefix + "/" + fileName).getFile(tempFile));
 
     assertThat(tempFile.exists()).isTrue();
   }

firebase-storage/src/main/java/com/google/firebase/storage/network/NetworkRequest.java

@@ -15,8 +15,6 @@
 package com.google.firebase.storage.network;
 
 import android.content.Context;
-import android.content.pm.PackageInfo;
-import android.content.pm.PackageManager;
 import android.net.ConnectivityManager;
 import android.net.Uri;
 import android.text.TextUtils;
@@ -28,6 +26,7 @@
 import com.google.android.gms.tasks.TaskCompletionSource;
 import com.google.firebase.FirebaseApp;
 import com.google.firebase.emulators.EmulatedServiceSettings;
+import com.google.firebase.storage.BuildConfig;
 import com.google.firebase.storage.StorageException;
 import com.google.firebase.storage.internal.StorageReferenceUri;
 import com.google.firebase.storage.network.connection.HttpURLConnectionFactory;
@@ -50,23 +49,18 @@
 /** Encapsulates a single network request and response */
 @SuppressWarnings("unused")
 public abstract class NetworkRequest {
-  private static final String TAG = "NetworkRequest";
-
-  private static final String X_FIREBASE_GMPID = "x-firebase-gmpid";
-  private static final String X_FIREBASE_APPCHECK = "x-firebase-appcheck";
-
   public static final Uri PROD_BASE_URL = Uri.parse("https://firebasestorage.googleapis.com/v0");
-
   /* Do not change these values without changing corresponding logic on the SDK side*/
   public static final int INITIALIZATION_EXCEPTION = -1;
   public static final int NETWORK_UNAVAILABLE = -2;
-
   /*package*/ static final String GET = "GET";
   /*package*/ static final String DELETE = "DELETE";
   /*package*/ static final String POST = "POST";
   /*package*/ static final String PATCH = "PATCH";
   /*package*/ static final String PUT = "PUT";
-
+  private static final String TAG = "NetworkRequest";
+  private static final String X_FIREBASE_GMPID = "x-firebase-gmpid";
+  private static final String X_FIREBASE_APPCHECK = "x-firebase-appcheck";
   private static final int MAXIMUM_TOKEN_WAIT_TIME_MS = 30000;
   private static final String CONTENT_TYPE = "Content-Type";
   private static final String APPLICATION_JSON = "application/json";
@@ -80,7 +74,6 @@ public abstract class NetworkRequest {
 
   private StorageReferenceUri storageReferenceUri;
 
-  private static String gmsCoreVersion;
   private Context context;
   private Map<String, List<String>> resultHeaders;
   private int resultCode;
@@ -321,23 +314,6 @@ private HttpURLConnection createConnection() throws IOException {
     return conn;
   }
 
-  @NonNull
-  private static String getGmsCoreVersion(Context context) {
-    if (gmsCoreVersion == null) {
-      PackageManager packageManager = context.getPackageManager();
-      try {
-        PackageInfo info = packageManager.getPackageInfo("com.google.android.gms", 0);
-        gmsCoreVersion = info.versionName;
-      } catch (PackageManager.NameNotFoundException e) {
-        Log.e(TAG, "Unable to find gmscore in package manager", e);
-      }
-      if (gmsCoreVersion == null) {
-        gmsCoreVersion = "[No Gmscore]";
-      }
-    }
-    return gmsCoreVersion;
-  }
-
   @SuppressWarnings("TryFinallyCanBeTryWithResources")
   private void constructMessage(
       @NonNull HttpURLConnection conn, @Nullable String authToken, @Nullable String appCheckToken)
@@ -355,14 +331,8 @@ private void constructMessage(
     } else {
       Log.w(TAG, "No App Check token for request.");
     }
-
-    StringBuilder userAgent = new StringBuilder("Android/");
-    String gmsCore = getGmsCoreVersion(context);
-    if (!TextUtils.isEmpty(gmsCore)) {
-      userAgent.append(gmsCore);
-    }
-    conn.setRequestProperty("X-Firebase-Storage-Version", userAgent.toString());
-
+    String userAgent = "Android/" + BuildConfig.VERSION_NAME;
+    conn.setRequestProperty("X-Firebase-Storage-Version", userAgent);
     Map<String, String> requestProperties = requestHeaders;
     for (Map.Entry<String, String> entry : requestProperties.entrySet()) {
       conn.setRequestProperty(entry.getKey(), entry.getValue());

firebase-storage/src/test/java/com/google/firebase/storage/network/MockConnectionFactory.java

@@ -20,6 +20,7 @@
 
 import android.util.Base64;
 import androidx.annotation.NonNull;
+import com.google.firebase.storage.BuildConfig;
 import com.google.firebase.storage.network.connection.HttpURLConnectionFactory;
 import java.io.BufferedReader;
 import java.io.ByteArrayOutputStream;
@@ -44,10 +45,10 @@
 
 public class MockConnectionFactory implements HttpURLConnectionFactory {
   private final boolean binaryBody;
-  private HttpURLConnection oldMock;
-  private List<String> verifications = new ArrayList<>();
   private final BufferedReader br;
   private final Semaphore pauseSemaphore = new Semaphore(0);
+  private HttpURLConnection oldMock;
+  private List<String> verifications = new ArrayList<>();
   private int lineCount = 0;
   private int pauseRecord = Integer.MAX_VALUE;
   private int currentRecord = 0;
@@ -69,6 +70,11 @@ public MockConnectionFactory(String testName, boolean binaryBody) {
     }
   }
 
+  private static InputStream getResFile(String fileName) {
+    ClassLoader classLoader = MockConnectionFactory.class.getClassLoader();
+    return classLoader.getResourceAsStream("activitylogs/" + fileName);
+  }
+
   public void setPauseRecord(int pauseRecord) {
     this.pauseRecord = pauseRecord;
   }
@@ -77,11 +83,6 @@ public Semaphore getSemaphore() {
     return pauseSemaphore;
   }
 
-  private static InputStream getResFile(String fileName) {
-    ClassLoader classLoader = MockConnectionFactory.class.getClassLoader();
-    return classLoader.getResourceAsStream("activitylogs/" + fileName);
-  }
-
   @Override
   public synchronized HttpURLConnection createInstance(@NonNull URL url) throws IOException {
     verifyOldMock();
@@ -206,7 +207,11 @@ public synchronized void verifyOldMock() {
             key = value.substring(0, comma);
             value = value.substring(comma + 1);
             requestPropertyKeys.add(key);
-            requestPropertyValues.add(value);
+            if (key.equals("X-Firebase-Storage-Version")) {
+              requestPropertyValues.add("Android/" + BuildConfig.VERSION_NAME);
+            } else {
+              requestPropertyValues.add(value);
+            }
           }
         } else if (key.equalsIgnoreCase("setDoOutput")) {
           verify(oldMock).setDoOutput("true".equals(value));