Add protocol as a separate property for FirebaseStorage service

common/api-review/storage.api.md

@@ -97,6 +97,8 @@ export class _FirebaseStorageImpl implements FirebaseStorage {
     // (undocumented)
     readonly _pool: ConnectionPool;
     // (undocumented)
+    protocol: string;
+    // (undocumented)
     readonly _url?: string | undefined;
 }
 
@@ -292,7 +294,7 @@ export interface UploadResult {
 }
 
 // @public
-export function uploadString(ref: StorageReference, value: string, format?: string, metadata?: UploadMetadata): Promise<UploadResult>;
+export function uploadString(ref: StorageReference, value: string, format?: StringFormat, metadata?: UploadMetadata): Promise<UploadResult>;
 
 // @public
 export interface UploadTask {

packages/storage/src/api.ts

@@ -51,6 +51,7 @@ import {
 } from './reference';
 import { STORAGE_TYPE } from './constants';
 import { EmulatorMockTokenOptions, getModularInstance } from '@firebase/util';
+import { StringFormat } from './implementation/string';
 
 export { EmulatorMockTokenOptions } from '@firebase/util';
 
@@ -109,7 +110,7 @@ export function uploadBytes(
 export function uploadString(
   ref: StorageReference,
   value: string,
-  format?: string,
+  format?: StringFormat,
   metadata?: UploadMetadata
 ): Promise<UploadResult> {
   ref = getModularInstance(ref);

packages/storage/src/implementation/metadata.ts

@@ -156,7 +156,8 @@ export function fromResourceString(
 export function downloadUrlFromResourceString(
   metadata: Metadata,
   resourceString: string,
-  host: string
+  host: string,
+  protocol: string
 ): string | null {
   const obj = jsonObjectOrNull(resourceString);
   if (obj === null) {
@@ -177,7 +178,7 @@ export function downloadUrlFromResourceString(
     const bucket: string = metadata['bucket'] as string;
     const path: string = metadata['fullPath'] as string;
     const urlPart = '/b/' + encode(bucket) + '/o/' + encode(path);
-    const base = makeUrl(urlPart, host);
+    const base = makeUrl(urlPart, host, protocol);
     const queryString = makeQueryString({
       alt: 'media',
       token

packages/storage/src/implementation/requests.ts

@@ -90,7 +90,8 @@ export function downloadUrlHandler(
     return downloadUrlFromResourceString(
       metadata as Metadata,
       text,
-      service.host
+      service.host,
+      service.protocol
     );
   }
   return handler;
@@ -99,10 +100,7 @@ export function downloadUrlHandler(
 export function sharedErrorHandler(
   location: Location
 ): (p1: Connection, p2: StorageError) => StorageError {
-  function errorHandler(
-    xhr: Connection,
-    err: StorageError
-  ): StorageError {
+  function errorHandler(xhr: Connection, err: StorageError): StorageError {
     let newErr;
     if (xhr.getStatus() === 401) {
       if (
@@ -136,10 +134,7 @@ export function objectErrorHandler(
 ): (p1: Connection, p2: StorageError) => StorageError {
   const shared = sharedErrorHandler(location);
 
-  function errorHandler(
-    xhr: Connection,
-    err: StorageError
-  ): StorageError {
+  function errorHandler(xhr: Connection, err: StorageError): StorageError {
     let newErr = shared(xhr, err);
     if (xhr.getStatus() === 404) {
       newErr = objectNotFound(location.path);
@@ -156,7 +151,7 @@ export function getMetadata(
   mappings: Mappings
 ): RequestInfo<Metadata> {
   const urlPart = location.fullServerUrl();
-  const url = makeUrl(urlPart, service.host);
+  const url = makeUrl(urlPart, service.host, service.protocol);
   const method = 'GET';
   const timeout = service.maxOperationRetryTime;
   const requestInfo = new RequestInfo(
@@ -192,7 +187,7 @@ export function list(
     urlParams['maxResults'] = maxResults;
   }
   const urlPart = location.bucketOnlyServerUrl();
-  const url = makeUrl(urlPart, service.host);
+  const url = makeUrl(urlPart, service.host, service.protocol);
   const method = 'GET';
   const timeout = service.maxOperationRetryTime;
   const requestInfo = new RequestInfo(
@@ -212,7 +207,7 @@ export function getDownloadUrl(
   mappings: Mappings
 ): RequestInfo<string | null> {
   const urlPart = location.fullServerUrl();
-  const url = makeUrl(urlPart, service.host);
+  const url = makeUrl(urlPart, service.host, service.protocol);
   const method = 'GET';
   const timeout = service.maxOperationRetryTime;
   const requestInfo = new RequestInfo(
@@ -232,7 +227,7 @@ export function updateMetadata(
   mappings: Mappings
 ): RequestInfo<Metadata> {
   const urlPart = location.fullServerUrl();
-  const url = makeUrl(urlPart, service.host);
+  const url = makeUrl(urlPart, service.host, service.protocol);
   const method = 'PATCH';
   const body = toResourceString(metadata, mappings);
   const headers = { 'Content-Type': 'application/json; charset=utf-8' };
@@ -254,7 +249,7 @@ export function deleteObject(
   location: Location
 ): RequestInfo<void> {
   const urlPart = location.fullServerUrl();
-  const url = makeUrl(urlPart, service.host);
+  const url = makeUrl(urlPart, service.host, service.protocol);
   const method = 'DELETE';
   const timeout = service.maxOperationRetryTime;
 
@@ -334,7 +329,7 @@ export function multipartUpload(
     throw cannotSliceBlob();
   }
   const urlParams: UrlParams = { name: metadata_['fullPath']! };
-  const url = makeUrl(urlPart, service.host);
+  const url = makeUrl(urlPart, service.host, service.protocol);
   const method = 'POST';
   const timeout = service.maxUploadRetryTime;
   const requestInfo = new RequestInfo(
@@ -397,7 +392,7 @@ export function createResumableUpload(
   const urlPart = location.bucketOnlyServerUrl();
   const metadataForUpload = metadataForUpload_(location, blob, metadata);
   const urlParams: UrlParams = { name: metadataForUpload['fullPath']! };
-  const url = makeUrl(urlPart, service.host);
+  const url = makeUrl(urlPart, service.host, service.protocol);
   const method = 'POST';
   const headers = {
     'X-Goog-Upload-Protocol': 'resumable',

packages/storage/src/implementation/url.ts

@@ -20,14 +20,16 @@
  */
 import { UrlParams } from './requestinfo';
 
-export function makeUrl(urlPart: string, host: string): string {
-  const protocolMatch = host.match(/^(\w+):\/\/.+/);
-  const protocol = protocolMatch?.[1];
+export function makeUrl(
+  urlPart: string,
+  host: string,
+  protocol: string
+): string {
   let origin = host;
   if (protocol == null) {
     origin = `https://${host}`;
   }
-  return `${origin}/v0${urlPart}`;
+  return `${protocol}://${origin}/v0${urlPart}`;
 }
 
 export function makeQueryString(params: UrlParams): string {

packages/storage/src/service.ts

@@ -136,7 +136,8 @@ export function connectStorageEmulator(
     mockUserToken?: EmulatorMockTokenOptions | string;
   } = {}
 ): void {
-  storage.host = `http://${host}:${port}`;
+  storage.host = `${host}:${port}`;
+  storage.protocol = 'http';
   const { mockUserToken } = options;
   if (mockUserToken) {
     storage._overrideAuthToken =
@@ -149,7 +150,7 @@ export function connectStorageEmulator(
 /**
  * A service that provides Firebase Storage Reference instances.
  * @param opt_url - gs:// url to a custom Storage Bucket
- * 
+ *
  * @internal
  */
 export class FirebaseStorageImpl implements FirebaseStorage {
@@ -158,9 +159,9 @@ export class FirebaseStorageImpl implements FirebaseStorage {
    * This string can be in the formats:
    * - host
    * - host:port
-   * - protocol://host:port
    */
   private _host: string = DEFAULT_HOST;
+  protocol: string = 'https';
   protected readonly _appId: string | null = null;
   private readonly _requests: Set<Request<unknown>>;
   private _deleted: boolean = false;
@@ -201,8 +202,7 @@ export class FirebaseStorageImpl implements FirebaseStorage {
 
   /**
    * Set host string for this service.
-   * @param host - host string in the form of host, host:port,
-   * or protocol://host:port
+   * @param host - host string in the form of host or host:port
    */
   set host(host: string) {
     this._host = host;

packages/storage/test/unit/location.test.ts

@@ -0,0 +1,50 @@
+/**
+ * @license
+ * Copyright 2017 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import { expect } from 'chai';
+import { DEFAULT_HOST } from '../../src/implementation/constants';
+import { Location } from '../../src/implementation/location';
+
+describe('Firebase Storage > Location', () => {
+  it('makeFromUrl handles an emulator url correctly', () => {
+    const loc = Location.makeFromUrl(
+      'http://localhost:3001/v0/b/abcdefg.appspot.com/o/abcde.txt',
+      'localhost:3001'
+    );
+    expect(loc.bucket).to.equal('abcdefg.appspot.com');
+  });
+  it('makeFromUrl handles a Firebase Storage url correctly', () => {
+    const loc = Location.makeFromUrl(
+      'https://firebasestorage.googleapis.com/v0/b/abcdefgh.appspot.com/o/abcde.txt',
+      DEFAULT_HOST
+    );
+    expect(loc.bucket).to.equal('abcdefgh.appspot.com');
+  });
+  it('makeFromUrl handles a gs url correctly', () => {
+    const loc = Location.makeFromUrl(
+      'gs://mybucket/child/path/abcde.txt',
+      DEFAULT_HOST
+    );
+    expect(loc.bucket).to.equal('mybucket');
+  });
+  it('makeFromUrl handles a Cloud Storage url correctly', () => {
+    const loc = Location.makeFromUrl(
+      'https://storage.googleapis.com/mybucket/abcde.txt',
+      DEFAULT_HOST
+    );
+    expect(loc.bucket).to.equal('mybucket');
+  });
+});

packages/storage/test/unit/requests.test.ts

@@ -202,7 +202,7 @@ describe('Firebase Storage > Requests', () => {
       const requestInfo = getMetadata(storageService, location, mappings);
       assertObjectIncludes(
         {
-          url: makeUrl(url, storageService.host),
+          url: makeUrl(url, storageService.host, storageService.protocol),
           method: 'GET',
           body: null,
           headers: {},
@@ -222,7 +222,11 @@ describe('Firebase Storage > Requests', () => {
     const requestInfo = list(storageService, locationRoot, '/');
     assertObjectIncludes(
       {
-        url: makeUrl(locationNormalNoObjUrl, storageService.host),
+        url: makeUrl(
+          locationNormalNoObjUrl,
+          storageService.host,
+          storageService.protocol
+        ),
         method: 'GET',
         body: null,
         headers: {},
@@ -252,7 +256,11 @@ describe('Firebase Storage > Requests', () => {
       );
       assertObjectIncludes(
         {
-          url: makeUrl(locationNoObjectUrl, storageService.host),
+          url: makeUrl(
+            locationNoObjectUrl,
+            storageService.host,
+            storageService.protocol
+          ),
           method: 'GET',
           body: null,
           headers: {},
@@ -319,7 +327,7 @@ describe('Firebase Storage > Requests', () => {
       const requestInfo = getDownloadUrl(storageService, location, mappings);
       assertObjectIncludes(
         {
-          url: makeUrl(url, storageService.host),
+          url: makeUrl(url, storageService.host, storageService.protocol),
           method: 'GET',
           body: null,
           headers: {},
@@ -354,7 +362,7 @@ describe('Firebase Storage > Requests', () => {
       );
       assertObjectIncludes(
         {
-          url: makeUrl(url, storageService.host),
+          url: makeUrl(url, storageService.host, storageService.protocol),
           method: 'PATCH',
           body: metadataString,
           headers: { 'Content-Type': metadataContentType },
@@ -385,7 +393,7 @@ describe('Firebase Storage > Requests', () => {
       const requestInfo = deleteObject(storageService, location);
       assertObjectIncludes(
         {
-          url: makeUrl(url, storageService.host),
+          url: makeUrl(url, storageService.host, storageService.protocol),
           method: 'DELETE',
           body: null,
           headers: {},
@@ -451,7 +459,7 @@ describe('Firebase Storage > Requests', () => {
 
       assertObjectIncludes(
         {
-          url: makeUrl(url, storageService.host),
+          url: makeUrl(url, storageService.host, storageService.protocol),
           method: 'POST',
           urlParams: { name: location.path },
           headers: {
@@ -496,7 +504,7 @@ describe('Firebase Storage > Requests', () => {
       );
       assertObjectIncludes(
         {
-          url: makeUrl(url, storageService.host),
+          url: makeUrl(url, storageService.host, storageService.protocol),
           method: 'POST',
           urlParams: { name: location.path },
           headers: {

packages/storage/test/unit/service.test.ts

@@ -257,7 +257,8 @@ GOOG4-RSA-SHA256`
         testShared.makePool(newSend)
       );
       connectStorageEmulator(service, 'test.host.org', 1234);
-      expect(service.host).to.equal('http://test.host.org:1234');
+      expect(service.host).to.equal('test.host.org:1234');
+      expect(service.protocol).to.equal('http');
       void getDownloadURL(ref(service, 'test.png'));
     });
     it('sets mock user token string if specified', done => {
@@ -283,7 +284,8 @@ GOOG4-RSA-SHA256`
         testShared.makePool(newSend)
       );
       connectStorageEmulator(service, 'test.host.org', 1234, { mockUserToken });
-      expect(service.host).to.equal('http://test.host.org:1234');
+      expect(service.host).to.equal('test.host.org:1234');
+      expect(service.protocol).to.equal('http');
       expect(service._overrideAuthToken).to.equal(mockUserToken);
       void getDownloadURL(ref(service, 'test.png'));
     });
@@ -313,7 +315,8 @@ GOOG4-RSA-SHA256`
       connectStorageEmulator(service, 'test.host.org', 1234, {
         mockUserToken: { sub: 'alice' }
       });
-      expect(service.host).to.equal('http://test.host.org:1234');
+      expect(service.host).to.equal('test.host.org:1234');
+      expect(service.protocol).to.equal('http');
       token = service._overrideAuthToken;
       // Token should be an unsigned JWT with header { "alg": "none", "type": "JWT" } (base64url):
       expect(token).to.match(/^eyJhbGciOiJub25lIiwidHlwZSI6IkpXVCJ9\./);
@@ -403,9 +406,7 @@ GOOG4-RSA-SHA256`
           TaskEvent.STATE_CHANGED,
           undefined,
           (err: StorageError | Error) => {
-            expect((err as StorageError).code).to.equal(
-              'storage/app-deleted'
-            );
+            expect((err as StorageError).code).to.equal('storage/app-deleted');
             resolve();
           },
           () => {