ui: fixes uri encoding for sql identifiers

The database, table, and index names were not properly escaped which causes the request to fail if it has a special character. The endpoint does not support names with /. This will not be an issue since all ui is being converted to sql-over-http. part of: #94328 Release note: none
cockroachdb · Jan 30, 2023 · 1c45064 · 1c45064
1 parent 10ef5d9
commit 1c45064
Show file tree

Hide file tree

Showing 7 changed files with 147 additions and 31 deletions.
diff --git a/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.tsx
@@ -19,34 +19,35 @@ import { CaretRight } from "src/icon/caretRight";
 import { DatabaseIcon } from "src/icon/databaseIcon";
 import { StackIcon } from "src/icon/stackIcon";
 import { PageConfig, PageConfigItem } from "src/pageConfig";
-import { Pagination, ResultsPerPageLabel } from "src/pagination";
+import { Pagination } from "src/pagination";
 import {
   ColumnDescriptor,
   ISortedTablePagination,
   SortedTable,
   SortSetting,
 } from "src/sortedtable";
 import * as format from "src/util/format";
-import { DATE_FORMAT } from "src/util/format";
+import {
+  DATE_FORMAT,
+  EncodeDatabaseTableUri,
+  EncodeDatabaseUri,
+} from "src/util/format";
 import { mvccGarbage, syncHistory, unique } from "../util";
 
 import styles from "./databaseDetailsPage.module.scss";
 import sortableTableStyles from "src/sortedtable/sortedtable.module.scss";
-import {
-  baseHeadingClasses,
-  statisticsClasses,
-} from "src/transactionsPage/transactionsPageClasses";
+import { baseHeadingClasses } from "src/transactionsPage/transactionsPageClasses";
 import { Moment } from "moment";
 import { Caution } from "@cockroachlabs/icons";
 import { Anchor } from "../anchor";
 import LoadingError from "../sqlActivity/errorComponent";
 import { Loading } from "../loading";
 import { Search } from "../search";
 import {
+  calculateActiveFilters,
+  defaultFilters,
   Filter,
   Filters,
-  defaultFilters,
-  calculateActiveFilters,
 } from "src/queryFilter";
 import { UIConfigState } from "src/store";
 import { TableStatistics } from "src/tableStatistics";
