Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Hubs Cloud] Fix local assets path and upload proxy for localhost #5146

Merged
merged 2 commits into from
Feb 17, 2022

Conversation

brianpeiris
Copy link
Contributor

@brianpeiris brianpeiris commented Feb 16, 2022

This PR fixes a couple of issues that occur when running a local client against a Hubs Cloud stack.

  1. It converts the BASE_ASSETS_PATH config from a relative URL to and absolute URL. This fixes an error in the physics system, where we use BASE_ASSETS_PATH to load the Ammo.js WASM bundle.
  2. It fixes the path used by the local cors-proxy, so that it includes the query string token for uploaded files. Previously, uploads would fail to render correctly in a room.

The change to BASE_ASSETS_PATH ought to be explained further, so I will do that here.

In Hubs-Foundation/reticulum#508 we introduced a security fix that prevents uploaded files from being served from the primary domain. Uploads must now be served from secondary domains in order to keep them isolated.

This security change worked well with the frontend hubs client, but it broke some aspects of the admin panel. The admin panel issues were fixed by introducing an UPLOADS_HOST environment variable (#4565) which contained the secondary domain that uploads are typically served from.

However, the UPLOADS_HOST variable did not work in scenarios where Hubs Cloud stacks used Cloudflare as a CDN, since uploads were being served via the CDN, which is a separate domain. We fixed the issue with Cloudflare in #5100 by using the BASE_ASSETS_PATH configuration variable, which did include the correct Cloudflare domain.

Unfortunately, that fix also broke another scenario, when users were running local clients against a Hubs Cloud stack. BASE_ASSETS_PATH was previously never used locally, so it was undefined, which causes a null reference error in getUploadsUrl. We then fixed this in #5125 by defining BASE_ASSETS_PATH locally.

This caused yet another problem. When running locally, BASE_ASSETS_PATH is set to "/" by default in .defaults.env. This breaks the physics system, since it relies on BASE_ASSETS_PATH to load the Ammo.js WASM bundle, and expects BASE_ASSETS_PATH to be an absolute URL, not a relative one. This PR fixes that issue.

Arguably, we could have fixed the null reference error in getUploadsUrl by doing a null check instead of defining BASE_ASSETS_PATH, but it might be better to move forward at this point, instead of reverting the change.

src/utils/configs.js Outdated Show resolved Hide resolved
src/utils/configs.js Outdated Show resolved Hide resolved
src/utils/configs.js Outdated Show resolved Hide resolved
src/utils/configs.js Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants