Skip to content

Commit 66d4a1e

Browse files
hsubox76hsubox76
authored
Add protocol as a separate property for FirebaseStorage service (#5453)
1 parent d9808aa commit 66d4a1e

File tree

10 files changed

+116
-51
lines changed

10 files changed

+116
-51
lines changed

‎.changeset/tender-pumpkins-love.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@firebase/storage': patch
3+
---
4+
5+
Store protocol and host separately on Storage service instance. Fixes a bug when generating url strings.

‎common/api-review/storage.api.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@ export class _FirebaseStorageImpl implements FirebaseStorage {
7575
_getAppCheckToken(): Promise<string | null>;
7676
// (undocumented)
7777
_getAuthToken(): Promise<string | null>;
78-
// (undocumented)
7978
get host(): string;
8079
set host(host: string);
8180
// Warning: (ae-forgotten-export) The symbol "RequestInfo" needs to be exported by the entry point index.d.ts
@@ -97,6 +96,8 @@ export class _FirebaseStorageImpl implements FirebaseStorage {
9796
// (undocumented)
9897
readonly _pool: ConnectionPool;
9998
// (undocumented)
99+
_protocol: string;
100+
// (undocumented)
100101
readonly _url?: string | undefined;
101102
}
102103

@@ -292,7 +293,7 @@ export interface UploadResult {
292293
}
293294

294295
// @public
295-
export function uploadString(ref: StorageReference, value: string, format?: string, metadata?: UploadMetadata): Promise<UploadResult>;
296+
export function uploadString(ref: StorageReference, value: string, format?: StringFormat, metadata?: UploadMetadata): Promise<UploadResult>;
296297

297298
// @public
298299
export interface UploadTask {

‎packages/storage/src/api.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ import {
5151
} from './reference';
5252
import { STORAGE_TYPE } from './constants';
5353
import { EmulatorMockTokenOptions, getModularInstance } from '@firebase/util';
54+
import { StringFormat } from './implementation/string';
5455

5556
export { EmulatorMockTokenOptions } from '@firebase/util';
5657

@@ -78,7 +79,7 @@ export {
7879
* Uploads data to this object's location.
7980
* The upload is not resumable.
8081
* @public
81-
* @param ref - StorageReference where data should be uploaded.
82+
* @param ref - {@link StorageReference} where data should be uploaded.
8283
* @param data - The data to upload.
8384
* @param metadata - Metadata for the data to upload.
8485
* @returns A Promise containing an UploadResult
@@ -100,7 +101,7 @@ export function uploadBytes(
100101
* Uploads a string to this object's location.
101102
* The upload is not resumable.
102103
* @public
103-
* @param ref - StorageReference where string should be uploaded.
104+
* @param ref - {@link StorageReference} where string should be uploaded.
104105
* @param value - The string to upload.
105106
* @param format - The format of the string to upload.
106107
* @param metadata - Metadata for the string to upload.
@@ -109,7 +110,7 @@ export function uploadBytes(
109110
export function uploadString(
110111
ref: StorageReference,
111112
value: string,
112-
format?: string,
113+
format?: StringFormat,
113114
metadata?: UploadMetadata
114115
): Promise<UploadResult> {
115116
ref = getModularInstance(ref);
@@ -125,7 +126,7 @@ export function uploadString(
125126
* Uploads data to this object's location.
126127
* The upload can be paused and resumed, and exposes progress updates.
127128
* @public
128-
* @param ref - StorageReference where data should be uploaded.
129+
* @param ref - {@link StorageReference} where data should be uploaded.
129130
* @param data - The data to upload.
130131
* @param metadata - Metadata for the data to upload.
131132
* @returns An UploadTask
@@ -317,7 +318,8 @@ export function getStorage(
317318
* @param storage - The {@link FirebaseStorage} instance
318319
* @param host - The emulator host (ex: localhost)
319320
* @param port - The emulator port (ex: 5001)
320-
* @param options.mockUserToken - the mock auth token to use for unit testing Security Rules.
321+
* @param options - Emulator options. `options.mockUserToken` is the mock auth
322+
* token to use for unit testing Security Rules.
321323
* @public
322324
*/
323325
export function connectStorageEmulator(

‎packages/storage/src/implementation/metadata.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,8 @@ export function fromResourceString(
156156
export function downloadUrlFromResourceString(
157157
metadata: Metadata,
158158
resourceString: string,
159-
host: string
159+
host: string,
160+
protocol: string
160161
): string | null {
161162
const obj = jsonObjectOrNull(resourceString);
162163
if (obj === null) {
@@ -177,7 +178,7 @@ export function downloadUrlFromResourceString(
177178
const bucket: string = metadata['bucket'] as string;
178179
const path: string = metadata['fullPath'] as string;
179180
const urlPart = '/b/' + encode(bucket) + '/o/' + encode(path);
180-
const base = makeUrl(urlPart, host);
181+
const base = makeUrl(urlPart, host, protocol);
181182
const queryString = makeQueryString({
182183
alt: 'media',
183184
token

‎packages/storage/src/implementation/requests.ts

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,8 @@ export function downloadUrlHandler(
9090
return downloadUrlFromResourceString(
9191
metadata as Metadata,
9292
text,
93-
service.host
93+
service.host,
94+
service._protocol
9495
);
9596
}
9697
return handler;
@@ -99,10 +100,7 @@ export function downloadUrlHandler(
99100
export function sharedErrorHandler(
100101
location: Location
101102
): (p1: Connection, p2: StorageError) => StorageError {
102-
function errorHandler(
103-
xhr: Connection,
104-
err: StorageError
105-
): StorageError {
103+
function errorHandler(xhr: Connection, err: StorageError): StorageError {
106104
let newErr;
107105
if (xhr.getStatus() === 401) {
108106
if (
@@ -136,10 +134,7 @@ export function objectErrorHandler(
136134
): (p1: Connection, p2: StorageError) => StorageError {
137135
const shared = sharedErrorHandler(location);
138136

139-
function errorHandler(
140-
xhr: Connection,
141-
err: StorageError
142-
): StorageError {
137+
function errorHandler(xhr: Connection, err: StorageError): StorageError {
143138
let newErr = shared(xhr, err);
144139
if (xhr.getStatus() === 404) {
145140
newErr = objectNotFound(location.path);
@@ -156,7 +151,7 @@ export function getMetadata(
156151
mappings: Mappings
157152
): RequestInfo<Metadata> {
158153
const urlPart = location.fullServerUrl();
159-
const url = makeUrl(urlPart, service.host);
154+
const url = makeUrl(urlPart, service.host, service._protocol);
160155
const method = 'GET';
161156
const timeout = service.maxOperationRetryTime;
162157
const requestInfo = new RequestInfo(
@@ -192,7 +187,7 @@ export function list(
192187
urlParams['maxResults'] = maxResults;
193188
}
194189
const urlPart = location.bucketOnlyServerUrl();
195-
const url = makeUrl(urlPart, service.host);
190+
const url = makeUrl(urlPart, service.host, service._protocol);
196191
const method = 'GET';
197192
const timeout = service.maxOperationRetryTime;
198193
const requestInfo = new RequestInfo(
@@ -212,7 +207,7 @@ export function getDownloadUrl(
212207
mappings: Mappings
213208
): RequestInfo<string | null> {
214209
const urlPart = location.fullServerUrl();
215-
const url = makeUrl(urlPart, service.host);
210+
const url = makeUrl(urlPart, service.host, service._protocol);
216211
const method = 'GET';
217212
const timeout = service.maxOperationRetryTime;
218213
const requestInfo = new RequestInfo(
@@ -232,7 +227,7 @@ export function updateMetadata(
232227
mappings: Mappings
233228
): RequestInfo<Metadata> {
234229
const urlPart = location.fullServerUrl();
235-
const url = makeUrl(urlPart, service.host);
230+
const url = makeUrl(urlPart, service.host, service._protocol);
236231
const method = 'PATCH';
237232
const body = toResourceString(metadata, mappings);
238233
const headers = { 'Content-Type': 'application/json; charset=utf-8' };
@@ -254,7 +249,7 @@ export function deleteObject(
254249
location: Location
255250
): RequestInfo<void> {
256251
const urlPart = location.fullServerUrl();
257-
const url = makeUrl(urlPart, service.host);
252+
const url = makeUrl(urlPart, service.host, service._protocol);
258253
const method = 'DELETE';
259254
const timeout = service.maxOperationRetryTime;
260255

@@ -334,7 +329,7 @@ export function multipartUpload(
334329
throw cannotSliceBlob();
335330
}
336331
const urlParams: UrlParams = { name: metadata_['fullPath']! };
337-
const url = makeUrl(urlPart, service.host);
332+
const url = makeUrl(urlPart, service.host, service._protocol);
338333
const method = 'POST';
339334
const timeout = service.maxUploadRetryTime;
340335
const requestInfo = new RequestInfo(
@@ -397,7 +392,7 @@ export function createResumableUpload(
397392
const urlPart = location.bucketOnlyServerUrl();
398393
const metadataForUpload = metadataForUpload_(location, blob, metadata);
399394
const urlParams: UrlParams = { name: metadataForUpload['fullPath']! };
400-
const url = makeUrl(urlPart, service.host);
395+
const url = makeUrl(urlPart, service.host, service._protocol);
401396
const method = 'POST';
402397
const headers = {
403398
'X-Goog-Upload-Protocol': 'resumable',

‎packages/storage/src/implementation/url.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,16 @@
2020
*/
2121
import { UrlParams } from './requestinfo';
2222

23-
export function makeUrl(urlPart: string, host: string): string {
24-
const protocolMatch = host.match(/^(\w+):\/\/.+/);
25-
const protocol = protocolMatch?.[1];
23+
export function makeUrl(
24+
urlPart: string,
25+
host: string,
26+
protocol: string
27+
): string {
2628
let origin = host;
2729
if (protocol == null) {
2830
origin = `https://${host}`;
2931
}
30-
return `${origin}/v0${urlPart}`;
32+
return `${protocol}://${origin}/v0${urlPart}`;
3133
}
3234

3335
export function makeQueryString(params: UrlParams): string {

‎packages/storage/src/service.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,8 @@ export function connectStorageEmulator(
136136
mockUserToken?: EmulatorMockTokenOptions | string;
137137
} = {}
138138
): void {
139-
storage.host = `http://${host}:${port}`;
139+
storage.host = `${host}:${port}`;
140+
storage._protocol = 'http';
140141
const { mockUserToken } = options;
141142
if (mockUserToken) {
142143
storage._overrideAuthToken =
@@ -149,7 +150,7 @@ export function connectStorageEmulator(
149150
/**
150151
* A service that provides Firebase Storage Reference instances.
151152
* @param opt_url - gs:// url to a custom Storage Bucket
152-
*
153+
*
153154
* @internal
154155
*/
155156
export class FirebaseStorageImpl implements FirebaseStorage {
@@ -158,9 +159,9 @@ export class FirebaseStorageImpl implements FirebaseStorage {
158159
* This string can be in the formats:
159160
* - host
160161
* - host:port
161-
* - protocol://host:port
162162
*/
163163
private _host: string = DEFAULT_HOST;
164+
_protocol: string = 'https';
164165
protected readonly _appId: string | null = null;
165166
private readonly _requests: Set<Request<unknown>>;
166167
private _deleted: boolean = false;
@@ -195,15 +196,14 @@ export class FirebaseStorageImpl implements FirebaseStorage {
195196
}
196197
}
197198

199+
/**
200+
* The host string for this service, in the form of `host` or
201+
* `host:port`.
202+
*/
198203
get host(): string {
199204
return this._host;
200205
}
201206

202-
/**
203-
* Set host string for this service.
204-
* @param host - host string in the form of host, host:port,
205-
* or protocol://host:port
206-
*/
207207
set host(host: string) {
208208
this._host = host;
209209
if (this._url != null) {

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/**
2+
* @license
3+
* Copyright 2021 Google LLC
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
import { expect } from 'chai';
18+
import { DEFAULT_HOST } from '../../src/implementation/constants';
19+
import { Location } from '../../src/implementation/location';
20+
21+
describe('Firebase Storage > Location', () => {
22+
it('makeFromUrl handles an emulator url correctly', () => {
23+
const loc = Location.makeFromUrl(
24+
'http://localhost:3001/v0/b/abcdefg.appspot.com/o/abcde.txt',
25+
'localhost:3001'
26+
);
27+
expect(loc.bucket).to.equal('abcdefg.appspot.com');
28+
});
29+
it('makeFromUrl handles a Firebase Storage url correctly', () => {
30+
const loc = Location.makeFromUrl(
31+
'https://firebasestorage.googleapis.com/v0/b/abcdefgh.appspot.com/o/abcde.txt',
32+
DEFAULT_HOST
33+
);
34+
expect(loc.bucket).to.equal('abcdefgh.appspot.com');
35+
});
36+
it('makeFromUrl handles a gs url correctly', () => {
37+
const loc = Location.makeFromUrl(
38+
'gs://mybucket/child/path/abcde.txt',
39+
DEFAULT_HOST
40+
);
41+
expect(loc.bucket).to.equal('mybucket');
42+
});
43+
it('makeFromUrl handles a Cloud Storage url correctly', () => {
44+
const loc = Location.makeFromUrl(
45+
'https://storage.googleapis.com/mybucket/abcde.txt',
46+
DEFAULT_HOST
47+
);
48+
expect(loc.bucket).to.equal('mybucket');
49+
});
50+
});

0 commit comments

Comments
 (0)