From 09f2baa1fe8e4210750e4c0bfd533cb920441d5a Mon Sep 17 00:00:00 2001 From: Sujay Jayakar Date: Wed, 11 Sep 2024 11:13:33 -0400 Subject: [PATCH] Make ComponentDefinitionPath more permissive, removing the need for encoding (#29668) GitOrigin-RevId: e81ab6d44201db457889747e71d2bedfa2b8ed3e --- .../components/component_definition_path.rs | 30 +++++++++++- .../cli/codegen_templates/component_api.ts | 6 +-- .../cli/codegen_templates/component_server.ts | 5 +- .../lib/components/definition/bundle.test.ts | 11 ----- .../cli/lib/components/definition/bundle.ts | 46 ++++++------------- .../definition/directoryStructure.ts | 7 --- npm-packages/convex/src/cli/lib/config.ts | 9 ++-- 7 files changed, 48 insertions(+), 66 deletions(-) delete mode 100644 npm-packages/convex/src/cli/lib/components/definition/bundle.test.ts diff --git a/crates/common/src/components/component_definition_path.rs b/crates/common/src/components/component_definition_path.rs index 775cf63f..77dcf8b5 100644 --- a/crates/common/src/components/component_definition_path.rs +++ b/crates/common/src/components/component_definition_path.rs @@ -8,7 +8,6 @@ use std::{ }; use anyhow::Context; -use sync_types::path::check_valid_path_component; // Path relative to a project's `convex/` directory for each component // definition's folder. This path is project-level and originates from @@ -36,6 +35,33 @@ impl ComponentDefinitionPath { } } +// Windows has a maximum path limit of 260 (UTF-16) codepoints, which then is at +// most 260 * 4 = 1040 bytes, which is then at most 1387 bytes encoded as +// base64. Since we encode component definition paths as base64 within our +// esbuild plugin, permit up to 2048 bytes here to be safe. +const MAX_DEFINITION_PATH_COMPONENT_LEN: usize = 2048; + +fn check_valid_definition_path_component(s: &str) -> anyhow::Result<()> { + if s.len() > MAX_DEFINITION_PATH_COMPONENT_LEN { + anyhow::bail!( + "Path component is too long ({} > maximum {}): {}...", + s.len(), + MAX_DEFINITION_PATH_COMPONENT_LEN, + &s[..s.len().min(32)] + ); + } + if s.is_empty() { + anyhow::bail!("Path component is empty"); + } + if !s + .chars() + .all(|c| c.is_ascii() && !c.is_ascii_control() && c != '/' && c != '\\') + { + anyhow::bail!("Path component {s} can only include non-control ASCII characters."); + } + Ok(()) +} + impl FromStr for ComponentDefinitionPath { type Err = anyhow::Error; @@ -47,7 +73,7 @@ impl FromStr for ComponentDefinitionPath { let s = c .to_str() .with_context(|| format!("Path {s} has an invalid Unicode character"))?; - check_valid_path_component(s)?; + check_valid_definition_path_component(s)?; }, // Component paths are allowed to have `..` (since they're relative from the root // component's source directory). diff --git a/npm-packages/convex/src/cli/codegen_templates/component_api.ts b/npm-packages/convex/src/cli/codegen_templates/component_api.ts index c54e0521..cd8596b9 100644 --- a/npm-packages/convex/src/cli/codegen_templates/component_api.ts +++ b/npm-packages/convex/src/cli/codegen_templates/component_api.ts @@ -15,7 +15,6 @@ import { import { Identifier } from "../lib/deployApi/types.js"; import { ComponentDefinitionPath } from "../lib/deployApi/paths.js"; import { resolveFunctionReference } from "./component_server.js"; -import { encodeDefinitionPath } from "../lib/components/definition/bundle.js"; export function componentApiJs() { const lines = []; @@ -161,13 +160,12 @@ async function buildMountTree( attributes: string[], ): Promise { // TODO make these types more precise when receiving analysis from server - const analysis = - startPush.analysis[encodeDefinitionPath(definitionPath as any)]; + const analysis = startPush.analysis[definitionPath]; if (!analysis) { return await ctx.crash({ exitCode: 1, errorType: "fatal", - printedMessage: `No analysis found for component ${encodeDefinitionPath(definitionPath as any)} orig: ${definitionPath}\nin\n${Object.keys(startPush.analysis).toString()}`, + printedMessage: `No analysis found for component ${definitionPath} orig: ${definitionPath}\nin\n${Object.keys(startPush.analysis).toString()}`, }); } let current = analysis.definition.exports.branch; diff --git a/npm-packages/convex/src/cli/codegen_templates/component_server.ts b/npm-packages/convex/src/cli/codegen_templates/component_server.ts index 49fcc0d7..bb4bec9f 100644 --- a/npm-packages/convex/src/cli/codegen_templates/component_server.ts +++ b/npm-packages/convex/src/cli/codegen_templates/component_server.ts @@ -17,7 +17,6 @@ import { Context } from "../../bundler/context.js"; import { CanonicalizedModulePath } from "../lib/deployApi/paths.js"; import { Value, jsonToConvex } from "../../values/value.js"; import { z } from "zod"; -import { encodeDefinitionPath } from "../lib/components/definition/bundle.js"; export function componentServerJS(): string { const result = ` @@ -277,12 +276,12 @@ export async function componentServerDTS( componentDirectory, ); - const analysis = startPush.analysis[encodeDefinitionPath(definitionPath)]; + const analysis = startPush.analysis[definitionPath]; if (!analysis) { return await ctx.crash({ exitCode: 1, errorType: "fatal", - printedMessage: `No analysis found for component ${encodeDefinitionPath(definitionPath as any)} orig: ${definitionPath}\nin\n${Object.keys(startPush.analysis).toString()}`, + printedMessage: `No analysis found for component ${definitionPath} orig: ${definitionPath}\nin\n${Object.keys(startPush.analysis).toString()}`, }); } for (const childComponent of analysis.definition.childComponents) { diff --git a/npm-packages/convex/src/cli/lib/components/definition/bundle.test.ts b/npm-packages/convex/src/cli/lib/components/definition/bundle.test.ts deleted file mode 100644 index 29ad434f..00000000 --- a/npm-packages/convex/src/cli/lib/components/definition/bundle.test.ts +++ /dev/null @@ -1,11 +0,0 @@ -import { test, expect, describe } from "vitest"; -import { encodeDefinitionPath } from "./bundle.js"; -import { ComponentDefinitionPath } from "./directoryStructure.js"; - -describe("encodeDefinitionPath", async () => { - test("Escaped definition paths are distinguishable from unescaped ones", () => { - const a = encodeDefinitionPath("foo/bar-baz" as ComponentDefinitionPath); - const b = encodeDefinitionPath("foo/bar_baz" as ComponentDefinitionPath); - expect(a).not.toEqual(b); - }); -}); diff --git a/npm-packages/convex/src/cli/lib/components/definition/bundle.ts b/npm-packages/convex/src/cli/lib/components/definition/bundle.ts index 9803f6db..c509261f 100644 --- a/npm-packages/convex/src/cli/lib/components/definition/bundle.ts +++ b/npm-packages/convex/src/cli/lib/components/definition/bundle.ts @@ -1,5 +1,4 @@ import path from "path"; -import crypto from "crypto"; import { ComponentDirectory, ComponentDefinitionPath, @@ -7,7 +6,6 @@ import { isComponentDirectory, qualifiedDefinitionPath, toComponentDefinitionPath, - EncodedComponentDefinitionPath, } from "./directoryStructure.js"; import { Context, @@ -145,9 +143,9 @@ function componentPlugin({ rootComponentDirectory, imported, ); - const encodedPath = hackyMapping(encodeDefinitionPath(componentPath)); + const importPath = definitionImportPath(componentPath); return { - path: encodedPath, + path: importPath, external: true, }; } @@ -157,8 +155,8 @@ function componentPlugin({ } /** The path on the deployment that identifier a component definition. */ -function hackyMapping(componentPath: EncodedComponentDefinitionPath): string { - return `./_componentDeps/${Buffer.from(componentPath).toString("base64").replace(/=+$/, "")}`; +function definitionImportPath(componentPath: ComponentDefinitionPath): string { + return `./_componentDeps/${Buffer.from(componentPath).toString("base64url")}`; } // Share configuration between the component definition discovery and bundling passes. @@ -328,26 +326,6 @@ async function findComponentDependencies( return { components, dependencyGraph }; } -// Each path component is less than 64 bytes and is limited to a-zA-Z0-9 -// This is the only version of the path the server will receive. -export function encodeDefinitionPath( - s: ComponentDefinitionPath, -): EncodedComponentDefinitionPath { - const components = s.split(path.sep); - return components - .map((s) => { - // some important characters to escape include @-+ . - const escaped = s.replace(/[^a-zA-Z0-9_]/g, "_"); - if (escaped.length <= 64 && escaped === s) { - // If escaping lost no information then use orig. - return escaped; - } - const hash = crypto.createHash("md5").update(s).digest("hex"); - return `${escaped.slice(0, 50)}${hash.slice(0, 14)}`; - }) - .join(path.sep) as EncodedComponentDefinitionPath; -} - // NB: If a directory linked to is not a member of the passed // componentDirectories array then there will be external links // with no corresponding definition bundle. @@ -453,8 +431,9 @@ export async function bundleDefinitions( const componentDefinitionSpecsWithoutImpls: ComponentDefinitionSpecWithoutImpls[] = componentBundles.map(({ directory, outputJs, outputJsMap }) => ({ - definitionPath: encodeDefinitionPath( - toComponentDefinitionPath(rootComponentDirectory, directory), + definitionPath: toComponentDefinitionPath( + rootComponentDirectory, + directory, ), origDefinitionPath: toComponentDefinitionPath( rootComponentDirectory, @@ -470,13 +449,13 @@ export async function bundleDefinitions( rootComponentDirectory, dependencyGraph, directory.definitionPath, - ).map(encodeDefinitionPath), + ), })); const appDeps = getDeps( rootComponentDirectory, dependencyGraph, appBundle.directory.definitionPath, - ).map(encodeDefinitionPath); + ); const appDefinitionSpecWithoutImpls: AppDefinitionSpecWithoutImpls = { definition: { path: path.relative(rootComponentDirectory.path, appBundle.outputJs.path), @@ -508,7 +487,7 @@ export async function bundleImplementations( componentImplementations: { schema: Bundle | null; functions: Bundle[]; - definitionPath: EncodedComponentDefinitionPath; + definitionPath: ComponentDefinitionPath; }[]; }> { let appImplementation; @@ -621,8 +600,9 @@ export async function bundleImplementations( } } // definitionPath is the canonical form - const definitionPath = encodeDefinitionPath( - toComponentDefinitionPath(rootComponentDirectory, directory), + const definitionPath = toComponentDefinitionPath( + rootComponentDirectory, + directory, ); componentImplementations.push({ definitionPath, schema, functions }); } diff --git a/npm-packages/convex/src/cli/lib/components/definition/directoryStructure.ts b/npm-packages/convex/src/cli/lib/components/definition/directoryStructure.ts index 3a17f10b..a9896385 100644 --- a/npm-packages/convex/src/cli/lib/components/definition/directoryStructure.ts +++ b/npm-packages/convex/src/cli/lib/components/definition/directoryStructure.ts @@ -123,13 +123,6 @@ export async function buildComponentDirectory( export type ComponentDefinitionPath = string & { __brand: "ComponentDefinitionPath"; }; -/** - * EncodedComponentDefinitionPath is the identifier of a component definition - * sent to the server. - */ -export type EncodedComponentDefinitionPath = string & { - __brand: "EncodedComponentDefinitionPath"; -}; export function toComponentDefinitionPath( rootComponent: ComponentDirectory, diff --git a/npm-packages/convex/src/cli/lib/config.ts b/npm-packages/convex/src/cli/lib/config.ts index 5792ea20..a4d8cbad 100644 --- a/npm-packages/convex/src/cli/lib/config.ts +++ b/npm-packages/convex/src/cli/lib/config.ts @@ -36,10 +36,7 @@ import { promisify } from "util"; import zlib from "zlib"; import { recursivelyDelete } from "./fsUtils.js"; import { NodeDependency } from "./deployApi/modules.js"; -import { - ComponentDefinitionPath, - EncodedComponentDefinitionPath, -} from "./components/definition/directoryStructure.js"; +import { ComponentDefinitionPath } from "./components/definition/directoryStructure.js"; export { productionProvisionHost, provisionHost } from "./utils/utils.js"; const brotli = promisify(zlib.brotliCompress); @@ -634,11 +631,11 @@ interface BundledModuleInfo { */ export type ComponentDefinitionSpec = { /** This path is relative to the app (root component) directory. */ - definitionPath: EncodedComponentDefinitionPath; + definitionPath: ComponentDefinitionPath; /** This path is relative to the app (root component) directory. */ origDefinitionPath: ComponentDefinitionPath; /** Dependencies are paths to the directory of the dependency component definition from the app (root component) directory */ - dependencies: EncodedComponentDefinitionPath[]; + dependencies: ComponentDefinitionPath[]; // All other paths are relative to the directory of the definitionPath above.