Skip to content

Commit

Permalink
Make ComponentDefinitionPath more permissive, removing the need for e…
Browse files Browse the repository at this point in the history
…ncoding (#29668)

GitOrigin-RevId: e81ab6d44201db457889747e71d2bedfa2b8ed3e
  • Loading branch information
sujayakar authored and Convex, Inc. committed Sep 11, 2024
1 parent 45ec027 commit 09f2baa
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 66 deletions.
30 changes: 28 additions & 2 deletions crates/common/src/components/component_definition_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;

Expand All @@ -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).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [];
Expand Down Expand Up @@ -161,13 +160,12 @@ async function buildMountTree(
attributes: string[],
): Promise<MountTree | null> {
// 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = `
Expand Down Expand Up @@ -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) {
Expand Down

This file was deleted.

46 changes: 13 additions & 33 deletions npm-packages/convex/src/cli/lib/components/definition/bundle.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import path from "path";
import crypto from "crypto";
import {
ComponentDirectory,
ComponentDefinitionPath,
buildComponentDirectory,
isComponentDirectory,
qualifiedDefinitionPath,
toComponentDefinitionPath,
EncodedComponentDefinitionPath,
} from "./directoryStructure.js";
import {
Context,
Expand Down Expand Up @@ -145,9 +143,9 @@ function componentPlugin({
rootComponentDirectory,
imported,
);
const encodedPath = hackyMapping(encodeDefinitionPath(componentPath));
const importPath = definitionImportPath(componentPath);
return {
path: encodedPath,
path: importPath,
external: true,
};
}
Expand All @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand All @@ -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),
Expand Down Expand Up @@ -508,7 +487,7 @@ export async function bundleImplementations(
componentImplementations: {
schema: Bundle | null;
functions: Bundle[];
definitionPath: EncodedComponentDefinitionPath;
definitionPath: ComponentDefinitionPath;
}[];
}> {
let appImplementation;
Expand Down Expand Up @@ -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 });
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 3 additions & 6 deletions npm-packages/convex/src/cli/lib/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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.

Expand Down

0 comments on commit 09f2baa

Please sign in to comment.