Page MenuHomePhabricator
Differential D7687

[blob-service] Set up CORS
ClosedPublic
Actions

Authored by bartek on Apr 29 2023, 9:12 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 2, 7:49 PM
Unknown Object (File)
Mon, Mar 31, 5:11 PM
Unknown Object (File)
Fri, Mar 28, 4:41 AM
Unknown Object (File)
Mon, Mar 17, 9:33 PM
Unknown Object (File)
Sat, Mar 15, 4:45 PM
Unknown Object (File)
Sat, Mar 8, 9:38 AM
Unknown Object (File)
Mar 5 2025, 9:35 PM
Unknown Object (File)
Mar 5 2025, 9:35 PM
View All Files
Subscribers
atul

Details

Reviewers
tomek
• jon
varun
ashoat
Commits
rCOMM2a42a7dad94a: [blob-service] Set up CORS
Summary

Details in ENG-3820 - we need to set up CORS for blob service to work on web.

Test Plan

Made GET and POST/PUT blob service requests from localhost web client ant from native. Tested both local sandbox and production service.
Without this change, the requests fail with CORS errors on web, they're fine on native.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

bartek created this revision.Apr 29 2023, 9:12 AM
bartek held this revision as a draft.
Herald added a subscriber: atul. · View Herald TranscriptApr 29 2023, 9:12 AM
bartek added a child revision: D7689: [web] Add function to upload media to Blob service.Apr 29 2023, 9:20 AM
bartek published this revision for review.Apr 29 2023, 9:24 AM
Harbormaster completed remote builds in B18937: Diff 25936.Apr 29 2023, 9:29 AM
ashoat resigned from this revision.Apr 29 2023, 11:53 AM

New dependency looks good. Not familiar with Rust

• jon accepted this revision.May 1 2023, 12:16 PM
• jon added inline comments.
services/blob/src/http/mod.rs
21

not directly related to this diff, but I would like to move away from --sandbox, and instead just see if the binary is in debug mode.

#[cfg(debug_assertions)]
fn is_sandbox() {
  return true;
}

Also gets rid of a bunch of branching logic

This revision is now accepted and ready to land.May 1 2023, 12:16 PM
bartek updated this revision to Diff 26170.May 8 2023, 4:02 AM

Rebase before landing

Harbormaster completed remote builds in B19116: Diff 26170.May 8 2023, 4:19 AM
Closed by commit rCOMM2a42a7dad94a: [blob-service] Set up CORS (authored by bartek). · Explain WhyMay 8 2023, 4:47 AM
This revision was automatically updated to reflect the committed changes.
bartek added a commit: rCOMM2a42a7dad94a: [blob-service] Set up CORS.

Revision Contents
Changeset List

PathSize
services/
blob/
1 line
src/
http/
18 lines

Diff 25936

View Options

services/blob/Cargo.lock

