Skip to content

Commit dd66835

Browse files
committed
Merge branch 'master' into mila/BloomFilter
2 parents 9e49b4c + e9bcd4c commit dd66835

File tree

20 files changed

+364
-32
lines changed

20 files changed

+364
-32
lines changed

.changeset/eleven-cycles-hang.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+
Fixed issue where pause throws an error when a request is in flight.

.changeset/heavy-mirrors-drum.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@firebase/auth-interop-types': patch
3+
---
4+
5+
Remove unused peerDependencies.

.changeset/pretty-carrots-kneel.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@firebase/database': patch
3+
---
4+
5+
Replace `innerHTML` call with `textContent`.

.changeset/tricky-ravens-stare.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@firebase/auth': patch
3+
---
4+
5+
Fix to minimize a potential race condition between auth init and signInWithRedirect

.github/workflows/merge-release-branch.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ jobs:
1818
run: |
1919
export VERSION_SCRIPT="const pkg = require('./packages/firebase/package.json'); console.log(pkg.version);"
2020
export VERSION=`node -e "${VERSION_SCRIPT}"`
21-
echo "::set-output name=RELEASE_VERSION::$VERSION"
21+
echo "RELEASE_VERSION=$VERSION" >> $GITHUB_OUTPUT
2222
- name: Echo version in shell
2323
run: |
2424
echo "Merging release ${{ steps.get-version.outputs.RELEASE_VERSION }}"

.github/workflows/release-prod.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ jobs:
9595
run: |
9696
VERSION_SCRIPT="const pkg = require('./packages/firebase/package.json'); console.log(pkg.version);"
9797
VERSION=`node -e "${VERSION_SCRIPT}"`
98-
echo "::set-output name=BASE_VERSION::$VERSION"
98+
echo "BASE_VERSION=$VERSION" >> $GITHUB_OUTPUT
9999
- name: Echo version in shell
100100
run: |
101101
echo "Base version: ${{ steps.get-version.outputs.BASE_VERSION }}"

.github/workflows/release-staging.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,9 @@ jobs:
114114
run: |
115115
VERSION_SCRIPT="const pkg = require('./packages/firebase/package.json'); console.log(pkg.version);"
116116
VERSION=`node -e "${VERSION_SCRIPT}"`
117-
echo "::set-output name=STAGING_VERSION::$VERSION"
117+
echo "STAGING_VERSION=$VERSION" >> $GITHUB_OUTPUT
118118
BASE_VERSION=$(echo $VERSION | cut -d "-" -f 1)
119-
echo "::set-output name=BASE_VERSION::$BASE_VERSION"
119+
echo "BASE_VERSION=$BASE_VERSION" >> $GITHUB_OUTPUT
120120
- name: Echo versions in shell
121121
run: |
122122
echo "Staging release ${{ steps.get-version.outputs.STAGING_VERSION }}"

packages/auth-interop-types/package.json

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,6 @@
1111
"files": [
1212
"index.d.ts"
1313
],
14-
"peerDependencies": {
15-
"@firebase/app-types": "0.x",
16-
"@firebase/util": "1.x"
17-
},
1814
"repository": {
1915
"directory": "packages/auth-types",
2016
"type": "git",

packages/auth/demo/src/index.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1812,6 +1812,27 @@ function initApp() {
18121812
},
18131813
onAuthError);
18141814

1815+
// Try sign in with redirect once upon page load, not on subsequent loads.
1816+
// This will demonstrate the behavior when signInWithRedirect is called before
1817+
// auth is fully initialized. This will fail on firebase/auth versions 0.21.0 and lower
1818+
// due to https://github.com/firebase/firebase-js-sdk/issues/6827
1819+
/*
1820+
if (sessionStorage.getItem('redirect-race-test') !== 'done') {
1821+
console.log('Starting redirect sign in upon page load.');
1822+
try {
1823+
sessionStorage.setItem('redirect-race-test', 'done');
1824+
signInWithRedirect(
1825+
auth,
1826+
new GoogleAuthProvider(),
1827+
browserPopupRedirectResolver
1828+
).catch(onAuthError);
1829+
} catch (error) {
1830+
console.log('Error while calling signInWithRedirect');
1831+
console.error(error);
1832+
}
1833+
}
1834+
*/
1835+
18151836
// Bootstrap tooltips.
18161837
$('[data-toggle="tooltip"]').tooltip();
18171838

