fix(client): Fix URL parsing in the CLI (#1296)

This is just to demonstrate the direction in which URL parsing can be
improved in the CLI.

First, more test coverage is needed for some not-too-uncommon edge
cases.

Second, more uniformunity is needed in the parsing code. Currently, in
`src/cli/utils.ts` alone the database URL is parsed using a regexp, the
service URL is parsed using the deprecated `url.parse()` method. All of
this can be replaced with the built-in `URL` class and its
properties/method. That class can also be used for building URLs,
instead of ad-hoc string concatenation that doesn't escape URL
components.

---------

Co-authored-by: msfstef <msfstef@gmail.com>

.changeset/nasty-buses-wink.md

@@ -0,0 +1,5 @@
+---
+"electric-sql": patch
+---
+
+Consistently use `URL` API for parsing and constructing URLs in CLI.

.changeset/pretty-zoos-mate.md

@@ -0,0 +1,5 @@
+---
+"electric-sql": patch
+---
+
+Ensure default port numbers are used when starting Electric with CLI.

clients/typescript/src/cli/config-options.ts

@@ -6,7 +6,7 @@ import {
   getConfigValue,
   type ConfigMap,
 } from './config'
-import { dedent, getAppName, buildDatabaseURL, parsePgProxyPort } from './utils'
+import { dedent, getAppName, buildDatabaseURL, parsePgProxyPort } from './util'
 import { LIB_VERSION } from '../version'
 
 const minorVersion = LIB_VERSION.split('.').slice(0, 2).join('.')
@@ -122,7 +122,7 @@ export const configOptions: Record<string, any> = {
   DATABASE_PORT: {
     doc: 'Port number of the database server.',
     valueType: Number,
-    inferVal: (options: ConfigMap) => inferDbUrlPart('port', options),
+    inferVal: (options: ConfigMap) => inferDbUrlPart('port', options, 5432),
     defaultVal: 5432,
     groups: ['database'],
   },
@@ -229,7 +229,7 @@ export const configOptions: Record<string, any> = {
   },
   PG_PROXY_PORT: {
     inferVal: (options: ConfigMap) =>
-      inferProxyUrlPart('port', options)?.toString(),
+      inferProxyUrlPart('port', options, 65432)?.toString(),
     defaultVal: '65432',
     valueType: String,
     valueTypeName: 'port',

clients/typescript/src/cli/config.ts

@@ -1,5 +1,5 @@
 import type { Command } from 'commander'
-import { extractDatabaseURL, extractServiceURL } from './utils'
+import { extractDatabaseURL, extractServiceURL } from './util'
 import { configOptions } from './config-options'
 
 export type ConfigMap = Record<string, string | number | boolean>

clients/typescript/src/cli/docker-commands/command-psql.ts

@@ -4,7 +4,7 @@ import {
   getConfig,
   GetConfigOptionsForGroup,
 } from '../config'
-import { buildDatabaseURL, parsePgProxyPort } from '../utils'
+import { buildDatabaseURL, parsePgProxyPort } from '../util'
 import { dockerCompose } from './docker-utils'
 
 export function makePsqlCommand() {

clients/typescript/src/cli/docker-commands/command-start.ts

@@ -1,5 +1,5 @@
 import { Command } from 'commander'
-import { dedent, parsePgProxyPort } from '../utils'
+import { dedent, parsePgProxyPort } from '../util'
 import { addOptionGroupToCommand, getConfig, Config } from '../config'
 import { dockerCompose } from './docker-utils'
 
@@ -60,7 +60,7 @@ export function start(options: StartSettings) {
     // PG_PROXY_PORT can have a 'http:' prefix, which we need to remove
     // for port mapping to work.
     env.PG_PROXY_PORT_PARSED = parsePgProxyPort(
-      env.PG_PROXY_PORT
+      env.PG_PROXY_PORT as `${number}` | `http` | `http:${number}`
     ).port.toString()
 
     const dockerConfig = {

clients/typescript/src/cli/migrations/command-generate.ts

@@ -1,5 +1,5 @@
 import { Command, InvalidArgumentError } from 'commander'
-import { dedent } from '../utils'
+import { dedent } from '../util'
 import {
   generate,
   type GeneratorOptions,

clients/typescript/src/cli/migrations/migrate.ts

@@ -9,7 +9,7 @@ import http from 'node:http'
 import https from 'node:https'
 import Module from 'node:module'
 import path from 'path'
-import { buildDatabaseURL, parsePgProxyPort } from '../utils'
+import { buildDatabaseURL, parsePgProxyPort, appRoot } from '../util'
 import { buildMigrations, getMigrationNames } from './builder'
 import { findAndReplaceInFile } from '../util'
 import { getConfig, type Config } from '../config'
@@ -33,7 +33,6 @@ const generatorPath = path.join(
   'bin.js'
 )
 
-const appRoot = path.resolve() // path where the user ran `npx electric migrate`
 const sqliteMigrationsFileName = 'migrations.ts'
 const pgMigrationsFileName = 'pg-migrations.ts'
 

clients/typescript/src/cli/tunnel/command-proxy-tunnel.ts

@@ -2,7 +2,7 @@ import { Command } from 'commander'
 import net from 'net'
 import { WebSocket, createWebSocketStream } from 'ws'
 import { addOptionGroupToCommand, getConfig } from '../config'
-import { parsePort } from '../utils'
+import { parsePort } from '../util'
 
 export function makeProxyTunnelCommand() {
   const command = new Command('proxy-tunnel')

clients/typescript/src/cli/util/index.ts

@@ -1 +1,4 @@
 export * from './io'
+export * from './parse'
+export * from './string'
+export * from './paths'

clients/typescript/src/cli/util/parse.ts

@@ -0,0 +1,104 @@
+import fs from 'fs'
+import { InvalidArgumentError } from 'commander'
+import { appPackageJsonPath } from './paths'
+
+/**
+ * Get the name of the current project.
+ */
+export function getAppName(): string | undefined {
+  return JSON.parse(fs.readFileSync(appPackageJsonPath, 'utf8')).name
+}
+
+/**
+ * Parse an integer from a string and throw the given error
+ * if parsing fails
+ */
+function parseIntOrFail(str: string, error: string) {
+  const parsed = parseInt(str)
+  if (isNaN(parsed)) {
+    throw new InvalidArgumentError(error)
+  }
+  return parsed
+}
+
+export function parsePort(str: string): number {
+  return parseIntOrFail(
+    str,
+    `Invalid port: ${str}. Must be integer between 1 and 65535.`
+  )
+}
+
+export function parseTimeout(str: string): number {
+  return parseIntOrFail(str, `Invalid timeout: ${str}. Must be an integer.`)
+}
+
+export function extractDatabaseURL(url: string): {
+  user: string
+  password: string
+  host: string
+  port: number | null
+  dbName: string
+} {
+  const parsed = new URL(url)
+  if (!(parsed.protocol === 'postgres:' || parsed.protocol === 'postgresql:')) {
+    throw new Error(`Invalid database URL scheme: ${url}`)
+  }
+
+  const user = decodeURIComponent(parsed.username)
+  if (!user) {
+    throw new Error(`Invalid or missing username: ${url}`)
+  }
+
+  return {
+    user: user,
+    password: decodeURIComponent(parsed.password),
+    host: decodeURIComponent(parsed.hostname),
+    port: parsed.port ? parseInt(parsed.port) : null,
+    dbName: decodeURIComponent(parsed.pathname.slice(1)) || user,
+  }
+}
+
+export function extractServiceURL(serviceUrl: string): {
+  host: string
+  port: number | null
+} {
+  const parsed = new URL(serviceUrl)
+  if (!parsed.hostname) {
+    throw new Error(`Invalid service URL: ${serviceUrl}`)
+  }
+  return {
+    host: decodeURIComponent(parsed.hostname),
+    port: parsed.port ? parseInt(parsed.port) : null,
+  }
+}
+
+/**
+ * Parse the given string or number into a port number and whether
+ * it uses the HTTP proxy or not.
+ * @example
+ * ```
+ * parsePgProxyPort('65432') // { http: false, port: 65432 }
+ * parsePgProxyPort('http:5123') // { http: true, port: 5123 }
+ * parsePgProxyPort('http') // { http: true, port: 65432 }
+ * ```
+ */
+export function parsePgProxyPort(
+  str: number | `${number}` | `http` | `http:${number}`
+): {
+  http: boolean
+  port: number
+} {
+  if (typeof str === 'number') {
+    return { http: false, port: str }
+  } else if (str.includes(':')) {
+    const [prefix, port] = str.split(':')
+    return {
+      http: prefix.toLocaleLowerCase() === 'http',
+      port: parsePort(port),
+    }
+  } else if (str.toLocaleLowerCase() === 'http') {
+    return { http: true, port: 65432 }
+  } else {
+    return { http: false, port: parsePort(str) }
+  }
+}

clients/typescript/src/cli/util/paths.ts

@@ -0,0 +1,11 @@
+import path from 'path'
+
+/**
+ * Path where the user ran `npx electric`
+ */
+export const appRoot = path.resolve()
+
+/**
+ * Path to the package.json of the user app
+ */
+export const appPackageJsonPath = path.join(appRoot, 'package.json')

clients/typescript/src/cli/util/string.ts

@@ -0,0 +1,71 @@
+/**
+ * Tagged template literal dedent function that also unwraps lines.
+ * Double newlines become a single newline.
+ */
+export function dedent(
+  strings: TemplateStringsArray,
+  ...values: unknown[]
+): string {
+  let str = strings[0]
+  for (let i = 0; i < values.length; i++) {
+    str += String(values[i]) + strings[i + 1]
+  }
+
+  const lines = str.split('\n')
+
+  const minIndent = lines
+    .filter((line) => line.trim())
+    .reduce((minIndent, line) => {
+      const indent = line.match(/^\s*/)?.[0].length ?? 0
+      return indent < minIndent ? indent : minIndent
+    }, Infinity)
+
+  if (lines[0] === '') {
+    // if first line is empty, remove it
+    lines.shift()
+  }
+  if (lines[lines.length - 1] === '') {
+    // if last line is empty, remove it
+    lines.pop()
+  }
+
+  return lines
+    .map((line) => {
+      line = line.slice(minIndent)
+      if (/^\s/.test(line)) {
+        // if line starts with whitespace, prefix it with a newline
+        // to preserve the indentation
+        return '\n' + line
+      } else if (line === '') {
+        // if line is empty, we want a newline here
+        return '\n'
+      } else {
+        return line.trim() + ' '
+      }
+    })
+    .join('')
+    .trim()
+}
+
+/**
+ * Builds the Postgres database URL for the given parameters.
+ */
+export function buildDatabaseURL(opts: {
+  user: string
+  password: string
+  host: string
+  port: number
+  dbName: string
+  ssl?: boolean
+}): string {
+  const base = new URL(`postgresql://${opts.host}`)
+  base.username = opts.user
+  base.password = opts.password
+  base.port = opts.port.toString()
+  base.pathname = opts.dbName
+  if (opts.ssl !== undefined) {
+    base.searchParams.set('sslmode', opts.ssl ? 'require' : 'disable')
+  }
+
+  return base.toString()
+}