Show First 20 Lines • Show All 396 Lines • ▼ Show 20 Lines async ensureAndroidPushNotifsEnabled() {
let [hasPermission] = permissionPromisesResult; let [hasPermission] = permissionPromisesResult;
const [, canRequestPermission] = permissionPromisesResult; const [, canRequestPermission] = permissionPromisesResult;
if (!hasPermission && canRequestPermission) { if (!hasPermission && canRequestPermission) {
const permissionResponse = await (async () => { const permissionResponse = await (async () => {
// We issue a call to sleep to match iOS behavior where prompt // We issue a call to sleep to match iOS behavior where prompt
// doesn't appear immediately but after logged-out modal disappears // doesn't appear immediately but after logged-out modal disappears
await sleep(10); await sleep(10);
await this.requestAndroidNotificationsPermission(); return await this.requestAndroidNotificationsPermission();
marcinUnsubmitted
Not Done
Inline Actions

We definitely should return something from this lambda since otherwise it is useless to assign it to variable.

When we find we don't have Android notif permissions, we ignore the result of requestAndroidNotificationsPermission. It seems like that would cause us to fail to register notif permissions on Android.

If we find that we don't have permissions then just calling await this.requestAndroidNotificationsPermission(); would result in a prompt asking for notifications permissions. However if the user grants those permissions then hasPermissions is still falsy (since promise returned nothing), so deviceToken will be set to null. Nevertheless permissions are actually granted byt the OS, so the state on the device and keyserver would heal itself on next render.

This differential fixes this case so that if user grants permissions correct state is achieved immediately without need for additional re-render to heal the state.

marcin: We definitely should return something from this lambda since otherwise it is useless to assign…
})(); })();
hasPermission = permissionResponse; hasPermission = permissionResponse;
} }
if (!hasPermission) { if (!hasPermission) {
this.failedToRegisterPushPermissionsAndroid(!canRequestPermission); this.failedToRegisterPushPermissionsAndroid(!canRequestPermission);
return; return;
} }
▲ Show 20 Lines • Show All 328 Lines • Show Last 20 Lines
View Options

services/blob/Cargo.toml

Show First 20 Lines • Show All 396 Lines • ▼ Show 20 Lines async ensureAndroidPushNotifsEnabled() {
let [hasPermission] = permissionPromisesResult; let [hasPermission] = permissionPromisesResult;
const [, canRequestPermission] = permissionPromisesResult; const [, canRequestPermission] = permissionPromisesResult;
if (!hasPermission && canRequestPermission) { if (!hasPermission && canRequestPermission) {
const permissionResponse = await (async () => { const permissionResponse = await (async () => {
// We issue a call to sleep to match iOS behavior where prompt // We issue a call to sleep to match iOS behavior where prompt
// doesn't appear immediately but after logged-out modal disappears // doesn't appear immediately but after logged-out modal disappears
await sleep(10); await sleep(10);
await this.requestAndroidNotificationsPermission(); return await this.requestAndroidNotificationsPermission();
marcinUnsubmitted
Not Done
Inline Actions

We definitely should return something from this lambda since otherwise it is useless to assign it to variable.

When we find we don't have Android notif permissions, we ignore the result of requestAndroidNotificationsPermission. It seems like that would cause us to fail to register notif permissions on Android.

If we find that we don't have permissions then just calling await this.requestAndroidNotificationsPermission(); would result in a prompt asking for notifications permissions. However if the user grants those permissions then hasPermissions is still falsy (since promise returned nothing), so deviceToken will be set to null. Nevertheless permissions are actually granted byt the OS, so the state on the device and keyserver would heal itself on next render.

This differential fixes this case so that if user grants permissions correct state is achieved immediately without need for additional re-render to heal the state.

marcin: We definitely should return something from this lambda since otherwise it is useless to assign…
})(); })();
hasPermission = permissionResponse; hasPermission = permissionResponse;
} }
if (!hasPermission) { if (!hasPermission) {
this.failedToRegisterPushPermissionsAndroid(!canRequestPermission); this.failedToRegisterPushPermissionsAndroid(!canRequestPermission);
return; return;
} }
▲ Show 20 Lines • Show All 328 Lines • Show Last 20 Lines
View Options

services/blob/src/http/mod.rs

Show First 20 Lines • Show All 396 Lines • ▼ Show 20 Lines async ensureAndroidPushNotifsEnabled() {
let [hasPermission] = permissionPromisesResult; let [hasPermission] = permissionPromisesResult;
const [, canRequestPermission] = permissionPromisesResult; const [, canRequestPermission] = permissionPromisesResult;
if (!hasPermission && canRequestPermission) { if (!hasPermission && canRequestPermission) {
const permissionResponse = await (async () => { const permissionResponse = await (async () => {
// We issue a call to sleep to match iOS behavior where prompt // We issue a call to sleep to match iOS behavior where prompt
// doesn't appear immediately but after logged-out modal disappears // doesn't appear immediately but after logged-out modal disappears
await sleep(10); await sleep(10);
await this.requestAndroidNotificationsPermission(); return await this.requestAndroidNotificationsPermission();
marcinUnsubmitted
Not Done
Inline Actions

We definitely should return something from this lambda since otherwise it is useless to assign it to variable.

When we find we don't have Android notif permissions, we ignore the result of requestAndroidNotificationsPermission. It seems like that would cause us to fail to register notif permissions on Android.

If we find that we don't have permissions then just calling await this.requestAndroidNotificationsPermission(); would result in a prompt asking for notifications permissions. However if the user grants those permissions then hasPermissions is still falsy (since promise returned nothing), so deviceToken will be set to null. Nevertheless permissions are actually granted byt the OS, so the state on the device and keyserver would heal itself on next render.

This differential fixes this case so that if user grants permissions correct state is achieved immediately without need for additional re-render to heal the state.

marcin: We definitely should return something from this lambda since otherwise it is useless to assign…
})(); })();
hasPermission = permissionResponse; hasPermission = permissionResponse;
} }
if (!hasPermission) { if (!hasPermission) {
this.failedToRegisterPushPermissionsAndroid(!canRequestPermission); this.failedToRegisterPushPermissionsAndroid(!canRequestPermission);
return; return;
} }
▲ Show 20 Lines • Show All 328 Lines • Show Last 20 Lines