packages/auth/src/platform_browser/strategies/redirect.test.ts

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,37 @@ describe('platform_browser/strategies/redirect', () => {
122122
'auth/argument-error'
123123
);
124124
});
125+
126+
it('awaits on the auth initialization promise before opening redirect', async () => {
127+
// Obtain an auth instance which does not await on the initialization promise.
128+
const authWithoutAwait: TestAuth = await testAuth(
129+
resolver,
130+
undefined,
131+
true
132+
);
133+
// completeRedirectFn calls getRedirectResult under the hood.
134+
const getRedirectResultSpy = sinon.spy(
135+
_getInstance<PopupRedirectResolverInternal>(resolver),
136+
'_completeRedirectFn'
137+
);
138+
const openRedirectSpy = sinon.spy(
139+
_getInstance<PopupRedirectResolverInternal>(resolver),
140+
'_openRedirect'
141+
);
142+
await signInWithRedirect(authWithoutAwait, provider);
143+
expect(getRedirectResultSpy).to.have.been.called;
144+
expect(getRedirectResultSpy).to.have.been.calledBefore(openRedirectSpy);
145+
expect(getRedirectResultSpy).to.have.been.calledWith(
146+
authWithoutAwait,
147+
resolver,
148+
true
149+
);
150+
expect(openRedirectSpy).to.have.been.calledWith(
151+
authWithoutAwait,
152+
provider,
153+
AuthEventType.SIGN_IN_VIA_REDIRECT
154+
);
155+
});
125156
});
126157

