Repository: firebase/firebase-tools (ts)

Issue:
hosting:channel:deploy partial deployment and 404 error when channel ID contains "#"
<!-- DO NOT DELETE
validate_template=true
template_path=.github/ISSUE_TEMPLATE/bug_report.md
-->

<!--
Thank you for contributing to the Firebase community!

Think you found a bug?
=======================
Yeah, we're definitely not perfect! Please use this template and include a minimal repro when opening the issue. If you know how to solve the issue, please create a Pull Request, and we'd be happy to review it!

Have a feature request?
========================
Great, we love hearing how we can improve our products! However, do not use this template to submit a feature request. Please submit your feature requests to: https://firebase.google.com/support/contact/bugs-features/

Have a usage question?
=======================
We get lots of those and we love helping you, but GitHub is not the best place for them and they will be closed. Please take a look at the guide first: https://firebase.google.com/docs/cli/

If the official documentation doesn't help, try asking through our official support channel: https://firebase.google.com/support/

Additional locations to check for solutions or assistance from the community:
- Stack Overflow: https://stackoverflow.com/
- Firebase Slack Community: https://firebase.community/

*Please avoid duplicate posting across multiple channels!*
-->

If a channel ID contains a `#`, the CLI begins deploying but errors out with a 404 after the "releasing new version..." step. It still creates the preview channel, using the characters before the `#` as the ID, however the resulting link is a "Site Not Found" page.

### [REQUIRED] Environment info

<!-- What version of the Firebase CLI (`firebase-tools`) are you using? Note that your issue may already be fixed in the latest versions. The latest version can be found at https://github.com/firebase/firebase-tools/releases -->

<!-- Output of `firebase --version` -->

**firebase-tools:** 8.15.0

<!-- e.g. macOS, Windows, Ubuntu -->

**Platform:** Manjaro Linux

### [REQUIRED] Test case

<!-- Provide a minimal, complete, and verifiable example (http://stackoverflow.com/help/mcve) -->

```
firebase hosting:channel:deploy test#removed
```

### [REQUIRED] Steps to reproduce

<!-- Provide the steps needed to reproduce the issue with the above test case. -->

:white_check_mark: This works fine:

```
firebase hosting:channel:deploy test-removed
```

:white_check_mark: This has the expected error:

```
firebase hosting:channel:deploy test@removed

Error: HTTP Error: 400, Channel IDs can only include letters, numbers, underscores, hyphens, and periods.
```

:stop_sign: This errors with a 404:

```
firebase hosting:channel:deploy test#removed

=== Deploying to 'redacted'...

i  deploying hosting
i  hosting[redacted]: beginning deploy...
i  hosting[redacted]: found 51 files in out
✔  hosting[redacted]: file upload complete
i  hosting[redacted]: finalizing version...
✔  hosting[redacted]: version finalized
i  hosting[redacted]: releasing new version...

Error: HTTP Error: 404, Not Found
```

### [REQUIRED] Expected behavior

<!-- What is the expected behavior? -->

The CLI should error out explaining why my channel ID is bad.

### [REQUIRED] Actual behavior

The CLI starts deploying and then errors out with `Error: HTTP Error: 404, Not Found`. The partial deploy is seen with `firebase hosting:channel:list` and on the online Firebase console. It uses the characters before the `#` as the ID (i.e. `test#removed` -> `test`). Visiting the link results in the Firebase "Site Not Found" page.

> Run the command with --debug flag, and include the logs below.

```firebase hosting:channel:deploy test#removed --debug``` contains some information I rather not post publicly. I can send privately on request if needed.



Pull request:
normalize channel names on hosting:channel:create and remove hashes
<!--

Thank you for contributing to the Firebase community! Please fill out the form below.

Run the linter and test suite
==============================
Run `npm test` to make sure your changes compile properly and the tests all pass on your local machine. We've hooked up this repo with continuous integration to double check those things for you.

-->

### Description

Fixes #2783

`hosting:channel:deploy` does normalizing of the channel ID provided. `:create` was not. Further, the normalization wasn't removing `#` from the provided IDs and causing issues when passed into a URL.

### Scenarios Tested

- `firebase hosting:channel:create test#bar`
- `firebase hosting:channel:deploy test#bar`

Patch:
diff --git a/src/commands/hosting-channel-create.ts b/src/commands/hosting-channel-create.ts
index 5f98c793b..52b5783fc 100644
--- a/src/commands/hosting-channel-create.ts
+++ b/src/commands/hosting-channel-create.ts
@@ -1,6 +1,6 @@
 import { bold, yellow } from "cli-color";
 
-import { Channel, createChannel, addAuthDomain } from "../hosting/api";
+import { Channel, createChannel, addAuthDomain, normalizeName } from "../hosting/api";
 import { Command } from "../command";
 import { DEFAULT_DURATION, calculateChannelExpireTTL } from "../hosting/expireUtils";
 import { FirebaseError } from "../error";
@@ -56,6 +56,8 @@ export default new Command("hosting:channel:create [channelId]")
         throw new FirebaseError(`"channelId" must not be empty`);
       }
 
+      channelId = normalizeName(channelId);
+
       let channel: Channel;
       try {
         channel = await createChannel(projectId, site, channelId, expireTTL);
diff --git a/src/hosting/api.ts b/src/hosting/api.ts
index e33ac451f..47977c866 100644
--- a/src/hosting/api.ts
+++ b/src/hosting/api.ts
@@ -153,14 +153,14 @@ export interface Version {
 
 /**
  * normalizeName normalizes a name given to it. Most useful for normalizing
- * user provided names. This removes any `/`, ':', or '_' characters and
+ * user provided names. This removes any `/`, ':', '_', or '#' characters and
  * replaces them with dashes (`-`).
  * @param s the name to normalize.
  * @return the normalized name.
  */
 export function normalizeName(s: string): string {
-  // Using a regex replaces *all* bad characters.
-  return s.replace(/[/:_]/g, "-");
+  // Using a regex replaces *all* specified characters at once.
+  return s.replace(/[/:_#]/g, "-");
 }
 
 const apiClient = new Client({


Test patch:
diff --git a/src/test/hosting/api.spec.ts b/src/test/hosting/api.spec.ts
index bf742bd66..c8a98defb 100644
--- a/src/test/hosting/api.spec.ts
+++ b/src/test/hosting/api.spec.ts
@@ -77,6 +77,7 @@ describe("normalizeName", () => {
     { in: "happyBranch", out: "happyBranch" },
     { in: "happy:branch", out: "happy-branch" },
     { in: "happy_branch", out: "happy-branch" },
+    { in: "happy#branch", out: "happy-branch" },
   ];
 
   for (const t of tests) {