codesandbox / codesandbox-client

An online IDE for rapid web development
https://codesandbox.io
Other
13.03k stars 2.27k forks source link

Forking a project *occasionally* generates a pseudo-random sub-domain name that starts with a digit. #5472

Open LeadDreamer opened 3 years ago

LeadDreamer commented 3 years ago

🐛 bug report

Preflight Checklist

Description of the problem

I use codesandbox a lot in developing Firestore-based React apps, and ran into an "interesting" issue. When Codesandbox forks a project, it generates a pseudo-random domain to allow for testing - so, you may see the project called lumininous-fraternity-df7gxk, and the built app will deploy at df7gxk.csb.app .

Where it gets fun is sometimes the generated sub-domain will START with a digit - for example, 2omvk.csb.app (real case)

According to the RFC 1034 standard, domains:

must start with a letter, end with a letter or digit, and have as interior characters only letters, digits, and hyphen. There are also some restrictions on the length. Labels must be 63 characters or less.

In practice, many browsers and other user agents support domains that start with a number. My personal take is this falls into a grey zone as to whether it's a bug or not. Firebase Auth is requiring a "valid" domain name, but some "invalid" domain names appear and can be used in practice.

I found out the hard way that Firebase Auth does NOT accept that as a sub-domain. the exact same code, forked again so that the sub-domain does not start with a digit, works fine.

Easy fix, obviously, just fork again. But it took a bit of sleuthing (and, well, 40 years of experience with weird problems) to speculate that that was the problem, and try the solution.

How has this issue affected you? What are you trying to accomplish?

About two hours of staring to see "something is not right" until solution was suspected and tested.

To Reproduce

Whelp, since it's pseudo-random, kinda hard to force it. I have a highly confidential sandbox that currently has this issue; I suspect codesandbox people can kinda force it if they try.

Link to sandbox: [link]() (optional)

Sadly, highly confidential

Your Environment

Software Name/Version
Сodesandbox 702e2e569
Browser Chromium 88.0.4324.150 (Official Build) (64-bit)
Operating System Win7
CompuIves commented 3 years ago

This is such a great report, and thanks for the elaborate explanation. This is definitely something that we should and can fix, we can't apply it easily retroactively, but for newly generated ids we can ensure that they don't start with a number. Again, thanks for the elaborate issue, this is very useful!

github-actions[bot] commented 3 years ago

This issue has automatically been marked stale because there has been no activity in a while. Please leave a comment if the issue has not been resolved, or if it is not stale for any other reason. After 2 weeks, this issue will automatically be closed, unless a comment is made or the stale label is removed.

lbogdan commented 3 years ago

Hey @LeadDreamer ,

I did a bit more research, and it seems this was changed in RFC 1123 to also allow for digits, e.g. https://www.freesoft.org/CIE/RFC/1123/14.htm :

The syntax of a legal Internet host name was specified in RFC-952 [DNS:4]. One aspect of host name syntax is hereby changed: the restriction on the first character is relaxed to allow either a letter or a digit. Host software MUST support this more liberal syntax.

lbogdan commented 3 years ago

Still, what about https://9gag.com/ , https://37signals.com/ etc.?

LeadDreamer commented 3 years ago

hence me deleting. Alas GOOGLE (at least the Firebase team) chooses to NOT follow that, hence how I ran into it. And as mentioned above, now that I know it's a problem, I simply re-do the sandbox - the odds are high it won't happen twice in a row.

lbogdan commented 3 years ago

I'm pretty sure there are domains starting with a digit registered through Google Domains. 🙂

Coming back to CodeSandbox, I wouldn't add this limitation based only on Google Firebase's opinions, so I'd be inclined to close this issue as "won't fix".

@CompuIves what do you think?