127158
context('linkWithRedirect', () => {
@@ -159,6 +190,39 @@ describe('platform_browser/strategies/redirect', () => {
159190
);
160191
});
161192

193+
it('awaits on the auth initialization promise before opening redirect', async () => {
194+
// Obtain an auth instance which does not await on the initialization promise.
195+
const authWithoutAwait: TestAuth = await testAuth(
196+
resolver,
197+
undefined,
198+
true
199+
);
200+
user = testUser(authWithoutAwait, 'uid', 'email', true);
201+
// completeRedirectFn calls getRedirectResult under the hood.
202+
const getRedirectResultSpy = sinon.spy(
203+
_getInstance<PopupRedirectResolverInternal>(resolver),
204+
'_completeRedirectFn'
205+
);
206+
const openRedirectSpy = sinon.spy(
207+
_getInstance<PopupRedirectResolverInternal>(resolver),
208+
'_openRedirect'
209+
);
210+
await authWithoutAwait._updateCurrentUser(user);
211+
await linkWithRedirect(user, provider, resolver);
212+
expect(getRedirectResultSpy).to.have.been.called;
213+
expect(getRedirectResultSpy).to.have.been.calledBefore(openRedirectSpy);
214+
expect(getRedirectResultSpy).to.have.been.calledWith(
215+
authWithoutAwait,
216+
resolver,
217+
true
218+
);
219+
expect(openRedirectSpy).to.have.been.calledWith(
220+
authWithoutAwait,
221+
provider,
222+
AuthEventType.LINK_VIA_REDIRECT
223+
);
224+
});
225+
162226
it('errors if no resolver available', async () => {
163227
auth._popupRedirectResolver = null;
164228
await expect(linkWithRedirect(user, provider)).to.be.rejectedWith(
@@ -236,6 +300,40 @@ describe('platform_browser/strategies/redirect', () => {
236300
);
237301
});
238302

303+
it('awaits on the auth initialization promise before opening redirect', async () => {
304+
// Obtain an auth instance which does not await on the initialization promise.
305+
const authWithoutAwait: TestAuth = await testAuth(
306+
resolver,
307+
undefined,
308+
true
309+
);
310+
user = testUser(authWithoutAwait, 'uid', 'email', true);
311+
// completeRedirectFn calls getRedirectResult under the hood.
312+
const getRedirectResultSpy = sinon.spy(
313+
_getInstance<PopupRedirectResolverInternal>(resolver),
314+
'_completeRedirectFn'
315+
);
316+
const openRedirectSpy = sinon.spy(
317+
_getInstance<PopupRedirectResolverInternal>(resolver),
318+
'_openRedirect'
319+
);
320+
await authWithoutAwait._updateCurrentUser(user);
321+
await signInWithRedirect(authWithoutAwait, provider);
322+
await reauthenticateWithRedirect(user, provider);
323+
expect(getRedirectResultSpy).to.have.been.called;
324+
expect(getRedirectResultSpy).to.have.been.calledBefore(openRedirectSpy);
325+
expect(getRedirectResultSpy).to.have.been.calledWith(
326+
authWithoutAwait,
327+
resolver,
328+
true
329+
);
330+
expect(openRedirectSpy).to.have.been.calledWith(
331+
authWithoutAwait,
332+
provider,
333+
AuthEventType.REAUTH_VIA_REDIRECT
334+
);
335+
});
336+
239337
it('errors if no resolver available', async () => {
240338
auth._popupRedirectResolver = null;
241339
await expect(

packages/auth/src/platform_browser/strategies/redirect.ts

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import { getModularInstance } from '@firebase/util';
4242
*
4343
* @remarks
4444
* To handle the results and errors for this operation, refer to {@link getRedirectResult}.
45+
* Follow the [best practices](https://firebase.google.com/docs/auth/web/redirect-best-practices) when using {@link signInWithRedirect}.
4546
*
4647
* @example
4748
* ```javascript
@@ -91,6 +92,10 @@ export async function _signInWithRedirect(
9192
): Promise<void | never> {
9293
const authInternal = _castAuth(auth);
9394
_assertInstanceOf(auth, provider, FederatedAuthProvider);
95+
// Wait for auth initialization to complete, this will process pending redirects and clear the
96+
// PENDING_REDIRECT_KEY in persistence. This should be completed before starting a new
97+
// redirect and creating a PENDING_REDIRECT_KEY entry.
98+
await authInternal._initializationPromise;
9499
const resolverInternal = _withDefaultResolver(authInternal, resolver);
95100
await _setPendingRedirectStatus(resolverInternal, authInternal);
96101

@@ -103,6 +108,9 @@ export async function _signInWithRedirect(
103108

104109
/**
105110
* Reauthenticates the current user with the specified {@link OAuthProvider} using a full-page redirect flow.
111+
* @remarks
112+
* To handle the results and errors for this operation, refer to {@link getRedirectResult}.
113+
* Follow the [best practices](https://firebase.google.com/docs/auth/web/redirect-best-practices) when using {@link reauthenticateWithRedirect}.
106114
*
107115
* @example
108116
* ```javascript
@@ -113,8 +121,8 @@ export async function _signInWithRedirect(
113121
*
114122
* // After returning from the redirect when your app initializes you can obtain the result
115123
* const result = await getRedirectResult(auth);
116-
* // Link using a redirect.
117-
* await linkWithRedirect(result.user, provider);
124+
* // Reauthenticate using a redirect.
125+
* await reauthenticateWithRedirect(result.user, provider);
118126
* // This will again trigger a full page redirect away from your app
119127
*
120128
* // After returning from the redirect when your app initializes you can obtain the result
@@ -147,6 +155,10 @@ export async function _reauthenticateWithRedirect(
147155
): Promise<void | never> {
148156
const userInternal = getModularInstance(user) as UserInternal;
149157
_assertInstanceOf(userInternal.auth, provider, FederatedAuthProvider);
158+
// Wait for auth initialization to complete, this will process pending redirects and clear the
159+
// PENDING_REDIRECT_KEY in persistence. This should be completed before starting a new
160+
// redirect and creating a PENDING_REDIRECT_KEY entry.
161+
await userInternal.auth._initializationPromise;
150162
// Allow the resolver to error before persisting the redirect user
151163
const resolverInternal = _withDefaultResolver(userInternal.auth, resolver);
152164
await _setPendingRedirectStatus(resolverInternal, userInternal.auth);
@@ -162,6 +174,9 @@ export async function _reauthenticateWithRedirect(
162174

163175
/**
164176
* Links the {@link OAuthProvider} to the user account using a full-page redirect flow.
177+
* @remarks
178+
* To handle the results and errors for this operation, refer to {@link getRedirectResult}.
179+
* Follow the [best practices](https://firebase.google.com/docs/auth/web/redirect-best-practices) when using {@link linkWithRedirect}.
165180
*
166181
* @example
167182
* ```javascript
@@ -199,6 +214,10 @@ export async function _linkWithRedirect(
199214
): Promise<void | never> {
200215
const userInternal = getModularInstance(user) as UserInternal;
201216
_assertInstanceOf(userInternal.auth, provider, FederatedAuthProvider);
217+
// Wait for auth initialization to complete, this will process pending redirects and clear the
218+
// PENDING_REDIRECT_KEY in persistence. This should be completed before starting a new
219+
// redirect and creating a PENDING_REDIRECT_KEY entry.
220+
await userInternal.auth._initializationPromise;
202221
// Allow the resolver to error before persisting the redirect user
203222
const resolverInternal = _withDefaultResolver(userInternal.auth, resolver);
204223
await _assertLinkedStatus(false, userInternal, provider.providerId);

packages/auth/test/helpers/mock_auth.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,8 @@ export class MockPersistenceLayer extends InMemoryPersistence {
7171

7272
export async function testAuth(
7373
popupRedirectResolver?: PopupRedirectResolver,
74-
persistence = new MockPersistenceLayer()
74+
persistence = new MockPersistenceLayer(),
75+
skipAwaitOnInit?: boolean
7576
): Promise<TestAuth> {
7677
const auth: TestAuth = new AuthImpl(
7778
FAKE_APP,
@@ -88,7 +89,13 @@ export async function testAuth(
8889
) as TestAuth;
8990
auth._updateErrorMap(debugErrorMap);
9091

91-
await auth._initializeWithPersistence([persistence], popupRedirectResolver);
92+
if (skipAwaitOnInit) {
93+
// This is used to verify scenarios where auth flows (like signInWithRedirect) are invoked before auth is fully initialized.
94+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
95+
auth._initializeWithPersistence([persistence], popupRedirectResolver);
96+
} else {
97+
await auth._initializeWithPersistence([persistence], popupRedirectResolver);
98+
}
9299
auth.persistenceLayer = persistence;
93100
auth.settings.appVerificationDisabledForTesting = true;
94101
return auth;

packages/database/src/realtime/BrowserPollConnection.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -547,8 +547,8 @@ export class FirebaseIFrameScriptHolder {
547547
if (this.myIFrame) {
548548
//We have to actually remove all of the html inside this iframe before removing it from the
549549
//window, or IE will continue loading and executing the script tags we've already added, which
550-
//can lead to some errors being thrown. Setting innerHTML seems to be the easiest way to do this.
551-
this.myIFrame.doc.body.innerHTML = '';
550+
//can lead to some errors being thrown. Setting textContent seems to be the safest way to do this.
551+
this.myIFrame.doc.body.textContent = '';
552552
setTimeout(() => {
553553
if (this.myIFrame !== null) {
554554
document.body.removeChild(this.myIFrame);

packages/storage/src/implementation/request.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -120,9 +120,9 @@ class NetworkRequest<I extends ConnectionType, O> implements Request<O> {
120120
const hitServer = connection.getErrorCode() === ErrorCode.NO_ERROR;
121121
const status = connection.getStatus();
122122
if (
123-
(!hitServer ||
124-
isRetryStatusCode(status, this.additionalRetryCodes_)) &&
125-
this.retry
123+
!hitServer ||
124+
(isRetryStatusCode(status, this.additionalRetryCodes_) &&
125+
this.retry)
126126
) {
127127
const wasCanceled = connection.getErrorCode() === ErrorCode.ABORT;
128128
backoffCallback(

packages/storage/test/integration/integration.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import {
3636
import { use, expect } from 'chai';
3737
import chaiAsPromised from 'chai-as-promised';
3838
import * as types from '../../src/public-types';
39+
import { Deferred } from '@firebase/util';
3940

4041
use(chaiAsPromised);
4142

@@ -187,4 +188,32 @@ describe('FirebaseStorage Exp', () => {
187188
expect(listResult.items.map(v => v.name)).to.have.members(['a', 'b']);
188189
expect(listResult.prefixes.map(v => v.name)).to.have.members(['c']);
189190
});
191+
192+
it('can pause uploads without an error', async () => {
193+
const referenceA = ref(storage, 'public/exp-upload/a');
194+
const bytesToUpload = new ArrayBuffer(1024 * 1024);
195+
const task = uploadBytesResumable(referenceA, bytesToUpload);
196+
const failureDeferred = new Deferred();
197+
let hasPaused = false;
198+
task.on(
199+
'state_changed',
200+
snapshot => {
201+
if (snapshot.bytesTransferred > 0 && !hasPaused) {
202+
task.pause();
203+
hasPaused = true;
204+
}
205+
},
206+
() => {
207+
failureDeferred.reject('Failed to upload file');
208+
}
209+
);
210+
await Promise.race([
211+
failureDeferred.promise,
212+
new Promise(resolve => setTimeout(resolve, 4000))
213+
]);
214+
task.resume();
215+
await task;
216+
const bytes = await getBytes(referenceA);
217+
expect(bytes).to.deep.eq(bytesToUpload);
218+
});
190219
});

packages/storage/test/unit/connection.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ export class TestingConnection implements Connection<string> {
117117

118118
abort(): void {
119119
this.state = State.START;
120-
this.errorCode = ErrorCode.NO_ERROR;
120+
this.errorCode = ErrorCode.ABORT;
121121
this.resolve();
122122
}
123123

0 commit comments

Comments
 (0)