weblab2: use subdomains for project preview #69557
Conversation
…'t work! entry point never runs, wtf webpack)
|
Deployed to a fullstack and was able to confirm that both the single editor and two project editing case works on this branch at the current point: Two projects open now works without hacks: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const port = | ||
| 'localhost' === environmentKey && location.port | ||
| ? `:${location.port}` | ||
| : ''; | ||
| return `${location.protocol}//${normalizedChannelId}.preview.${subdomain}codeprojects.org${port}`; |
There was a problem hiding this comment.
Channel IDs used verbatim as subdomains
The preview URL is now built as ${normalizedChannelId}.preview... without any sanitization. Channel IDs are produced via Base64.urlsafe_encode64 (see storage_encrypt_channel_id), which frequently yields underscores; underscores are not legal in DNS host labels, so any project whose channel ID includes _ will resolve to an invalid host and the preview iframe/service worker will fail to load. A DNS-safe mapping (e.g., base32/hex or encoding) is needed before using channel IDs as subdomains.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@snickell I updated the channel id normalization to remove underscores. I think the UUID change will allow us to get rid of the normalization logic.
|
heads up that @alex-m-brown is working to switch projects to use a UUID instead of the "channel id" (encrypted project id + storage id) in #69293 |
fisher-alice
left a comment
There was a problem hiding this comment.
Learned a lot from this PR and #69422!
| const prefix = | ||
| useLocalPrefixOverride && isLocalhost | ||
| ? 'localtesting' | ||
| : normalizedChannelId; |
There was a problem hiding this comment.
just confirming my understanding that this resolves the TODO in weblab2_project_service_worker.js.
There was a problem hiding this comment.
Also, if the same project is open in different tabs, would they always show the same file? I can't get this to currently work locally so just curious.
There was a problem hiding this comment.
If the same project is open in different tabs we will force reload a tab anyway, so I'm not too worried about that scenario. We generally don't support it.
There was a problem hiding this comment.
re: the todo, yes it resolves it! I'll remove the todo as a follow-up
There was a problem hiding this comment.
Thanks for the follow-up!
There was a problem hiding this comment.
If the same project is open in different tabs we will force reload a tab anyway, so I'm not too worried about that scenario. We generally don't support it.
Oh interesting - thanks!
Previously we use
https://preview.codeprojects.orgas the domain to serve ALL projects, directed into an iframe embedded within a https://studio.code.org page. A service worker was then used to intercept requests to preview.codeprojects.org, and potentially serve in-memory copies as the user edits, making preview updates fast and not require an HTTP roundtrip to our server.However, there's ONE service worker for all pages in this scenario, and multiple open projects presented an issue: how to identify which project contents to return from the service worker?
By moving to a URL like
https://${channelID}.preview.codeprojects.orgwe solve the problem for our service workers AND it means each student-authored website is served from its own unique domain, solving possible student data exfiltration issues by malicious projects.TODO
preview.