@@ -509,7 +510,7 @@ export class DatabaseDetailsPage extends React.Component<
         ),
         cell: table => (
           <Link
-            to={`/database/${this.props.name}/table/${table.name}`}
+            to={EncodeDatabaseTableUri(this.props.name, table.name)}
             className={cx("icon__container")}
           >
             <DatabaseIcon className={cx("icon--s", "icon--primary")} />
@@ -683,7 +684,10 @@ export class DatabaseDetailsPage extends React.Component<
         ),
         cell: table => (
           <Link
-            to={`/database/${this.props.name}/table/${table.name}?tab=grants`}
+            to={
+              EncodeDatabaseTableUri(this.props.name, table.name) +
+              `?tab=grants`
+            }
             className={cx("icon__container")}
           >
             <DatabaseIcon className={cx("icon--s")} />
@@ -802,7 +806,10 @@ export class DatabaseDetailsPage extends React.Component<
           <Breadcrumbs
             items={[
               { link: "/databases", name: "Databases" },
-              { link: `/database/${this.props.name}`, name: "Tables" },
+              {
+                link: EncodeDatabaseUri(this.props.name),
+                name: "Tables",
+              },
             ]}
             divider={
               <CaretRight className={cx("icon--xxs", "icon--primary")} />

diff --git a/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.tsx b/pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.tsx
@@ -31,7 +31,12 @@ import {
   SummaryCardItemBoolSetting,
 } from "src/summaryCard";
 import * as format from "src/util/format";
-import { DATE_FORMAT, DATE_FORMAT_24_UTC } from "src/util/format";
+import {
+  DATE_FORMAT,
+  DATE_FORMAT_24_UTC,
+  EncodeDatabaseTableUri,
+  EncodeDatabaseUri,
+} from "src/util/format";
 import {
   ascendingAttr,
   columnTitleAttr,
@@ -504,9 +509,15 @@ export class DatabaseTablePage extends React.Component<
           <Breadcrumbs
             items={[
               { link: "/databases", name: "Databases" },
-              { link: `/database/${this.props.databaseName}`, name: "Tables" },
               {
-                link: `/database/${this.props.databaseName}/table/${this.props.name}`,
+                link: EncodeDatabaseUri(this.props.databaseName),
+                name: "Tables",
+              },
+              {
+                link: EncodeDatabaseTableUri(
+                  this.props.databaseName,
+                  this.props.name,
+                ),
                 name: `Table: ${this.props.name}`,
               },
             ]}

diff --git a/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.tsx b/pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.tsx
@@ -17,7 +17,7 @@ import classnames from "classnames/bind";
 
 import { Anchor } from "src/anchor";
 import { StackIcon } from "src/icon/stackIcon";
-import { Pagination, ResultsPerPageLabel } from "src/pagination";
+import { Pagination } from "src/pagination";
 import { BooleanSetting } from "src/settings/booleanSetting";
 import { PageConfig, PageConfigItem } from "src/pageConfig";
 import {
@@ -28,13 +28,11 @@ import {
   SortSetting,
 } from "src/sortedtable";
 import * as format from "src/util/format";
+import { EncodeDatabaseUri } from "src/util/format";
 
 import styles from "./databasesPage.module.scss";
 import sortableTableStyles from "src/sortedtable/sortedtable.module.scss";
-import {
-  baseHeadingClasses,
-  statisticsClasses,
-} from "src/transactionsPage/transactionsPageClasses";
+import { baseHeadingClasses } from "src/transactionsPage/transactionsPageClasses";
 import { syncHistory, tableStatsClusterSetting, unique } from "src/util";
 import booleanSettingStyles from "../settings/booleanSetting.module.scss";
 import { CircleFilled } from "../icon";
@@ -43,9 +41,9 @@ import { Loading } from "../loading";
 import { Search } from "../search";
 import {
   calculateActiveFilters,
+  defaultFilters,
   Filter,
   Filters,
-  defaultFilters,
   handleFiltersFromQueryString,
 } from "../queryFilter";
 import { merge } from "lodash";
@@ -495,7 +493,7 @@ export class DatabasesPage extends React.Component<
       ),
       cell: database => (
         <Link
-          to={`/database/${database.name}`}
+          to={EncodeDatabaseUri(database.name)}
           className={cx("icon__container")}
         >
           <StackIcon className={cx("icon--s", "icon--primary")} />

diff --git a/pkg/ui/workspaces/cluster-ui/src/indexDetailsPage/indexDetailsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/indexDetailsPage/indexDetailsPage.tsx
@@ -34,6 +34,9 @@ import {
   calculateTotalWorkload,
   Count,
   DATE_FORMAT_24_UTC,
+  EncodeDatabaseTableIndexUri,
+  EncodeDatabaseTableUri,
+  EncodeDatabaseUri,
   performanceTuningRecipes,
 } from "../util";
 import {
@@ -258,15 +261,22 @@ export class IndexDetailsPage extends React.Component<
         items={[
           { link: "/databases", name: "Databases" },
           {
-            link: `/database/${this.props.databaseName}`,
+            link: EncodeDatabaseUri(this.props.databaseName),
             name: "Tables",
           },
           {
-            link: `/database/${this.props.databaseName}/table/${this.props.tableName}`,
+            link: EncodeDatabaseTableUri(
+              this.props.databaseName,
+              this.props.tableName,
+            ),
             name: `Table: ${this.props.tableName}`,
           },
           {
-            link: `/database/${this.props.databaseName}/table/${this.props.tableName}/index/${this.props.indexName}`,
+            link: EncodeDatabaseTableIndexUri(
+              this.props.databaseName,
+              this.props.tableName,
+              this.props.indexName,
+            ),
             name: `Index: ${this.props.indexName}`,
           },
         ]}

diff --git a/pkg/ui/workspaces/cluster-ui/src/insightsTable/insightsTable.tsx b/pkg/ui/workspaces/cluster-ui/src/insightsTable/insightsTable.tsx
@@ -19,12 +19,14 @@ import {
   clusterSettings,
   computeOrUseStmtSummary,
   Duration,
+  EncodeDatabasesToIndexUri,
+  EncodeDatabaseTableIndexUri,
   performanceBestPractices,
+  performanceTuningRecipes,
   statementsRetries,
 } from "../util";
 import { Anchor } from "../anchor";
 import { Link } from "react-router-dom";
-import { performanceTuningRecipes } from "../util";
 import { InsightRecommendation, insightType } from "../insights";
 
 const cx = classNames.bind(styles);
@@ -129,8 +131,17 @@ function descriptionCell(
   );
 
   const indexLink = isCockroachCloud
-    ? `databases/${insightRec.database}/${insightRec.indexDetails?.schema}/${insightRec.indexDetails?.table}/${insightRec.indexDetails?.indexName}`
-    : `database/${insightRec.database}/table/${insightRec.indexDetails?.table}/index/${insightRec.indexDetails?.indexName}`;
+    ? EncodeDatabasesToIndexUri(
+        insightRec.database,
+        insightRec.indexDetails?.schema,
+        insightRec.indexDetails?.table,
+        insightRec.indexDetails?.indexName,
+      )
+    : EncodeDatabaseTableIndexUri(
+        insightRec.database,
+        insightRec.indexDetails?.table,
+        insightRec.indexDetails?.indexName,
+      );
 
   switch (insightRec.type) {
     case "CreateIndex":

diff --git a/pkg/ui/workspaces/cluster-ui/src/util/format.ts b/pkg/ui/workspaces/cluster-ui/src/util/format.ts
@@ -290,3 +290,40 @@ export function capitalize(str: string): string {
   if (!str) return str;
   return str[0].toUpperCase() + str.substring(1);
 }
+
+export function EncodeUriName(name: string) {
+  return encodeURIComponent(name);
+}
+
+export function EncodeDatabasesUri(db: string) {
+  return `/databases/${EncodeUriName(db)}`;
+}
+
+export function EncodeDatabasesToIndexUri(
+  db: string,
+  schema: string,
+  table: string,
+  indexName: string,
+) {
+  return `${EncodeDatabasesUri(db)}/${EncodeUriName(schema)}/${EncodeUriName(
+    table,
+  )}/${EncodeUriName(indexName)}`;
+}
+
+export function EncodeDatabaseTableUri(db: string, table: string) {
+  return `${EncodeDatabaseUri(db)}/table/${EncodeUriName(table)}`;
+}
+
+export function EncodeDatabaseTableIndexUri(
+  db: string,
+  table: string,
+  indexName: string,
+) {
+  return `${EncodeDatabaseTableUri(db, table)}/index/${EncodeUriName(
+    indexName,
+  )}`;
+}
+
+export function EncodeDatabaseUri(db: string) {
+  return `/database/${EncodeUriName(db)}`;
+}
diff --git a/pkg/ui/workspaces/db-console/src/util/api.ts b/pkg/ui/workspaces/db-console/src/util/api.ts
@@ -357,9 +357,13 @@ export function getDatabaseDetails(
 ): Promise<DatabaseDetailsResponseMessage> {
   const queryString = req.include_stats ? "?include_stats=true" : "";
 
+  const promiseErr = IsValidateUriName(req.database);
+  if (promiseErr) {
+    return promiseErr;
+  }
   return timeoutFetch(
     serverpb.DatabaseDetailsResponse,
-    `${API_PREFIX}/databases/${req.database}${queryString}`,
+    `${API_PREFIX}/databases/${EncodeUriName(req.database)}${queryString}`,
     null,
     timeout,
   );
@@ -370,9 +374,16 @@ export function getTableDetails(
   req: TableDetailsRequestMessage,
   timeout?: moment.Duration,
 ): Promise<TableDetailsResponseMessage> {
+  const promiseErr = IsValidateUriName(req.database, req.table);
+  if (promiseErr) {
+    return promiseErr;
+  }
+
   return timeoutFetch(
     serverpb.TableDetailsResponse,
-    `${API_PREFIX}/databases/${req.database}/tables/${req.table}`,
+    `${API_PREFIX}/databases/${EncodeUriName(
+      req.database,
+    )}/tables/${EncodeUriName(req.table)}`,
     null,
     timeout,
   );
@@ -519,9 +530,16 @@ export function getTableStats(
   req: TableStatsRequestMessage,
   timeout?: moment.Duration,
 ): Promise<TableStatsResponseMessage> {
+  const promiseErr = IsValidateUriName(req.database, req.table);
+  if (promiseErr) {
+    return promiseErr;
+  }
+
   return timeoutFetch(
     serverpb.TableStatsResponse,
-    `${API_PREFIX}/databases/${req.database}/tables/${req.table}/stats`,
+    `${API_PREFIX}/databases/${EncodeUriName(
+      req.database,
+    )}/tables/${EncodeUriName(req.table)}/stats`,
     null,
     timeout,
   );
@@ -532,9 +550,16 @@ export function getIndexStats(
   req: IndexStatsRequestMessage,
   timeout?: moment.Duration,
 ): Promise<IndexStatsResponseMessage> {
+  const promiseErr = IsValidateUriName(req.database, req.table);
+  if (promiseErr) {
+    return promiseErr;
+  }
+
   return timeoutFetch(
     serverpb.TableIndexStatsResponse,
-    `${STATUS_PREFIX}/databases/${req.database}/tables/${req.table}/indexstats`,
+    `${STATUS_PREFIX}/databases/${EncodeUriName(
+      req.database,
+    )}/tables/${EncodeUriName(req.table)}/indexstats`,
     null,
     timeout,
   );
@@ -877,3 +902,20 @@ export function getKeyVisualizerSamples(
     timeout,
   );
 }
+
+export function IsValidateUriName(...args: string[]): Promise<any> {
+  for (const name of args) {
+    if (name.includes("/")) {
+      return Promise.reject(
+        new Error(
+          `util/api: The entity '${name}' contains '/' which is not currently supported in the UI.`,
+        ),
+      );
+    }
+  }
+  return null;
+}
+
+export function EncodeUriName(name: string): string {
+  return encodeURIComponent(name);
+}