From 2ba63c6af8d0d288e6c34b55b3bcb2e7d2944b9c Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 12 Nov 2026 11:56:51 +0000 Subject: [PATCH 01/11] feat: Windows and macOS Intel wheels and CLI Build sqlload and sqlload-cli wheels for Windows x86_64 (MSVC) and macOS x86_64 (macos-15-intel), and test Windows in CI. - librdkafka builds with CMake on Windows, where its configure script and make are missing. - Local paths: drive paths (C:\data), UNC shares (\\server\share), \\?\ paths and file:///C:/ and file://server/share URLs are local folders on Windows; Unix paths are unchanged. - A postgres_cdc replication session's relay on Windows is a 127.0.0.1 port, since the replication client cannot reach a socket file there. It relays only the connection whose startup message carries a random per-session secret, drops every other connection before a byte reaches the server, gives each 5 seconds, and never forwards the secret. - Verify gains a Windows job: unit tests, then the executable end to end over drive paths, a UNC share, psqlODBC against the runner's Postgres, and a postgres_cdc load streamed through the relay. - Publish builds and smoke-tests every wheel on pull requests that change packaging, and publishes only when dispatched. --- .github/scripts/windows_e2e.py | 208 ++++++++++++ .github/workflows/publish.yml | 51 ++- .github/workflows/verify.yml | 59 +++- Cargo.lock | 10 + crates/sqlload/Cargo.toml | 5 + .../src/adapters/files/main/table_path.rs | 9 +- .../sqlload/src/cdc/_helpers/snapshot_log.rs | 8 +- crates/sqlload/src/cdc/_helpers/stream.rs | 15 +- crates/sqlload/src/copy/_helpers/endpoints.rs | 3 +- .../_helpers/loopback_relay.rs | 135 ++++++++ .../src/postgres_source/_helpers/mod.rs | 2 + .../sqlload/src/postgres_source/constants.rs | 12 + .../src/postgres_source/main/relay_tls.rs | 45 ++- crates/sqlload/src/postgres_source/models.rs | 10 + crates/sqlload/src/postgres_source/tests.rs | 2 + .../src/postgres_source/tests/helpers.rs | 46 +++ .../tests/test_loopback_relay.rs | 304 ++++++++++++++++++ .../src/postgres_source/tests/test_types.rs | 7 + .../src/store_paths/_helpers/local_root.rs | 55 ++++ .../src/store_paths/_helpers/locations.rs | 22 +- .../sqlload/src/store_paths/_helpers/mod.rs | 1 + .../src/store_paths/main/file_url_path.rs | 10 + .../src/store_paths/main/local_separators.rs | 10 + crates/sqlload/src/store_paths/main/mod.rs | 2 + .../src/store_paths/main/resolve_location.rs | 25 +- crates/sqlload/src/store_paths/tests.rs | 2 + .../src/store_paths/tests/test_local_root.rs | 110 +++++++ .../tests/test_resolve_location.rs | 83 +++++ .../sqlload/tests/loader/test_postgres_tls.rs | 1 + website/src/content/docs/docs/cli/copy.mdx | 2 + .../src/content/docs/docs/configuration.mdx | 2 +- website/src/content/docs/docs/install.mdx | 3 +- .../content/docs/docs/sources/postgres.mdx | 24 +- .../content/docs/docs/supported-systems.mdx | 3 +- 34 files changed, 1239 insertions(+), 47 deletions(-) create mode 100644 .github/scripts/windows_e2e.py create mode 100644 crates/sqlload/src/postgres_source/_helpers/loopback_relay.rs create mode 100644 crates/sqlload/src/postgres_source/tests/test_loopback_relay.rs create mode 100644 crates/sqlload/src/store_paths/_helpers/local_root.rs create mode 100644 crates/sqlload/src/store_paths/main/file_url_path.rs create mode 100644 crates/sqlload/src/store_paths/main/local_separators.rs create mode 100644 crates/sqlload/src/store_paths/tests/test_local_root.rs diff --git a/.github/scripts/windows_e2e.py b/.github/scripts/windows_e2e.py new file mode 100644 index 00000000..4a555ce1 --- /dev/null +++ b/.github/scripts/windows_e2e.py @@ -0,0 +1,208 @@ +"""End-to-end checks of a Windows sqlload executable: drive paths, a UNC share and an ODBC driver. + +Usage: python windows_e2e.py SQLLOAD_EXE WORK_DIR SHARE [--odbc-driver NAME] [--cdc] + +WORK_DIR is a local folder (C:\\...) whose `share` folder is shared as SHARE (\\\\localhost\\name), +so what the share holds can also be read locally. With --odbc-driver, a table in the local Postgres (the runner's +preinstalled server, user postgres) is loaded through that ODBC driver, then loaded again +incrementally. With --cdc, a postgres_cdc source snapshots a table and then streams its changes, which +on Windows go through the loopback TLS relay (that server needs wal_level = logical). Needs the +duckdb Python package to read the results back. +""" + +import argparse +import os +import pathlib +import subprocess +import sys + +import duckdb + +ROWS = [(1, "first", "2026-01-01 10:00:00"), (2, "second", "2026-01-02 11:30:00"), (3, "third", "2026-01-03 12:45:30")] + + +def sqlload(exe, *args, config=None): + command = [exe, "--no-color"] + if config is not None: + command += ["--config", str(config)] + command += list(args) + print("$", subprocess.list2cmdline(command), flush=True) + result = subprocess.run(command, capture_output=True, text=True, encoding="utf-8", errors="replace") + print(result.stdout + result.stderr, flush=True) + if result.returncode != 0: + sys.exit(f"sqlload exited {result.returncode}") + return result.stdout + result.stderr + + +def query(database, sql): + with duckdb.connect(str(database), read_only=True) as connection: + return connection.execute(sql).fetchall() + + +def expect(found, expected, what): + if found != expected: + sys.exit(f"{what}: found {found!r}, expected {expected!r}") + print(f"ok: {what}", flush=True) + + +def files_checks(exe, work, share): + source = work / "in" / "orders" + source.mkdir(parents=True, exist_ok=True) + lines = ["id,name,updated_at"] + [f"{i},{name},{at}" for i, name, at in ROWS] + (source / "orders.csv").write_text("\n".join(lines) + "\n", encoding="utf-8") + database = work / "shop.duckdb" + + # A drive path with backslashes to a DuckDB file on a drive path. + sqlload(exe, "copy", str(source), f"duckdb://{database}#raw.orders", "--format", "csv") + expect(query(database, "SELECT count(*), max(name) FROM raw.orders"), [(3, "third")], "drive folder to DuckDB") + + # The same folder into Parquet files on the UNC share. + lake = f"{share}\\lake\\orders" + sqlload(exe, "copy", str(source), lake, "--format", "csv") + written = sorted(path.name for path in (work / "share" / "lake" / "orders").rglob("*.parquet")) + if not written: + sys.exit(f"no Parquet files under {lake}") + print(f"ok: UNC share holds {written}", flush=True) + + # Read back through a file:// URL naming the share, and through a \\?\UNC\ path. + server_share = share.lstrip("\\").replace("\\", "/") + sqlload(exe, "copy", f"file://{server_share}/lake/orders", f"duckdb://{database}#raw.from_share", "--format", "parquet") + expect(query(database, "SELECT count(*), sum(id) FROM raw.from_share"), [(3, 6)], "file:// share URL to DuckDB") + verbatim = "\\\\?\\UNC\\" + share.lstrip("\\") + "\\lake\\orders" + sqlload(exe, "copy", verbatim, f"duckdb://{database}#raw.from_verbatim", "--format", "parquet") + expect(query(database, "SELECT count(*) FROM raw.from_verbatim"), [(3,)], "\\\\?\\UNC\\ path to DuckDB") + + +def psql(sql): + psql_exe = pathlib.Path(os.environ["PGBIN"]) / "psql.exe" + subprocess.run( + [str(psql_exe), "-v", "ON_ERROR_STOP=1", "-h", "127.0.0.1", "-U", "postgres", "-d", "postgres", "-c", sql], + check=True, + env={**os.environ, "PGPASSWORD": "root"}, + ) + + +def odbc_checks(exe, work, driver): + psql( + "DROP SCHEMA IF EXISTS erp CASCADE; CREATE SCHEMA erp;" + " CREATE TABLE erp.items (id bigint PRIMARY KEY, name text, price numeric(10,2), updated_at timestamp NOT NULL);" + " INSERT INTO erp.items SELECT g, 'item ' || g, g * 1.25, TIMESTAMP '2026-01-01' + g * INTERVAL '1 minute' FROM generate_series(1, 500) g;" + ) + database = work / "odbc.duckdb" + project = work / "odbc" / "sqlload.toml" + project.parent.mkdir(parents=True, exist_ok=True) + # TOML literal strings, so the Windows path keeps its backslashes. + project.write_text( + f""" +[connections.erp] +kind = "odbc" +connection_string = "Driver={{{driver}}};Server=127.0.0.1;Port=5432;Database=postgres;Uid=postgres;Pwd=root;" + +[connections.local] +kind = "duckdb" +database = '{database}' + +[targets.dev] +connection = "local" +loader_schema = "raw" + +[sources.erp] +kind = "odbc" +connection = "erp" +description = "Items of a generic ERP." +include = ["erp.items"] + +[sources.erp.tables."erp.items"] +description = "Items, changed rows found by updated_at." +cursor_column = "updated_at" +unique_key = ["id"] +write_strategy = "merge" +""", + encoding="utf-8", + ) + sqlload(exe, "load", config=project) + expect(query(database, "SELECT count(*), sum(price) FROM raw.erp__items"), [(500, 156562.50)], "ODBC first load") + psql( + "UPDATE erp.items SET name = 'changed', updated_at = TIMESTAMP '2027-01-01 00:00:00.25' WHERE id = 7;" + " INSERT INTO erp.items VALUES (501, 'new', 1, TIMESTAMP '2027-01-01 00:00:00.5');" + ) + sqlload(exe, "load", config=project) + expect( + query(database, "SELECT count(*), (SELECT name FROM raw.erp__items WHERE id = 7), max(updated_at)::VARCHAR FROM raw.erp__items"), + [(501, "changed", "2027-01-01 00:00:00.5")], + "ODBC incremental merge on a fractional-second cursor", + ) + + +def cdc_checks(exe, work): + """A postgres_cdc load streams through the loopback relay: the server offers no TLS, so under the default sslmode (prefer) the replication session is relayed in plain text.""" + psql( + "DROP SCHEMA IF EXISTS shop CASCADE; CREATE SCHEMA shop;" + " CREATE TABLE shop.orders (id bigint PRIMARY KEY, status text, total numeric(10,2));" + " INSERT INTO shop.orders SELECT g, 'new', g FROM generate_series(1, 100) g;" + ) + database = work / "cdc.duckdb" + project = work / "cdc" / "sqlload.toml" + project.parent.mkdir(parents=True, exist_ok=True) + project.write_text( + f""" +[connections.pg] +kind = "postgres" +host = "127.0.0.1" +port = 5432 +user = "postgres" +password = "${{ENV:PGPASSWORD}}" +database = "postgres" + +[connections.local] +kind = "duckdb" +database = '{database}' + +[targets.dev] +connection = "local" +loader_schema = "raw" + +[sources.shop] +kind = "postgres_cdc" +connection = "pg" +include = ["shop.*"] +slot = "sqlload_windows_e2e" +publication = "sqlload_windows_e2e" + +[sources.shop.tables."shop.orders"] +description = "Orders, captured." +""", + encoding="utf-8", + ) + os.environ["PGPASSWORD"] = "root" + sqlload(exe, "load", config=project) + expect(query(database, "SELECT count(*) FROM raw.shop__orders"), [(100,)], "CDC snapshot") + psql("UPDATE shop.orders SET status = 'paid' WHERE id <= 10; DELETE FROM shop.orders WHERE id = 100; INSERT INTO shop.orders VALUES (101, 'new', 5);") + sqlload(exe, "load", config=project) + expect( + query( + database, + "SELECT count(*) FILTER (WHERE status = 'paid'), count(*) FILTER (WHERE id = 101) FROM raw.shop__orders", + ), + [(10, 1)], + "CDC changes streamed through the loopback relay", + ) + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument("exe") + parser.add_argument("work", type=pathlib.Path) + parser.add_argument("share") + parser.add_argument("--odbc-driver") + parser.add_argument("--cdc", action="store_true") + arguments = parser.parse_args() + files_checks(arguments.exe, arguments.work, arguments.share) + if arguments.odbc_driver: + odbc_checks(arguments.exe, arguments.work, arguments.odbc_driver) + if arguments.cdc: + cdc_checks(arguments.exe, arguments.work) + + +if __name__ == "__main__": + main() diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 6ed94151..dea80311 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -7,6 +7,15 @@ on: description: "Release tag to publish, for example v0.1.0" required: true type: string + # A pull request changing how wheels are built builds and smoke-tests them all, and publishes + # nothing. + pull_request: + paths: + - .github/workflows/publish.yml + - .github/scripts/assert_*_wheel_tags.sh + - packages/*/pyproject.toml + - crates/sqlload/Cargo.toml + - crates/sqlload-python/Cargo.toml permissions: contents: read @@ -22,6 +31,10 @@ jobs: - name: Resolve release tag id: release-tag run: | + if [[ "${{ github.event_name }}" == "pull_request" ]]; then + echo "A pull request: building its head, publishing nothing." + exit 0 + fi tag="${{ inputs.tag }}" if [[ ! "$tag" =~ ^v0\.[0-9]+\.[0-9]+$ ]]; then @@ -41,6 +54,7 @@ jobs: with: ref: ${{ needs.release-tag.outputs.tag }} - name: Match package versions to release tag + if: github.event_name == 'workflow_dispatch' env: RELEASE_VERSION: ${{ needs.release-tag.outputs.version }} run: | @@ -84,6 +98,13 @@ jobs: - name: macos-arm64 runner: macos-latest target: aarch64 + # GitHub's last Intel macOS image, available until August 2027. + - name: macos-x86_64 + runner: macos-15-intel + target: x86_64 + - name: windows-x86_64 + runner: windows-latest + target: x86_64 steps: - uses: actions/checkout@v5 with: @@ -94,6 +115,17 @@ jobs: - uses: dtolnay/rust-toolchain@master with: toolchain: 1.95.0 + # Vendored OpenSSL's Configure needs a native Windows Perl; Git Bash's lacks modules it loads. + # Without nasm it builds with no-asm. + - name: Use Strawberry Perl for vendored OpenSSL + if: runner.os == 'Windows' + shell: pwsh + run: '"OPENSSL_SRC_PERL=C:\Strawberry\perl\bin\perl.exe" >> $env:GITHUB_ENV' + # The arm64 wheel's floor already; the Intel one would otherwise get Rust's (10.12), older than + # the C++ library DuckDB's bundled build needs. + - name: Build for macOS 11 and newer + if: runner.os == 'macOS' + run: echo "MACOSX_DEPLOYMENT_TARGET=11.0" >> "$GITHUB_ENV" # sqlload-cli ships one standalone Rust executable (OpenSSL built in) and links no libpython, # so maturin tags it py3-none-. The tag is asserted before release. - name: Build native CLI wheel @@ -124,13 +156,20 @@ jobs: - name: Assert Python extension wheel targets the stable ABI shell: bash run: bash .github/scripts/assert_python_wheel_tags.sh dist - - name: Smoke-test the Python extension wheel + - name: Smoke-test the wheels shell: bash run: | - python -m venv /tmp/smoke - /tmp/smoke/bin/pip install --quiet dist/sqlload-*.whl - /tmp/smoke/bin/sqlload --help - /tmp/smoke/bin/python -c "import sqlload, sqlload._native" + python -m venv smoke + bin=smoke/bin + if [[ "${RUNNER_OS}" == "Windows" ]]; then bin=smoke/Scripts; fi + "${bin}/python" -m pip install --quiet dist/sqlload-*.whl + "${bin}/sqlload" --help + "${bin}/python" -c "import sqlload, sqlload._native" + python -m venv smoke-cli + cli=smoke-cli/bin + if [[ "${RUNNER_OS}" == "Windows" ]]; then cli=smoke-cli/Scripts; fi + "${cli}/python" -m pip install --quiet dist/sqlload_cli-*.whl + "${cli}/sqlload" --version - uses: actions/upload-artifact@v5 with: name: wheel-${{ matrix.platform.name }} @@ -166,6 +205,7 @@ jobs: pypi: name: Publish to PyPI + if: github.event_name == 'workflow_dispatch' needs: - release-tag - wheels @@ -200,6 +240,7 @@ jobs: crates: name: Publish to crates.io + if: github.event_name == 'workflow_dispatch' needs: - release-tag - wheels diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index bc06eae3..4e1d80c2 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -146,6 +146,60 @@ jobs: df -h docker system df + # The unit tests on Windows, then the executable end to end: drive paths, a UNC share, the + # psqlODBC driver against the runner's own Postgres, and a postgres_cdc load streaming through + # the loopback relay Windows uses instead of a socket file. + windows: + name: Windows + needs: changes + if: needs.changes.outputs.code == 'true' + runs-on: windows-2025 + # A cold cache compiles DuckDB from source with MSVC (about 40 minutes); warm runs take a few. + timeout-minutes: 90 + env: + E2E_DIR: C:\sqlload-e2e + steps: + - uses: actions/checkout@v5 + - uses: dtolnay/rust-toolchain@master + with: + toolchain: 1.95.0 + - uses: Swatinem/rust-cache@v2 + with: + shared-key: windows + - name: Remove incomplete DuckDB sources + shell: bash + run: | + for dir in target/duckdb-sources/*/; do + [ -d "$dir" ] && [ ! -e "$dir.complete" ] && echo "removing incomplete $dir" && rm -rf "$dir" + done; true + - name: Unit tests + run: cargo test -p sqlload --lib --features odbc --quiet + - name: Build the executable + run: cargo build -p sqlload --features odbc --bin sqlload --quiet + - name: Start Postgres with logical replication + shell: pwsh + run: | + Add-Content -Path "$env:PGDATA\postgresql.conf" -Value "wal_level = logical" + Set-Service -Name postgresql-x64-17 -StartupType Manual + Start-Service -Name postgresql-x64-17 + - name: Install psqlODBC + shell: pwsh + run: | + $msi = "$env:RUNNER_TEMP\psqlodbc_x64.msi" + Invoke-WebRequest -Uri https://ftp.postgresql.org/pub/odbc/releases/REL-18_00_0006/psqlodbc_x64.msi -OutFile $msi + Start-Process msiexec.exe -ArgumentList "/i", $msi, "/qn", "/norestart" -Wait + Get-OdbcDriver -Name "PostgreSQL*" -Platform 64-bit | Format-Table Name, Platform + - name: Share a folder as \\localhost\sqlload-e2e + shell: pwsh + run: | + New-Item -ItemType Directory -Force -Path "$env:E2E_DIR\share" | Out-Null + New-SmbShare -Name sqlload-e2e -Path "$env:E2E_DIR\share" -FullAccess Everyone | Out-Null + - name: End-to-end checks + shell: pwsh + run: | + python -m pip install --quiet duckdb + python .github/scripts/windows_e2e.py target\debug\sqlload.exe $env:E2E_DIR \\localhost\sqlload-e2e --odbc-driver "PostgreSQL Unicode(x64)" --cdc + # Postgres to DuckDB, 1M narrow rows, sqlload alone with a release build: a wall-time figure # in the job summary, not a gate (shared runners are too noisy to gate on time), so Verify # does not wait on it. @@ -256,7 +310,7 @@ jobs: verify: name: Verify if: always() - needs: [changes, checks, containers, perf] + needs: [changes, checks, containers, perf, windows] runs-on: ubuntu-24.04 steps: - name: Confirm every required job passed or was skipped for a docs-only change @@ -265,12 +319,13 @@ jobs: CHECKS_RESULT: ${{ needs.checks.result }} CONTAINERS_RESULT: ${{ needs.containers.result }} PERF_RESULT: ${{ needs.perf.result }} + WINDOWS_RESULT: ${{ needs.windows.result }} run: | if [[ "${CHANGES_RESULT}" != "success" ]]; then echo "Scope classification: ${CHANGES_RESULT}" >&2 exit 1 fi - for result in "${CHECKS_RESULT}" "${CONTAINERS_RESULT}" "${PERF_RESULT}"; do + for result in "${CHECKS_RESULT}" "${CONTAINERS_RESULT}" "${PERF_RESULT}" "${WINDOWS_RESULT}"; do case "${result}" in success|skipped) ;; *) diff --git a/Cargo.lock b/Cargo.lock index 87608b56..fff20f99 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -979,6 +979,15 @@ version = "1.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1c133bc6a41be0d194c306b5506d15e6feeea7b1d6604bd3f8310dfb2ca96486" +[[package]] +name = "cmake" +version = "0.1.58" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c0f78a02292a74a88ac736019ab962ece0bc380e3f977bf72e376c5d78ff0678" +dependencies = [ + "cc", +] + [[package]] name = "cmov" version = "0.5.4" @@ -4558,6 +4567,7 @@ version = "4.10.0+2.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e234cf318915c1059d4921ef7f75616b5219b10b46e9f3a511a15eb4b56a3f77" dependencies = [ + "cmake", "libc", "libz-sys", "num_enum", diff --git a/crates/sqlload/Cargo.toml b/crates/sqlload/Cargo.toml index 062dcbdf..9234ff38 100644 --- a/crates/sqlload/Cargo.toml +++ b/crates/sqlload/Cargo.toml @@ -100,6 +100,11 @@ zstd.workspace = true [target.'cfg(any(target_os = "linux", target_os = "macos"))'.dependencies] rustix.workspace = true +# librdkafka's default build runs its mklove `configure` script and `make`, which MSVC has +# neither of; on Windows it is built with CMake instead. +[target.'cfg(windows)'.dependencies] +rdkafka = { workspace = true, features = ["cmake-build"] } + [dev-dependencies] bytes.workspace = true arrow-array = { workspace = true, features = ["chrono-tz"] } diff --git a/crates/sqlload/src/adapters/files/main/table_path.rs b/crates/sqlload/src/adapters/files/main/table_path.rs index c761ab3d..d5a77935 100644 --- a/crates/sqlload/src/adapters/files/main/table_path.rs +++ b/crates/sqlload/src/adapters/files/main/table_path.rs @@ -3,12 +3,13 @@ use crate::adapters::files::_helpers::layout::template::partition_folder; use crate::adapters::files::constants::{CURSOR_PLACEHOLDER, CURSOR_PLACEHOLDER_START}; use crate::adapters::files::models::{TablePath, TablePathRequest}; +use crate::store_paths::main::local_separators::local_separators; use crate::store_paths::main::render_template::render_template; use crate::store_paths::main::resolve_location::resolve_location; /// The table's folder and partition folder: placeholders filled, the cursor allowed only in the last folder. pub fn table_path(request: &TablePathRequest<'_>) -> Result { - let template = request.template; + let template = &*local_separators(request.template); if template.contains("://") { return Err(format!( "path {template:?} must be relative to connection {:?}, not a URL", @@ -42,10 +43,8 @@ pub fn table_path(request: &TablePathRequest<'_>) -> Result { )); } }; - let root = match template.starts_with('/') { - true => "/", - false => "", - }; + // `/`, or `//` before a Windows share; a drive (`C:`) is the first folder. + let root = &template[..template.len() - template.trim_start_matches('/').len()]; let rendered = render_template(&format!("{root}{}", folder_segments.join("/")), &values) .map_err(|error| error.to_string())?; let folder = resolve_location(Some((request.connection, request.store)), &rendered) diff --git a/crates/sqlload/src/cdc/_helpers/snapshot_log.rs b/crates/sqlload/src/cdc/_helpers/snapshot_log.rs index 92f917f5..d944e2cd 100644 --- a/crates/sqlload/src/cdc/_helpers/snapshot_log.rs +++ b/crates/sqlload/src/cdc/_helpers/snapshot_log.rs @@ -15,6 +15,7 @@ use crate::file_store::main::read_file::read_file; use crate::file_store::main::write_file::write_file; use crate::loader::errors::LoaderError; use crate::schema_plan::constants::CDC_LSN_COLUMN; +use crate::store_paths::main::local_separators::local_separators; use crate::store_paths::models::Location; use crate::time_text::main::now_utc_text::now_utc_text; @@ -205,10 +206,9 @@ impl SnapshotLog<'_> { /// The records folder of `source`'s slot `slot` in a files target whose path template is `template`: the template's folders up to the first one naming anything but `{source}`, with `{source}` filled (all but the last when every one is fixed, the table's own folder then), then `_sqlload/cdc///`. Outside every table's folder, and the same whichever tables the source selects; a table's own `destination_path` does not move it. pub(crate) fn snapshot_folder(template: &str, source: &str, slot: &str) -> String { - let root = match template.starts_with('/') { - true => "/", - false => "", - }; + let template = &*local_separators(template); + // `/`, or `//` before a Windows share; a drive (`C:`) is the first folder. + let root = &template[..template.len() - template.trim_start_matches('/').len()]; let segments: Vec = template .split('/') .filter(|part| !part.is_empty()) diff --git a/crates/sqlload/src/cdc/_helpers/stream.rs b/crates/sqlload/src/cdc/_helpers/stream.rs index 2c73ed05..50fc1d6a 100644 --- a/crates/sqlload/src/cdc/_helpers/stream.rs +++ b/crates/sqlload/src/cdc/_helpers/stream.rs @@ -44,6 +44,7 @@ use crate::loader::models::{ CdcSpec, CommitRule, LoadRun, PlanOptions, ProjectPlan, Publisher, TableLoaded, TableOutcome, TablePlan, WholeReason, }; +use crate::postgres_source::constants::APPLICATION_NAME; use crate::postgres_source::constants::DEFAULT_BATCH_BYTES; use crate::postgres_source::errors::PostgresError; use crate::postgres_source::main::connect_postgres::connect_postgres; @@ -145,19 +146,27 @@ impl CdcSource<'_> { async fn replication_config(&self) -> Result { let address = self.address()?; let relay = relay_tls(&address).await?; + let (host, port, application_name) = match &relay { + Some(relay) => ( + relay.host.as_str(), + relay.port, + relay.application_name.expose(), + ), + None => (address.host, address.port, APPLICATION_NAME), + }; Ok(ReplicationConfig::new( - relay.as_deref().unwrap_or(address.host), + host, address.user, address.password.unwrap_or_default(), address.database, self.cdc.slot.clone(), self.cdc.publication.clone(), ) - .with_port(address.port) + .with_port(port) .with_tls(TlsConfig::disabled()) .with_status_interval(STATUS_INTERVAL) .with_options(REPLICATION_OPTIONS) - .with_application_name("sqlload")) + .with_application_name(application_name)) } /// The source's snapshot records: beside its first table in a warehouse, in the records folder of its path template in a files target (see [`snapshot_folder`]). diff --git a/crates/sqlload/src/copy/_helpers/endpoints.rs b/crates/sqlload/src/copy/_helpers/endpoints.rs index 5f4fb3f3..6a3c0e8e 100644 --- a/crates/sqlload/src/copy/_helpers/endpoints.rs +++ b/crates/sqlload/src/copy/_helpers/endpoints.rs @@ -4,6 +4,7 @@ use toml::{Table, Value}; use crate::copy::constants::{POSTGRES_SETTINGS, REDACTED, S3_SETTINGS}; use crate::copy::models::{Endpoint, Role}; +use crate::store_paths::main::file_url_path::file_url_path; /// The endpoint `text` names at `role`: `connection:locator` for a connection in `connections` (or a bare name), a URI, or a local folder; every message shows `text` with its password redacted. pub(crate) fn endpoint(text: &str, role: Role, connections: &Table) -> Result { @@ -123,7 +124,7 @@ fn uri_endpoint(shown: &str, scheme: &str, rest: &str, role: Role) -> Result s3(shown, body, role), - "file" => Ok(folder(body, shown)), + "file" => Ok(folder(&file_url_path(body), shown)), "snowflake" => Err(format!( "the {} {shown}: a Snowflake connection needs auth and a load_method, which a URI does not carry; declare it under [connections] in sqlload.toml and name it, as warehouse:SCHEMA.TABLE", role.label() diff --git a/crates/sqlload/src/postgres_source/_helpers/loopback_relay.rs b/crates/sqlload/src/postgres_source/_helpers/loopback_relay.rs new file mode 100644 index 00000000..e4b2e073 --- /dev/null +++ b/crates/sqlload/src/postgres_source/_helpers/loopback_relay.rs @@ -0,0 +1,135 @@ +//! A relay on a loopback port, where the replication client cannot reach a private socket (Windows). Any local user can connect to the port, so the relay forwards the session only to the connection whose startup message names this relay's secret as its application name, and nothing reaches the server before that check passes. + +use std::net::Ipv4Addr; + +use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt}; +use tokio::net::{TcpListener, TcpStream}; +use tokio::task::JoinSet; + +use crate::auth::models::Secret; +use crate::postgres_source::constants::{ + APPLICATION_NAME, PROTOCOL_VERSION, RELAY_ACCEPT_TIMEOUT, RELAY_STARTUP_TIMEOUT, + STARTUP_MESSAGE_LIMIT, +}; +use crate::postgres_source::models::RelayEndpoint; + +/// A port on 127.0.0.1 that pipes `server` to the first connection presenting the returned application name; it closes once that connection is admitted, or after [`RELAY_ACCEPT_TIMEOUT`] without one. Other connections are dropped, each after [`RELAY_STARTUP_TIMEOUT`] at most, without a byte of theirs reaching the server. +pub(crate) async fn loopback_relay(server: S) -> Result +where + S: AsyncRead + AsyncWrite + Unpin + Send + 'static, +{ + let mut secret = [0u8; 32]; + getrandom::fill(&mut secret).map_err(|error| error.to_string())?; + let expected = Secret(format!("{APPLICATION_NAME}-{}", hex::encode(secret))); + let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)) + .await + .map_err(|error| error.to_string())?; + let port = listener + .local_addr() + .map_err(|error| error.to_string())? + .port(); + tokio::spawn(serve(listener, server, expected.clone())); + Ok(RelayEndpoint { + host: Ipv4Addr::LOCALHOST.to_string(), + port, + application_name: expected, + }) +} + +/// Admit connections to `listener` side by side, so a silent one holds up none of the others, and pipe `server` to the first admitted. +async fn serve(listener: TcpListener, mut server: S, expected: Secret) +where + S: AsyncRead + AsyncWrite + Unpin, +{ + let deadline = tokio::time::Instant::now() + RELAY_ACCEPT_TIMEOUT; + let mut pending = JoinSet::new(); + let (mut local, startup) = loop { + tokio::select! { + accepted = listener.accept() => { + if let Ok((stream, _)) = accepted { + pending.spawn(admit(stream, expected.clone())); + } + } + Some(Ok(Some(admitted))) = pending.join_next() => break admitted, + () = tokio::time::sleep_until(deadline) => return, + } + }; + drop(listener); + drop(pending); + if server.write_all(&startup).await.is_ok() { + let _ = tokio::io::copy_bidirectional(&mut local, &mut server).await; + } +} + +/// `stream` and the startup message to forward for it, when it sends one naming `expected` within [`RELAY_STARTUP_TIMEOUT`]. +async fn admit(mut stream: TcpStream, expected: Secret) -> Option<(TcpStream, Vec)> { + let message = tokio::time::timeout(RELAY_STARTUP_TIMEOUT, read_startup(&mut stream)) + .await + .ok()??; + let forwarded = admitted_startup(&message, expected.expose())?; + let _ = stream.set_nodelay(true); + Some((stream, forwarded)) +} + +/// One startup message: its length, and as many bytes as that gives, up to [`STARTUP_MESSAGE_LIMIT`]. +async fn read_startup(stream: &mut R) -> Option> { + let mut length = [0u8; 4]; + stream.read_exact(&mut length).await.ok()?; + let declared = usize::try_from(u32::from_be_bytes(length)).ok()?; + if !(8..=STARTUP_MESSAGE_LIMIT).contains(&declared) { + return None; + } + let mut message = vec![0; declared]; + message[..4].copy_from_slice(&length); + stream.read_exact(&mut message[4..]).await.ok()?; + Some(message) +} + +/// `message`, a whole protocol 3.0 startup message, with its one `application_name` replaced by [`APPLICATION_NAME`]; `None` unless that name is `expected`. +pub(crate) fn admitted_startup(message: &[u8], expected: &str) -> Option> { + let declared = usize::try_from(u32::from_be_bytes(message.get(..4)?.try_into().ok()?)).ok()?; + if declared != message.len() || message.get(4..8)? != PROTOCOL_VERSION { + return None; + } + // Pairs of NUL-terminated names and values, then one more NUL. + let fields: Vec<&[u8]> = message[8..] + .strip_suffix(b"\0\0")? + .split(|byte| *byte == 0) + .collect(); + if !fields.len().is_multiple_of(2) || fields.iter().step_by(2).any(|name| name.is_empty()) { + return None; + } + let named: Vec = (0..fields.len()) + .step_by(2) + .filter(|index| fields[*index] == b"application_name") + .collect(); + let [name] = named.as_slice() else { + return None; + }; + if !same_secret(fields[name + 1], expected.as_bytes()) { + return None; + } + let mut forwarded = vec![0; 4]; + forwarded.extend_from_slice(&PROTOCOL_VERSION); + for (index, field) in fields.iter().enumerate() { + match index == name + 1 { + true => forwarded.extend_from_slice(APPLICATION_NAME.as_bytes()), + false => forwarded.extend_from_slice(field), + } + forwarded.push(0); + } + forwarded.push(0); + let length = u32::try_from(forwarded.len()).ok()?; + forwarded[..4].copy_from_slice(&length.to_be_bytes()); + Some(forwarded) +} + +/// Whether `given` is `expected`, comparing every byte whatever the first difference, so the time taken tells nothing of the secret. +fn same_secret(given: &[u8], expected: &[u8]) -> bool { + given.len() == expected.len() + && given + .iter() + .zip(expected) + .fold(0u8, |difference, (left, right)| difference | (left ^ right)) + == 0 +} diff --git a/crates/sqlload/src/postgres_source/_helpers/mod.rs b/crates/sqlload/src/postgres_source/_helpers/mod.rs index d329a9dd..ca8074c8 100644 --- a/crates/sqlload/src/postgres_source/_helpers/mod.rs +++ b/crates/sqlload/src/postgres_source/_helpers/mod.rs @@ -2,6 +2,8 @@ pub(crate) mod builders; pub(crate) mod catalog; pub(crate) mod copy_decoder; pub(crate) mod copy_stream; +#[cfg(any(windows, test))] +pub(crate) mod loopback_relay; pub(crate) mod rustls_connect; pub(crate) mod server_reason; pub(crate) mod split_read; diff --git a/crates/sqlload/src/postgres_source/constants.rs b/crates/sqlload/src/postgres_source/constants.rs index d61a157f..9c6e1cea 100644 --- a/crates/sqlload/src/postgres_source/constants.rs +++ b/crates/sqlload/src/postgres_source/constants.rs @@ -126,6 +126,18 @@ pub const SSL_REQUEST: [u8; 8] = [0, 0, 0, 8, 0x04, 0xd2, 0x16, 0x2f]; /// How long a TLS relay waits for its replication client to connect. pub const RELAY_ACCEPT_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30); +/// How long a loopback relay gives each connection to send its whole startup message, so a silent or slow one is dropped without holding up the replication client. +pub const RELAY_STARTUP_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(5); + +/// The longest startup message a loopback relay reads: Postgres's own limit (`MAX_STARTUP_PACKET_LENGTH`). +pub const STARTUP_MESSAGE_LIMIT: usize = 10_000; + +/// Protocol 3.0, as the replication client's startup message gives it. +pub const PROTOCOL_VERSION: [u8; 4] = [0, 3, 0, 0]; + +/// The application name SQLLoad's sessions show the server. +pub const APPLICATION_NAME: &str = "sqlload"; + /// Signature algorithm OIDs (DER contents) whose certificates bind with SHA-256: sha256WithRSAEncryption, sha1WithRSAEncryption, md5WithRSAEncryption, ecdsa-with-SHA256, ecdsa-with-SHA1. pub const SHA256_SIGNATURES: &[&[u8]] = &[ &[0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0b], diff --git a/crates/sqlload/src/postgres_source/main/relay_tls.rs b/crates/sqlload/src/postgres_source/main/relay_tls.rs index b6722fb3..6d8b10ec 100644 --- a/crates/sqlload/src/postgres_source/main/relay_tls.rs +++ b/crates/sqlload/src/postgres_source/main/relay_tls.rs @@ -1,19 +1,28 @@ //! TLS for a client that speaks only plain text: the replication client. +#[cfg(unix)] use std::os::unix::fs::PermissionsExt; use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt}; -use tokio::net::{TcpStream, UnixListener}; +use tokio::net::TcpStream; +#[cfg(unix)] +use tokio::net::UnixListener; use tokio_rustls::TlsConnector; +#[cfg(windows)] +use crate::postgres_source::_helpers::loopback_relay::loopback_relay; use crate::postgres_source::_helpers::tls::client_config; -use crate::postgres_source::constants::{RELAY_ACCEPT_TIMEOUT, SSL_REQUEST}; +use crate::postgres_source::constants::SSL_REQUEST; +#[cfg(unix)] +use crate::postgres_source::constants::{APPLICATION_NAME, RELAY_ACCEPT_TIMEOUT}; use crate::postgres_source::errors::PostgresError; -use crate::postgres_source::models::{PostgresAddress, SslMode}; +use crate::postgres_source::models::{PostgresAddress, RelayEndpoint, SslMode}; use crate::ssh_tunnel::main::open_channel::open_channel; -/// When `address` may use TLS or goes through a bastion: a private socket folder whose `.s.PGSQL.` socket carries the next session opened on it to the server, encrypted, or in plain text when the server refuses TLS under `prefer` (one connection either way, so the server logs no abandoned one). `None` to connect directly: `sslmode = "disable"`, a Unix socket host, or a failed handshake under `prefer`, which libpq also retries in plain text. Through a bastion it never connects directly: that retry opens a fresh channel. -pub async fn relay_tls(address: &PostgresAddress<'_>) -> Result, PostgresError> { +/// When `address` may use TLS or goes through a bastion: where the replication client connects (a private socket folder whose `.s.PGSQL.` socket, or on Windows a loopback port, carries the next session opened on it to the server, encrypted, or in plain text when the server refuses TLS under `prefer` (one connection either way, so the server logs no abandoned one), and the application name it must send. `None` to connect directly: `sslmode = "disable"`, a Unix socket host, or a failed handshake under `prefer`, which libpq also retries in plain text. Through a bastion it never connects directly: that retry opens a fresh channel. +pub async fn relay_tls( + address: &PostgresAddress<'_>, +) -> Result, PostgresError> { let failed = |reason: String| PostgresError::Tls { address: address.shown(), reason, @@ -26,7 +35,7 @@ pub async fn relay_tls(address: &PostgresAddress<'_>) -> Result, } } let server = open_channel(tunnel, address.host, address.port).await?; - return Ok(Some(relay(server, address.port).map_err(failed)?)); + return Ok(Some(relay(server, address.port).await.map_err(failed)?)); } if address.tls.mode == SslMode::Disable || address.host.starts_with('/') { return Ok(None); @@ -44,7 +53,7 @@ pub async fn relay_tls(address: &PostgresAddress<'_>) -> Result, async fn negotiate( mut server: S, address: &PostgresAddress<'_>, -) -> Result, PostgresError> +) -> Result, PostgresError> where S: AsyncRead + AsyncWrite + Unpin + Send + 'static, { @@ -64,7 +73,7 @@ where != b'S' { return match address.tls.mode { - SslMode::Prefer => Ok(Some(relay(server, address.port).map_err(failed)?)), + SslMode::Prefer => Ok(Some(relay(server, address.port).await.map_err(failed)?)), _ => Err(failed("the server does not offer TLS".to_owned())), }; } @@ -75,11 +84,12 @@ where Err(_) if address.tls.mode == SslMode::Prefer => return Ok(None), Err(error) => return Err(failed(format!("TLS handshake failed: {error}"))), }; - Ok(Some(relay(session, address.port).map_err(failed)?)) + Ok(Some(relay(session, address.port).await.map_err(failed)?)) } /// A folder only this user can enter (some systems ignore a socket's own permissions) whose `.s.PGSQL.` socket pipes the first session opened on it through `server`; the folder and its socket go once it is accepted, or after [`RELAY_ACCEPT_TIMEOUT`] without one. -fn relay(mut server: S, port: u16) -> Result +#[cfg(unix)] +async fn relay(mut server: S, port: u16) -> Result where S: AsyncRead + AsyncWrite + Unpin + Send + 'static, { @@ -99,5 +109,18 @@ where let _ = tokio::io::copy_bidirectional(&mut local, &mut server).await; } }); - Ok(host) + Ok(RelayEndpoint { + host, + port, + application_name: crate::auth::models::Secret(APPLICATION_NAME.to_owned()), + }) +} + +/// pgwire-replication reaches no socket file on Windows, only TCP, so the session goes through a loopback port that admits only this relay's secret (see [`loopback_relay`]). +#[cfg(windows)] +async fn relay(server: S, _port: u16) -> Result +where + S: AsyncRead + AsyncWrite + Unpin + Send + 'static, +{ + loopback_relay(server).await } diff --git a/crates/sqlload/src/postgres_source/models.rs b/crates/sqlload/src/postgres_source/models.rs index 950b3013..c1e8ccb0 100644 --- a/crates/sqlload/src/postgres_source/models.rs +++ b/crates/sqlload/src/postgres_source/models.rs @@ -192,3 +192,13 @@ pub struct CopyRequest<'a> { /// More connections to the same database, idle and outside any transaction, to read a large table in parallel; empty reads in one pass. pub helpers: &'a [tokio_postgres::Client], } + +/// Where the replication client reaches a relayed session, and the application name its startup message must carry there. +#[derive(Debug)] +pub struct RelayEndpoint { + /// The relay's socket folder (Unix) or `127.0.0.1` (Windows). + pub host: String, + pub port: u16, + /// `sqlload` on Unix. On Windows `sqlload-`, which the relay checks and replaces with `sqlload` before the server sees it. + pub application_name: crate::auth::models::Secret, +} diff --git a/crates/sqlload/src/postgres_source/tests.rs b/crates/sqlload/src/postgres_source/tests.rs index 934df596..03a616d3 100644 --- a/crates/sqlload/src/postgres_source/tests.rs +++ b/crates/sqlload/src/postgres_source/tests.rs @@ -4,6 +4,8 @@ mod copy_decoder; mod decode_copy; #[path = "tests/helpers.rs"] mod helpers; +#[path = "tests/test_loopback_relay.rs"] +mod loopback_relay; #[path = "tests/test_read_column.rs"] mod read_column; #[path = "tests/test_read_numeric.rs"] diff --git a/crates/sqlload/src/postgres_source/tests/helpers.rs b/crates/sqlload/src/postgres_source/tests/helpers.rs index 4cd42e8b..dbe21b06 100644 --- a/crates/sqlload/src/postgres_source/tests/helpers.rs +++ b/crates/sqlload/src/postgres_source/tests/helpers.rs @@ -96,3 +96,49 @@ pub(crate) fn source_column(pg_type: PgType) -> SourceColumn { source_spelling: None, } } + +/// A startup message of protocol `version` with `params`, framed as libpq and pgwire-replication frame it. +pub(crate) fn startup_message(version: [u8; 4], params: &[(&str, &str)]) -> Vec { + let mut body = version.to_vec(); + for (name, value) in params { + body.extend_from_slice(name.as_bytes()); + body.push(0); + body.extend_from_slice(value.as_bytes()); + body.push(0); + } + body.push(0); + let length = u32::try_from(body.len() + 4).unwrap_or(u32::MAX); + let mut message = length.to_be_bytes().to_vec(); + message.extend_from_slice(&body); + message +} + +/// The parameters of a well-formed protocol 3.0 startup message, in order; an error naming what is wrong otherwise. +pub(crate) fn startup_params(message: &[u8]) -> Result, String> { + let declared = message + .get(..4) + .and_then(|bytes| <[u8; 4]>::try_from(bytes).ok()) + .map(u32::from_be_bytes) + .ok_or("no length")?; + if usize::try_from(declared).ok() != Some(message.len()) { + return Err(format!("length {declared} for {} bytes", message.len())); + } + if message.get(4..8) != Some(&[0, 3, 0, 0][..]) { + return Err("not protocol 3.0".to_owned()); + } + let text = String::from_utf8(message[8..].to_vec()).map_err(|error| error.to_string())?; + let mut parts: Vec<&str> = text.split('\0').collect(); + // The pairs' terminators and the final NUL leave two empty parts at the end. + if parts.len() < 2 || parts.pop() != Some("") || parts.pop() != Some("") { + return Err("not NUL-terminated".to_owned()); + } + Ok(parts + .chunks(2) + .map(|pair| { + ( + pair[0].to_owned(), + pair.get(1).copied().unwrap_or_default().to_owned(), + ) + }) + .collect()) +} diff --git a/crates/sqlload/src/postgres_source/tests/test_loopback_relay.rs b/crates/sqlload/src/postgres_source/tests/test_loopback_relay.rs new file mode 100644 index 00000000..8a2645c2 --- /dev/null +++ b/crates/sqlload/src/postgres_source/tests/test_loopback_relay.rs @@ -0,0 +1,304 @@ +use std::time::{Duration, Instant}; + +use tokio::io::{AsyncReadExt, AsyncWriteExt, DuplexStream}; +use tokio::net::TcpStream; + +use crate::postgres_source::_helpers::loopback_relay::{admitted_startup, loopback_relay}; +use crate::postgres_source::constants::{PROTOCOL_VERSION, RELAY_STARTUP_TIMEOUT}; +use crate::postgres_source::models::RelayEndpoint; +use crate::postgres_source::tests::helpers::{startup_message, startup_params}; +use crate::postgres_source::tests::test_types::AdmittedStartupTestCase; + +const SECRET: &str = "sqlload-6f1c0d2a9b8e7f6a5b4c3d2e1f0a9b8c7d6e5f4a3b2c1d0e9f8a7b6c5d4e3f2a"; + +fn block_on(body: impl std::future::Future>) -> Result<(), String> { + tokio::runtime::Builder::new_multi_thread() + .worker_threads(2) + .enable_all() + .build() + .map_err(|error| error.to_string())? + .block_on(body) +} + +fn owned(params: &[(&str, &str)]) -> Vec<(String, String)> { + params + .iter() + .map(|(name, value)| ((*name).to_owned(), (*value).to_owned())) + .collect() +} + +/// The replication client's startup message, naming `application_name`. +fn client_startup(application_name: &str) -> Vec { + startup_message( + PROTOCOL_VERSION, + &[ + ("user", "replicator"), + ("database", "shop"), + ("replication", "database"), + ("application_name", application_name), + ], + ) +} + +/// The whole first message the server side of the relay received. +async fn read_forwarded(upstream: &mut DuplexStream) -> Result, String> { + let mut length = [0u8; 4]; + upstream + .read_exact(&mut length) + .await + .map_err(|error| error.to_string())?; + let declared = + usize::try_from(u32::from_be_bytes(length)).map_err(|error| error.to_string())?; + let mut message = length.to_vec(); + message.resize(declared.max(4), 0); + upstream + .read_exact(&mut message[4..]) + .await + .map_err(|error| error.to_string())?; + Ok(message) +} + +fn contains(haystack: &[u8], needle: &str) -> bool { + haystack + .windows(needle.len()) + .any(|window| window == needle.as_bytes()) +} + +async fn connect(endpoint: &RelayEndpoint) -> Result { + TcpStream::connect((endpoint.host.as_str(), endpoint.port)) + .await + .map_err(|error| error.to_string()) +} + +/// Whether the relay closes `stream` without answering, within `wait`. +async fn closed_unanswered(stream: &mut TcpStream, wait: Duration) -> Result { + let mut byte = [0u8; 1]; + match tokio::time::timeout(wait, stream.read(&mut byte)).await { + Ok(Ok(0) | Err(_)) => Ok(true), + Ok(Ok(_)) => Ok(false), + Err(_) => Err("the relay kept the connection open".to_owned()), + } +} + +#[test] +fn given_startup_messages_when_the_relay_checks_them_then_only_the_secret_is_admitted_and_never_forwarded() +-> Result<(), String> { + let wrong_last = format!("{}0", &SECRET[..SECRET.len() - 1]); + let mut short_length = client_startup(SECRET); + short_length[3] -= 1; + let mut no_terminator = client_startup(SECRET); + no_terminator.pop(); + no_terminator[3] -= 1; + let test_cases = [ + AdmittedStartupTestCase { + description: "the secret, among the other parameters", + message: client_startup(SECRET), + expected_forwarded: Some(owned(&[ + ("user", "replicator"), + ("database", "shop"), + ("replication", "database"), + ("application_name", "sqlload"), + ])), + }, + AdmittedStartupTestCase { + description: "the secret first, before an empty value", + message: startup_message( + PROTOCOL_VERSION, + &[("application_name", SECRET), ("options", ""), ("user", "u")], + ), + expected_forwarded: Some(owned(&[ + ("application_name", "sqlload"), + ("options", ""), + ("user", "u"), + ])), + }, + AdmittedStartupTestCase { + description: "a secret wrong in its last character", + message: client_startup(&wrong_last), + expected_forwarded: None, + }, + AdmittedStartupTestCase { + description: "a prefix of the secret", + message: client_startup(&SECRET[..SECRET.len() - 1]), + expected_forwarded: None, + }, + AdmittedStartupTestCase { + description: "the secret with more after it", + message: client_startup(&format!("{SECRET}0")), + expected_forwarded: None, + }, + AdmittedStartupTestCase { + description: "the plain application name", + message: client_startup("sqlload"), + expected_forwarded: None, + }, + AdmittedStartupTestCase { + description: "no application name", + message: startup_message(PROTOCOL_VERSION, &[("user", "u"), ("database", "d")]), + expected_forwarded: None, + }, + AdmittedStartupTestCase { + description: "the secret, then a second application name", + message: startup_message( + PROTOCOL_VERSION, + &[("application_name", SECRET), ("application_name", "other")], + ), + expected_forwarded: None, + }, + AdmittedStartupTestCase { + description: "an SSLRequest", + message: vec![0, 0, 0, 8, 0x04, 0xd2, 0x16, 0x2f], + expected_forwarded: None, + }, + AdmittedStartupTestCase { + description: "protocol 2.0 with the secret", + message: startup_message([0, 2, 0, 0], &[("application_name", SECRET)]), + expected_forwarded: None, + }, + AdmittedStartupTestCase { + description: "a length one short of the message", + message: short_length, + expected_forwarded: None, + }, + AdmittedStartupTestCase { + description: "no final NUL", + message: no_terminator, + expected_forwarded: None, + }, + ]; + for test_case in test_cases { + let forwarded = admitted_startup(&test_case.message, SECRET); + let found = forwarded.as_deref().map(startup_params).transpose()?; + if found != test_case.expected_forwarded { + return Err(format!( + "{}: forwarded {found:?}, expected {:?}", + test_case.description, test_case.expected_forwarded + )); + } + if forwarded.is_some_and(|bytes| contains(&bytes, &SECRET["sqlload-".len()..])) { + return Err(format!( + "{}: the secret was forwarded", + test_case.description + )); + } + } + Ok(()) +} + +#[test] +fn given_a_silent_connection_first_when_the_replication_client_connects_then_it_is_relayed_without_waiting() +-> Result<(), String> { + block_on(async { + let (server, mut upstream) = tokio::io::duplex(64 * 1024); + let endpoint = loopback_relay(server).await?; + let _silent = connect(&endpoint).await?; + let started = Instant::now(); + let mut client = connect(&endpoint).await?; + client + .write_all(&client_startup(endpoint.application_name.expose())) + .await + .map_err(|error| error.to_string())?; + let forwarded = tokio::time::timeout(RELAY_STARTUP_TIMEOUT, read_forwarded(&mut upstream)) + .await + .map_err(|_| "the client was held up behind the silent connection".to_owned())??; + if started.elapsed() >= RELAY_STARTUP_TIMEOUT { + return Err(format!("relayed after {:?}", started.elapsed())); + } + let name = startup_params(&forwarded)? + .into_iter() + .find(|(name, _)| name == "application_name"); + if name != Some(("application_name".to_owned(), "sqlload".to_owned())) { + return Err(format!("forwarded application name {name:?}")); + } + // The session is piped both ways once admitted. + upstream + .write_all(b"R") + .await + .map_err(|error| error.to_string())?; + let mut answer = [0u8; 1]; + client + .read_exact(&mut answer) + .await + .map_err(|error| error.to_string())?; + client + .write_all(b"p") + .await + .map_err(|error| error.to_string())?; + upstream + .read_exact(&mut answer) + .await + .map_err(|error| error.to_string())?; + match &answer { + b"p" => Ok(()), + other => Err(format!("piped {other:?}")), + } + }) +} + +#[test] +fn given_connections_without_the_secret_when_they_connect_then_they_are_dropped_and_the_server_receives_nothing() +-> Result<(), String> { + block_on(async { + let (server, mut upstream) = tokio::io::duplex(64 * 1024); + let endpoint = loopback_relay(server).await?; + let secret = endpoint.application_name.expose().to_owned(); + if format!("{endpoint:?}").contains(&secret) { + return Err("the relay's Debug shows its secret".to_owned()); + } + let wrong = format!("sqlload-{}", "0".repeat(64)); + let attempts: [(&str, Vec); 4] = [ + ("a wrong secret", client_startup(&wrong)), + ("the plain application name", client_startup("sqlload")), + ("an oversize length", vec![0, 1, 0, 0, 0, 3, 0, 0]), + ("an SSLRequest", vec![0, 0, 0, 8, 0x04, 0xd2, 0x16, 0x2f]), + ]; + for (description, bytes) in attempts { + let mut stream = connect(&endpoint).await?; + stream + .write_all(&bytes) + .await + .map_err(|error| error.to_string())?; + if !closed_unanswered(&mut stream, RELAY_STARTUP_TIMEOUT * 2) + .await + .map_err(|error| format!("{description}: {error}"))? + { + return Err(format!("{description}: the relay answered")); + } + } + let mut byte = [0u8; 1]; + if tokio::time::timeout(Duration::from_millis(300), upstream.read(&mut byte)) + .await + .is_ok() + { + return Err("the server received bytes from a refused connection".to_owned()); + } + let mut client = connect(&endpoint).await?; + client + .write_all(&client_startup(&secret)) + .await + .map_err(|error| error.to_string())?; + let forwarded = read_forwarded(&mut upstream).await?; + if contains(&forwarded, &secret["sqlload-".len()..]) { + return Err("the secret reached the server".to_owned()); + } + let expected = owned(&[ + ("user", "replicator"), + ("database", "shop"), + ("replication", "database"), + ("application_name", "sqlload"), + ]); + let found = startup_params(&forwarded)?; + if found != expected { + return Err(format!("forwarded {found:?}")); + } + // Admitted once: the port is closed to every later connection. + match tokio::time::timeout(Duration::from_secs(10), connect(&endpoint)).await { + Ok(Err(_)) => Ok(()), + Ok(Ok(mut late)) => match closed_unanswered(&mut late, Duration::from_secs(2)).await { + Ok(true) => Ok(()), + _ => Err("a connection after the admitted one was kept".to_owned()), + }, + Err(_) => Err("connecting after admission hung".to_owned()), + } + }) +} diff --git a/crates/sqlload/src/postgres_source/tests/test_types.rs b/crates/sqlload/src/postgres_source/tests/test_types.rs index 4443f9d2..87899d09 100644 --- a/crates/sqlload/src/postgres_source/tests/test_types.rs +++ b/crates/sqlload/src/postgres_source/tests/test_types.rs @@ -37,3 +37,10 @@ pub(crate) struct ReadColumnTestCase { /// The select expression, or the error text. pub(crate) expected_result: Result<&'static str, &'static str>, } + +pub(crate) struct AdmittedStartupTestCase { + pub(crate) description: &'static str, + pub(crate) message: Vec, + /// The parameters forwarded to the server, in order; `None` when the connection is refused. + pub(crate) expected_forwarded: Option>, +} diff --git a/crates/sqlload/src/store_paths/_helpers/local_root.rs b/crates/sqlload/src/store_paths/_helpers/local_root.rs new file mode 100644 index 00000000..02fc4e96 --- /dev/null +++ b/crates/sqlload/src/store_paths/_helpers/local_root.rs @@ -0,0 +1,55 @@ +use std::borrow::Cow; + +/// `path` with Windows separators read as `/` and the `\\?\` (`\\?\UNC\`) prefix dropped, when `windows`; unchanged otherwise. +pub(crate) fn windows_separators(path: &str, windows: bool) -> Cow<'_, str> { + if !windows || !path.contains('\\') { + return Cow::Borrowed(path); + } + let path = path.replace('\\', "/"); + Cow::Owned(match path.strip_prefix("//?/") { + Some(rest) => match rest.get(..4) { + Some(unc) if unc.eq_ignore_ascii_case("UNC/") => format!("//{}", &rest[4..]), + _ => rest.to_owned(), + }, + None => path, + }) +} + +/// The root and the rest of `path`, `/`-separated (see [`windows_separators`]), when it is absolute on this machine: `/`, or when `windows` also a drive (`C:/`, `/C:/` as a `file:///C:/` URL gives it) or a share (`//server/share/`). `None` for a relative path. +pub(crate) fn local_root(path: &str, windows: bool) -> Option<(String, &str)> { + if windows { + if let Some(share) = path.strip_prefix("//") { + let mut parts = share.splitn(3, '/'); + let (server, name) = (parts.next()?, parts.next()?); + return (!server.is_empty() && !name.is_empty()).then(|| { + ( + format!("//{server}/{name}/"), + parts.next().unwrap_or_default(), + ) + }); + } + let drive = path.strip_prefix('/').unwrap_or(path); + let bytes = drive.as_bytes(); + if bytes.len() >= 3 + && bytes[0].is_ascii_alphabetic() + && bytes[1] == b':' + && bytes[2] == b'/' + { + return Some(( + format!("{}:/", char::from(bytes[0]).to_ascii_uppercase()), + &drive[3..], + )); + } + } + path.strip_prefix('/').map(|rest| ("/".to_owned(), rest)) +} + +/// The local path a `file://` URL names, given what follows `file://`: on Windows `file:///C:/folder` is `C:/folder` and `file://server/share/folder` is the share `//server/share/folder`. +pub(crate) fn windows_file_url_path(rest: &str, windows: bool) -> Cow<'_, str> { + match (windows, rest.strip_prefix('/')) { + (false, _) => Cow::Borrowed(rest), + (true, Some(path)) if path.as_bytes().get(1) == Some(&b':') => Cow::Borrowed(path), + (true, Some(_)) => Cow::Borrowed(rest), + (true, None) => Cow::Owned(format!("//{rest}")), + } +} diff --git a/crates/sqlload/src/store_paths/_helpers/locations.rs b/crates/sqlload/src/store_paths/_helpers/locations.rs index c5cf45d6..f7af877e 100644 --- a/crates/sqlload/src/store_paths/_helpers/locations.rs +++ b/crates/sqlload/src/store_paths/_helpers/locations.rs @@ -1,8 +1,13 @@ +use std::borrow::Cow; + use crate::store_paths::_helpers::folder::folder; +use crate::store_paths::_helpers::local_root::{ + local_root, windows_file_url_path, windows_separators, +}; use crate::store_paths::errors::PathError; use crate::store_paths::models::{Location, StoreKind}; -/// The location in a URL or absolute path, `None` for a relative path. +/// The location in a URL or absolute path (on Windows also `C:\folder`, `\\server\share\folder` and `file://server/share/folder`), `None` for a relative path. pub(crate) fn url_location(path: &str) -> Result, PathError> { if let Some(rest) = path.strip_prefix("s3://") { let (bucket, key) = rest.split_once('/').unwrap_or((rest, "")); @@ -22,9 +27,14 @@ pub(crate) fn url_location(path: &str) -> Result, PathError> { prefix: folder(&[key])?, })); } - let absolute = path.strip_prefix("file://").unwrap_or(path); - if absolute.starts_with('/') { - let prefix = folder(&[absolute])?; + let windows = cfg!(windows); + let local = match path.strip_prefix("file://") { + Some(rest) => windows_file_url_path(rest, windows), + None => Cow::Borrowed(path), + }; + let local = windows_separators(&local, windows); + if let Some((root, rest)) = local_root(&local, windows) { + let prefix = folder(&[rest])?; if prefix.is_empty() { return Err(PathError::Empty { path: path.to_owned(), @@ -32,9 +42,7 @@ pub(crate) fn url_location(path: &str) -> Result, PathError> { }); } return Ok(Some(Location { - store: StoreKind::Filesystem { - root: "/".to_owned(), - }, + store: StoreKind::Filesystem { root }, prefix, })); } diff --git a/crates/sqlload/src/store_paths/_helpers/mod.rs b/crates/sqlload/src/store_paths/_helpers/mod.rs index c48429aa..78a0a2ce 100644 --- a/crates/sqlload/src/store_paths/_helpers/mod.rs +++ b/crates/sqlload/src/store_paths/_helpers/mod.rs @@ -1,4 +1,5 @@ pub(crate) mod escapes; pub(crate) mod folder; +pub(crate) mod local_root; pub(crate) mod locations; pub(crate) mod segments; diff --git a/crates/sqlload/src/store_paths/main/file_url_path.rs b/crates/sqlload/src/store_paths/main/file_url_path.rs new file mode 100644 index 00000000..b5984fdf --- /dev/null +++ b/crates/sqlload/src/store_paths/main/file_url_path.rs @@ -0,0 +1,10 @@ +//! The local path a `file://` URL names. + +use std::borrow::Cow; + +use crate::store_paths::_helpers::local_root::windows_file_url_path; + +/// The local path a `file://` URL names, given what follows `file://`: the same on Unix; on Windows `file:///C:/folder` is `C:/folder` and `file://server/share/folder` is the share `//server/share/folder`. +pub fn file_url_path(rest: &str) -> Cow<'_, str> { + windows_file_url_path(rest, cfg!(windows)) +} diff --git a/crates/sqlload/src/store_paths/main/local_separators.rs b/crates/sqlload/src/store_paths/main/local_separators.rs new file mode 100644 index 00000000..5efdcd32 --- /dev/null +++ b/crates/sqlload/src/store_paths/main/local_separators.rs @@ -0,0 +1,10 @@ +//! Read a local path's Windows separators as `/`. + +use std::borrow::Cow; + +use crate::store_paths::_helpers::local_root::windows_separators; + +/// `path` with `\` read as `/` and the `\\?\` prefix dropped on Windows, so its folders split on `/` as everywhere else; unchanged on other systems. +pub fn local_separators(path: &str) -> Cow<'_, str> { + windows_separators(path, cfg!(windows)) +} diff --git a/crates/sqlload/src/store_paths/main/mod.rs b/crates/sqlload/src/store_paths/main/mod.rs index f5f96a2e..82df1fed 100644 --- a/crates/sqlload/src/store_paths/main/mod.rs +++ b/crates/sqlload/src/store_paths/main/mod.rs @@ -1,5 +1,7 @@ pub mod escape_partition_value; +pub mod file_url_path; pub mod hive_values; +pub mod local_separators; pub mod render_template; pub mod resolve_location; pub mod unescape_partition_value; diff --git a/crates/sqlload/src/store_paths/main/resolve_location.rs b/crates/sqlload/src/store_paths/main/resolve_location.rs index 3cf38b18..e7e4ccdb 100644 --- a/crates/sqlload/src/store_paths/main/resolve_location.rs +++ b/crates/sqlload/src/store_paths/main/resolve_location.rs @@ -1,6 +1,7 @@ //! Work out which store and folder a path means. use crate::store_paths::_helpers::folder::folder; +use crate::store_paths::_helpers::local_root::{local_root, windows_separators}; use crate::store_paths::_helpers::locations::{describe, same_store, url_location}; use crate::store_paths::errors::PathError; use crate::store_paths::models::{Location, StoreKind}; @@ -40,8 +41,22 @@ pub fn resolve_location( }) } (Some((name, StoreKind::Filesystem { root })), Some(location)) => { - let root_prefix = folder(&[root])?; - match location.prefix.strip_prefix(&root_prefix) { + let windows = cfg!(windows); + let separated = windows_separators(root, windows); + // A relative root is matched on its folders alone, whatever the path's root. + let (same_root, root_folders) = match (local_root(&separated, windows), &location.store) + { + (Some((root, folders)), StoreKind::Filesystem { root: found }) => { + (root.eq_ignore_ascii_case(found), folders) + } + _ => (true, separated.as_ref()), + }; + let root_prefix = folder(&[root_folders])?; + match location + .prefix + .strip_prefix(&root_prefix) + .filter(|_| same_root) + { Some(relative) => Ok(Location { store: StoreKind::Filesystem { root: root.clone() }, prefix: relative.to_owned(), @@ -49,7 +64,7 @@ pub fn resolve_location( None => Err(PathError::ConnectionMismatch { url: path.to_owned(), connection: name.to_owned(), - found: format!("filesystem /{}", location.prefix), + found: format!("filesystem {location}"), expected: format!("filesystem {root}"), }), } @@ -58,6 +73,10 @@ pub fn resolve_location( store: store.clone(), prefix: location.prefix, }), + (Some((_, store @ StoreKind::Filesystem { .. })), None) => Ok(Location { + store: store.clone(), + prefix: folder(&[&windows_separators(path, cfg!(windows))])?, + }), (Some((_, store)), None) => Ok(Location { store: store.clone(), prefix: folder(&[path])?, diff --git a/crates/sqlload/src/store_paths/tests.rs b/crates/sqlload/src/store_paths/tests.rs index fc22ae49..00f29b1a 100644 --- a/crates/sqlload/src/store_paths/tests.rs +++ b/crates/sqlload/src/store_paths/tests.rs @@ -4,6 +4,8 @@ mod folder; mod helpers; #[path = "tests/test_hive_values.rs"] mod hive_values; +#[path = "tests/test_local_root.rs"] +mod local_root; #[path = "tests/test_partition_value.rs"] mod partition_value; #[path = "tests/test_render_template.rs"] diff --git a/crates/sqlload/src/store_paths/tests/test_local_root.rs b/crates/sqlload/src/store_paths/tests/test_local_root.rs new file mode 100644 index 00000000..5615e0c4 --- /dev/null +++ b/crates/sqlload/src/store_paths/tests/test_local_root.rs @@ -0,0 +1,110 @@ +use crate::store_paths::_helpers::local_root::{ + local_root, windows_file_url_path, windows_separators, +}; + +/// How a local path reads on Windows, and on Unix, where only `/` starts an absolute path. +struct LocalRootTestCase { + description: &'static str, + path: &'static str, + /// Root and folders on Windows; `None` for a relative path. + expected_windows: Option<(&'static str, &'static str)>, + expected_unix: Option<(&'static str, &'static str)>, +} + +#[test] +fn given_local_paths_when_finding_their_root_then_drives_and_shares_are_roots_only_on_windows() { + let test_cases = [ + LocalRootTestCase { + description: "a Unix absolute path", + path: "/data/exports", + expected_windows: Some(("/", "data/exports")), + expected_unix: Some(("/", "data/exports")), + }, + LocalRootTestCase { + description: "a drive with backslashes, lower-case letter", + path: r"c:\Data\exports", + expected_windows: Some(("C:/", "Data/exports")), + expected_unix: None, + }, + LocalRootTestCase { + description: "a drive as a file:/// URL gives it", + path: "/D:/lake", + expected_windows: Some(("D:/", "lake")), + expected_unix: Some(("/", "D:/lake")), + }, + LocalRootTestCase { + description: "a UNC share", + path: r"\\nas01\exports\daily\orders", + expected_windows: Some(("//nas01/exports/", "daily/orders")), + expected_unix: None, + }, + LocalRootTestCase { + description: "a UNC share's root alone", + path: r"\\nas01\exports", + expected_windows: Some(("//nas01/exports/", "")), + expected_unix: None, + }, + LocalRootTestCase { + description: "an extended-length drive path", + path: r"\\?\C:\very\long", + expected_windows: Some(("C:/", "very/long")), + expected_unix: None, + }, + LocalRootTestCase { + description: "an extended-length UNC path", + path: r"\\?\UNC\nas01\exports\x", + expected_windows: Some(("//nas01/exports/", "x")), + expected_unix: None, + }, + LocalRootTestCase { + description: "a server without a share is no root", + path: r"\\nas01", + expected_windows: None, + expected_unix: None, + }, + LocalRootTestCase { + description: "a drive-relative path is relative", + path: "C:data", + expected_windows: None, + expected_unix: None, + }, + LocalRootTestCase { + description: "a relative path", + path: r"exports\daily", + expected_windows: None, + expected_unix: None, + }, + ]; + for test_case in test_cases { + for (windows, expected) in [ + (true, test_case.expected_windows), + (false, test_case.expected_unix), + ] { + let separated = windows_separators(test_case.path, windows); + let found = local_root(&separated, windows); + let found = found.as_ref().map(|(root, rest)| (root.as_str(), *rest)); + assert_eq!( + found, expected, + "{} (windows: {windows})", + test_case.description + ); + } + } +} + +#[test] +fn given_file_urls_when_reading_their_path_then_windows_drives_and_shares_are_found() { + let test_cases = [ + ("/data/x", "/data/x", "/data/x"), + ("/C:/data/x", "C:/data/x", "/C:/data/x"), + ("nas01/exports/x", "//nas01/exports/x", "nas01/exports/x"), + ]; + for (rest, expected_windows, expected_unix) in test_cases { + assert_eq!( + windows_file_url_path(rest, true), + expected_windows, + "{rest}" + ); + assert_eq!(windows_file_url_path(rest, false), expected_unix, "{rest}"); + } +} diff --git a/crates/sqlload/src/store_paths/tests/test_resolve_location.rs b/crates/sqlload/src/store_paths/tests/test_resolve_location.rs index be4de648..94249d6e 100644 --- a/crates/sqlload/src/store_paths/tests/test_resolve_location.rs +++ b/crates/sqlload/src/store_paths/tests/test_resolve_location.rs @@ -87,3 +87,86 @@ fn given_a_path_when_resolving_then_the_store_and_folder_are_found() { ); } } + +#[cfg(windows)] +#[test] +fn given_windows_paths_when_resolving_then_drives_and_shares_are_local_folders() { + let share = || { + Some(StoreKind::Filesystem { + root: r"\\nas01\exports".to_owned(), + }) + }; + let test_cases = [ + ResolveLocationTestCase { + description: "a drive path with backslashes", + connection: None, + path: r"C:\data\exports", + expected_location: Ok("C:/data/exports/"), + }, + ResolveLocationTestCase { + description: "a drive as a file:/// URL", + connection: None, + path: "file:///d:/lake/scores", + expected_location: Ok("D:/lake/scores/"), + }, + ResolveLocationTestCase { + description: "a UNC share", + connection: None, + path: r"\\nas01\exports\daily", + expected_location: Ok("//nas01/exports/daily/"), + }, + ResolveLocationTestCase { + description: "a share as a file:// URL", + connection: None, + path: "file://nas01/exports/daily", + expected_location: Ok("//nas01/exports/daily/"), + }, + ResolveLocationTestCase { + description: "a path under a share connection becomes relative to it", + connection: share(), + path: r"\\NAS01\exports\daily\orders", + expected_location: Ok(r"\\nas01\exports/daily/orders/"), + }, + ResolveLocationTestCase { + description: "a relative path with backslashes under a share connection", + connection: share(), + path: r"daily\orders", + expected_location: Ok(r"\\nas01\exports/daily/orders/"), + }, + ResolveLocationTestCase { + description: "another share is not under the connection", + connection: share(), + path: r"\\nas02\exports\daily", + expected_location: Err("connection \"lake\""), + }, + ResolveLocationTestCase { + description: "another drive is not under a drive connection", + connection: Some(StoreKind::Filesystem { + root: r"C:\lake".to_owned(), + }), + path: r"D:\lake\x", + expected_location: Err("connection \"lake\""), + }, + ResolveLocationTestCase { + description: "a share's root alone is no folder", + connection: None, + path: r"\\nas01\exports", + expected_location: Err("is empty"), + }, + ]; + for test_case in test_cases { + let connection = test_case.connection.as_ref().map(|store| ("lake", store)); + let result = resolve_location(connection, test_case.path) + .map(|location| location.to_string()) + .map_err(|error| error.to_string()); + match (&result, test_case.expected_location) { + (Ok(found), Ok(expected)) => assert_eq!(found, expected, "{}", test_case.description), + (Err(found), Err(expected)) => assert!( + found.contains(expected), + "{}: {found}", + test_case.description + ), + _ => panic!("{}: {result:?}", test_case.description), + } + } +} diff --git a/crates/sqlload/tests/loader/test_postgres_tls.rs b/crates/sqlload/tests/loader/test_postgres_tls.rs index ff80cc3f..8fbe5932 100644 --- a/crates/sqlload/tests/loader/test_postgres_tls.rs +++ b/crates/sqlload/tests/loader/test_postgres_tls.rs @@ -453,6 +453,7 @@ fn given_a_relay_when_its_session_is_taken_then_its_socket_folder_is_gone() -> R let folder = relay_tls(&address) .await .map_err(|error| error.to_string())? + .map(|relay| relay.host) .unwrap_or_default(); let socket = Path::new(&folder).join(format!(".s.PGSQL.{port}")); let mode = std::os::unix::fs::PermissionsExt::mode( diff --git a/website/src/content/docs/docs/cli/copy.mdx b/website/src/content/docs/docs/cli/copy.mdx index b454030c..eccb604d 100644 --- a/website/src/content/docs/docs/cli/copy.mdx +++ b/website/src/content/docs/docs/cli/copy.mdx @@ -31,6 +31,8 @@ Each end is a connection named in `sqlload.toml`, or a URI: | `s3://bucket/prefix/?region=eu-west-2` | Files in S3 | Parquet files in S3 | | `file:///folder`, or a folder | Files in a folder | Parquet files in a folder | +On Windows a folder can be a drive path (`C:\data\orders`) or a network share (`\\server\share\orders`), and the URL forms are `file:///C:/data/orders` and `file://server/share/orders`. + Snowflake needs `auth` and a `load_method`, which a URI can't carry, so a Snowflake end is always a named connection. diff --git a/website/src/content/docs/docs/configuration.mdx b/website/src/content/docs/docs/configuration.mdx index d0f11a9a..2e12cac0 100644 --- a/website/src/content/docs/docs/configuration.mdx +++ b/website/src/content/docs/docs/configuration.mdx @@ -143,7 +143,7 @@ delete, set `AWS_DISABLE_BULK_DELETE=true`: SQLLoad then deletes each file with | Key | Meaning | |-----|---------| -| `root` | A folder on the machine SQLLoad runs on. | +| `root` | A folder on the machine SQLLoad runs on. On Windows also a drive path (`'C:\data\lake'`) or a network share (`'\\server\share\lake'`); write it as a TOML literal string, in single quotes, so the backslashes stay. | ### `sftp` diff --git a/website/src/content/docs/docs/install.mdx b/website/src/content/docs/docs/install.mdx index b40fd7f1..1becc774 100644 --- a/website/src/content/docs/docs/install.mdx +++ b/website/src/content/docs/docs/install.mdx @@ -43,7 +43,8 @@ Both packages are released for: | Platform | Architectures | |----------|---------------| | Linux (glibc 2.28 or newer) | x86_64, aarch64 | -| macOS 11 or newer | arm64 | +| macOS 11 or newer | arm64, x86_64 (Intel) | +| Windows 10 or newer, Windows Server 2019 or newer | x86_64 | ## Use it from Python diff --git a/website/src/content/docs/docs/sources/postgres.mdx b/website/src/content/docs/docs/sources/postgres.mdx index 0535ef1f..4a79727f 100644 --- a/website/src/content/docs/docs/sources/postgres.mdx +++ b/website/src/content/docs/docs/sources/postgres.mdx @@ -163,6 +163,23 @@ libpq does by default. That stops an attacker who only relays the password excha who changes how the client signs in. Only `verify-full`, or `verify-ca` on a private network, keeps a man in the middle out. +### Replication sessions on Windows + +A `postgres_cdc` source's replication client speaks plain text only, so its session reaches the +database through a relay inside SQLLoad, which adds TLS and the SSH tunnel. On Linux and macOS the +relay is a Unix socket in a folder only your user can enter. On Windows the replication client can +only connect over TCP, so the relay listens on a random port on 127.0.0.1, for at most 30 seconds. + +- Any local process can connect to that port. SQLLoad makes a random secret for each session and + relays only the connection whose startup message carries it. It drops every other connection + before a byte of it reaches the database, gives each one 5 seconds to send its startup message, + and closes the port once its own client is in. The database never sees the secret. +- What remains: between the replication client and the relay, the sign-in exchange and then every + replicated row cross the loopback interface in plain text. SCRAM never sends the password + itself, but row data can be read by anyone who can capture loopback traffic. On Windows that + takes an administrator, who could also read SQLLoad's memory. +- From the relay to the database, the connection is encrypted as the TLS settings above say. + ## SSH tunnels From 0.27.0. A connection with an `ssh` table reaches the database through a bastion host. `host` @@ -170,9 +187,10 @@ and `port` are then the database as the bastion sees it. To set one up, see [Connect through an SSH tunnel](/docs/guides/postgres/#connect-through-an-ssh-tunnel). - SQLLoad opens the tunnel itself, with no `ssh` program needed. It keeps one session to the - bastion per run, shared by every table, and closes it when the run ends. It opens no local - TCP port: direct reads use the tunnel in process, and a `postgres_cdc` source's replication - session reaches it through a Unix socket in a folder only your user can enter. + bastion per run, shared by every table, and closes it when the run ends. Direct reads use the + tunnel in process. A `postgres_cdc` source's replication session reaches it through a Unix + socket in a folder only your user can enter, or on Windows through a loopback port guarded by a + secret (see [Replication sessions on Windows](#replication-sessions-on-windows)). - It signs in with `private_key_file` (with `private_key_passphrase` if the key has one), or with each key ssh-agent holds when no file is set. From 0.32.0, `password` signs in when no key file is set; before, passwords aren't supported. diff --git a/website/src/content/docs/docs/supported-systems.mdx b/website/src/content/docs/docs/supported-systems.mdx index f5c209a8..62bd6272 100644 --- a/website/src/content/docs/docs/supported-systems.mdx +++ b/website/src/content/docs/docs/supported-systems.mdx @@ -78,5 +78,6 @@ them into DuckDB. ## Platforms The `sqlload` and `sqlload-cli` packages are released for Linux (x86_64 and aarch64, glibc 2.28 or -newer) and macOS 11 or newer (arm64). The `sqlload` package needs CPython 3.10 or newer. See +newer), macOS 11 or newer (arm64 and Intel x86_64) and Windows 10 or Windows Server 2019 or newer +(x86_64). The `sqlload` package needs CPython 3.10 or newer. See [Install](/docs/install/). From 450ef4823cf9875337506790f4c41b1d83b78d18 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 12 Nov 2026 12:21:24 +0000 Subject: [PATCH 02/11] fix: admit the client after refused connections, and move the relay into session_relay A relay refusal no longer disables tokio::select!'s join arm, which could leave an admitted client unread until the relay's deadline when two refusals finished together; a test now opens refused connections first. The Unix socket-folder and Windows loopback relays live in session_relay. On Windows, folder names under a filesystem connection match ignoring case, and file://localhost/C:/ names the drive. Publish's PR trigger also covers the workspace manifest and lockfile. --- .github/workflows/publish.yml | 2 + .../src/adapters/files/main/table_path.rs | 3 +- .../sqlload/src/cdc/_helpers/snapshot_log.rs | 3 +- crates/sqlload/src/cdc/_helpers/stream.rs | 2 +- crates/sqlload/src/lib.rs | 1 + .../_helpers/loopback_relay.rs | 135 -------- .../src/postgres_source/_helpers/mod.rs | 2 - .../sqlload/src/postgres_source/constants.rs | 15 - .../src/postgres_source/main/relay_tls.rs | 63 +--- crates/sqlload/src/postgres_source/models.rs | 10 - crates/sqlload/src/postgres_source/tests.rs | 2 - .../src/postgres_source/tests/helpers.rs | 46 --- .../tests/test_loopback_relay.rs | 304 ------------------ .../src/postgres_source/tests/test_types.rs | 7 - .../src/session_relay/_helpers/loopback.rs | 102 ++++++ .../sqlload/src/session_relay/_helpers/mod.rs | 6 + .../session_relay/_helpers/socket_folder.rs | 36 +++ .../src/session_relay/_helpers/startup.rs | 51 +++ crates/sqlload/src/session_relay/constants.rs | 17 + crates/sqlload/src/session_relay/main/mod.rs | 1 + .../src/session_relay/main/relay_session.rs | 24 ++ crates/sqlload/src/session_relay/mod.rs | 9 + crates/sqlload/src/session_relay/models.rs | 9 + crates/sqlload/src/session_relay/tests.rs | 8 + .../src/session_relay/tests/helpers.rs | 192 +++++++++++ .../tests/test_admitted_startup.rs | 111 +++++++ .../src/session_relay/tests/test_loopback.rs | 54 ++++ .../src/session_relay/tests/test_types.rs | 29 ++ .../src/store_paths/_helpers/local_root.rs | 41 ++- .../src/store_paths/main/resolve_location.rs | 11 +- .../sqlload/src/store_paths/tests/helpers.rs | 7 + .../src/store_paths/tests/test_local_root.rs | 155 ++++++--- .../tests/test_resolve_location.rs | 32 +- .../src/store_paths/tests/test_types.rs | 26 ++ 34 files changed, 869 insertions(+), 647 deletions(-) delete mode 100644 crates/sqlload/src/postgres_source/_helpers/loopback_relay.rs delete mode 100644 crates/sqlload/src/postgres_source/tests/test_loopback_relay.rs create mode 100644 crates/sqlload/src/session_relay/_helpers/loopback.rs create mode 100644 crates/sqlload/src/session_relay/_helpers/mod.rs create mode 100644 crates/sqlload/src/session_relay/_helpers/socket_folder.rs create mode 100644 crates/sqlload/src/session_relay/_helpers/startup.rs create mode 100644 crates/sqlload/src/session_relay/constants.rs create mode 100644 crates/sqlload/src/session_relay/main/mod.rs create mode 100644 crates/sqlload/src/session_relay/main/relay_session.rs create mode 100644 crates/sqlload/src/session_relay/mod.rs create mode 100644 crates/sqlload/src/session_relay/models.rs create mode 100644 crates/sqlload/src/session_relay/tests.rs create mode 100644 crates/sqlload/src/session_relay/tests/helpers.rs create mode 100644 crates/sqlload/src/session_relay/tests/test_admitted_startup.rs create mode 100644 crates/sqlload/src/session_relay/tests/test_loopback.rs create mode 100644 crates/sqlload/src/session_relay/tests/test_types.rs diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index dea80311..eedc1248 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -14,6 +14,8 @@ on: - .github/workflows/publish.yml - .github/scripts/assert_*_wheel_tags.sh - packages/*/pyproject.toml + - Cargo.toml + - Cargo.lock - crates/sqlload/Cargo.toml - crates/sqlload-python/Cargo.toml diff --git a/crates/sqlload/src/adapters/files/main/table_path.rs b/crates/sqlload/src/adapters/files/main/table_path.rs index d5a77935..e35e1bea 100644 --- a/crates/sqlload/src/adapters/files/main/table_path.rs +++ b/crates/sqlload/src/adapters/files/main/table_path.rs @@ -7,7 +7,7 @@ use crate::store_paths::main::local_separators::local_separators; use crate::store_paths::main::render_template::render_template; use crate::store_paths::main::resolve_location::resolve_location; -/// The table's folder and partition folder: placeholders filled, the cursor allowed only in the last folder. +/// The table's folder and partition folder: placeholders filled, the cursor allowed only in the last folder. A leading `/` (or `//` before a Windows share) is kept as the root; a Windows drive (`C:`) is the first folder. pub fn table_path(request: &TablePathRequest<'_>) -> Result { let template = &*local_separators(request.template); if template.contains("://") { @@ -43,7 +43,6 @@ pub fn table_path(request: &TablePathRequest<'_>) -> Result { )); } }; - // `/`, or `//` before a Windows share; a drive (`C:`) is the first folder. let root = &template[..template.len() - template.trim_start_matches('/').len()]; let rendered = render_template(&format!("{root}{}", folder_segments.join("/")), &values) .map_err(|error| error.to_string())?; diff --git a/crates/sqlload/src/cdc/_helpers/snapshot_log.rs b/crates/sqlload/src/cdc/_helpers/snapshot_log.rs index d944e2cd..df5671bd 100644 --- a/crates/sqlload/src/cdc/_helpers/snapshot_log.rs +++ b/crates/sqlload/src/cdc/_helpers/snapshot_log.rs @@ -204,10 +204,9 @@ impl SnapshotLog<'_> { } } -/// The records folder of `source`'s slot `slot` in a files target whose path template is `template`: the template's folders up to the first one naming anything but `{source}`, with `{source}` filled (all but the last when every one is fixed, the table's own folder then), then `_sqlload/cdc///`. Outside every table's folder, and the same whichever tables the source selects; a table's own `destination_path` does not move it. +/// The records folder of `source`'s slot `slot` in a files target whose path template is `template`: the template's folders up to the first one naming anything but `{source}`, with `{source}` filled (all but the last when every one is fixed, the table's own folder then), then `_sqlload/cdc///`. Outside every table's folder, and the same whichever tables the source selects; a table's own `destination_path` does not move it. A leading `/` (or `//` before a Windows share) is kept as the root. pub(crate) fn snapshot_folder(template: &str, source: &str, slot: &str) -> String { let template = &*local_separators(template); - // `/`, or `//` before a Windows share; a drive (`C:`) is the first folder. let root = &template[..template.len() - template.trim_start_matches('/').len()]; let segments: Vec = template .split('/') diff --git a/crates/sqlload/src/cdc/_helpers/stream.rs b/crates/sqlload/src/cdc/_helpers/stream.rs index 50fc1d6a..f7188418 100644 --- a/crates/sqlload/src/cdc/_helpers/stream.rs +++ b/crates/sqlload/src/cdc/_helpers/stream.rs @@ -44,7 +44,6 @@ use crate::loader::models::{ CdcSpec, CommitRule, LoadRun, PlanOptions, ProjectPlan, Publisher, TableLoaded, TableOutcome, TablePlan, WholeReason, }; -use crate::postgres_source::constants::APPLICATION_NAME; use crate::postgres_source::constants::DEFAULT_BATCH_BYTES; use crate::postgres_source::errors::PostgresError; use crate::postgres_source::main::connect_postgres::connect_postgres; @@ -55,6 +54,7 @@ use crate::project::constants::DEFAULT_DESTINATION_PATH; use crate::project::main::postgres_address::postgres_address; use crate::project::models::{SourceConfig, TargetConfig}; use crate::schema_plan::constants::{CDC_LSN_COLUMN, CDC_PREFIX, CDC_UNCHANGED_COLUMN}; +use crate::session_relay::constants::APPLICATION_NAME; use crate::ssh_tunnel::main::tunnel_closed::tunnel_closed; /// How often the replication session reports the confirmed position. diff --git a/crates/sqlload/src/lib.rs b/crates/sqlload/src/lib.rs index 4955a06f..5f66d488 100644 --- a/crates/sqlload/src/lib.rs +++ b/crates/sqlload/src/lib.rs @@ -42,6 +42,7 @@ pub mod routing; pub mod row_batches; pub mod run_log; pub mod schema_plan; +pub mod session_relay; pub mod sheets_source; pub mod similar_names; pub mod snowflake_source; diff --git a/crates/sqlload/src/postgres_source/_helpers/loopback_relay.rs b/crates/sqlload/src/postgres_source/_helpers/loopback_relay.rs deleted file mode 100644 index e4b2e073..00000000 --- a/crates/sqlload/src/postgres_source/_helpers/loopback_relay.rs +++ /dev/null @@ -1,135 +0,0 @@ -//! A relay on a loopback port, where the replication client cannot reach a private socket (Windows). Any local user can connect to the port, so the relay forwards the session only to the connection whose startup message names this relay's secret as its application name, and nothing reaches the server before that check passes. - -use std::net::Ipv4Addr; - -use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt}; -use tokio::net::{TcpListener, TcpStream}; -use tokio::task::JoinSet; - -use crate::auth::models::Secret; -use crate::postgres_source::constants::{ - APPLICATION_NAME, PROTOCOL_VERSION, RELAY_ACCEPT_TIMEOUT, RELAY_STARTUP_TIMEOUT, - STARTUP_MESSAGE_LIMIT, -}; -use crate::postgres_source::models::RelayEndpoint; - -/// A port on 127.0.0.1 that pipes `server` to the first connection presenting the returned application name; it closes once that connection is admitted, or after [`RELAY_ACCEPT_TIMEOUT`] without one. Other connections are dropped, each after [`RELAY_STARTUP_TIMEOUT`] at most, without a byte of theirs reaching the server. -pub(crate) async fn loopback_relay(server: S) -> Result -where - S: AsyncRead + AsyncWrite + Unpin + Send + 'static, -{ - let mut secret = [0u8; 32]; - getrandom::fill(&mut secret).map_err(|error| error.to_string())?; - let expected = Secret(format!("{APPLICATION_NAME}-{}", hex::encode(secret))); - let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)) - .await - .map_err(|error| error.to_string())?; - let port = listener - .local_addr() - .map_err(|error| error.to_string())? - .port(); - tokio::spawn(serve(listener, server, expected.clone())); - Ok(RelayEndpoint { - host: Ipv4Addr::LOCALHOST.to_string(), - port, - application_name: expected, - }) -} - -/// Admit connections to `listener` side by side, so a silent one holds up none of the others, and pipe `server` to the first admitted. -async fn serve(listener: TcpListener, mut server: S, expected: Secret) -where - S: AsyncRead + AsyncWrite + Unpin, -{ - let deadline = tokio::time::Instant::now() + RELAY_ACCEPT_TIMEOUT; - let mut pending = JoinSet::new(); - let (mut local, startup) = loop { - tokio::select! { - accepted = listener.accept() => { - if let Ok((stream, _)) = accepted { - pending.spawn(admit(stream, expected.clone())); - } - } - Some(Ok(Some(admitted))) = pending.join_next() => break admitted, - () = tokio::time::sleep_until(deadline) => return, - } - }; - drop(listener); - drop(pending); - if server.write_all(&startup).await.is_ok() { - let _ = tokio::io::copy_bidirectional(&mut local, &mut server).await; - } -} - -/// `stream` and the startup message to forward for it, when it sends one naming `expected` within [`RELAY_STARTUP_TIMEOUT`]. -async fn admit(mut stream: TcpStream, expected: Secret) -> Option<(TcpStream, Vec)> { - let message = tokio::time::timeout(RELAY_STARTUP_TIMEOUT, read_startup(&mut stream)) - .await - .ok()??; - let forwarded = admitted_startup(&message, expected.expose())?; - let _ = stream.set_nodelay(true); - Some((stream, forwarded)) -} - -/// One startup message: its length, and as many bytes as that gives, up to [`STARTUP_MESSAGE_LIMIT`]. -async fn read_startup(stream: &mut R) -> Option> { - let mut length = [0u8; 4]; - stream.read_exact(&mut length).await.ok()?; - let declared = usize::try_from(u32::from_be_bytes(length)).ok()?; - if !(8..=STARTUP_MESSAGE_LIMIT).contains(&declared) { - return None; - } - let mut message = vec![0; declared]; - message[..4].copy_from_slice(&length); - stream.read_exact(&mut message[4..]).await.ok()?; - Some(message) -} - -/// `message`, a whole protocol 3.0 startup message, with its one `application_name` replaced by [`APPLICATION_NAME`]; `None` unless that name is `expected`. -pub(crate) fn admitted_startup(message: &[u8], expected: &str) -> Option> { - let declared = usize::try_from(u32::from_be_bytes(message.get(..4)?.try_into().ok()?)).ok()?; - if declared != message.len() || message.get(4..8)? != PROTOCOL_VERSION { - return None; - } - // Pairs of NUL-terminated names and values, then one more NUL. - let fields: Vec<&[u8]> = message[8..] - .strip_suffix(b"\0\0")? - .split(|byte| *byte == 0) - .collect(); - if !fields.len().is_multiple_of(2) || fields.iter().step_by(2).any(|name| name.is_empty()) { - return None; - } - let named: Vec = (0..fields.len()) - .step_by(2) - .filter(|index| fields[*index] == b"application_name") - .collect(); - let [name] = named.as_slice() else { - return None; - }; - if !same_secret(fields[name + 1], expected.as_bytes()) { - return None; - } - let mut forwarded = vec![0; 4]; - forwarded.extend_from_slice(&PROTOCOL_VERSION); - for (index, field) in fields.iter().enumerate() { - match index == name + 1 { - true => forwarded.extend_from_slice(APPLICATION_NAME.as_bytes()), - false => forwarded.extend_from_slice(field), - } - forwarded.push(0); - } - forwarded.push(0); - let length = u32::try_from(forwarded.len()).ok()?; - forwarded[..4].copy_from_slice(&length.to_be_bytes()); - Some(forwarded) -} - -/// Whether `given` is `expected`, comparing every byte whatever the first difference, so the time taken tells nothing of the secret. -fn same_secret(given: &[u8], expected: &[u8]) -> bool { - given.len() == expected.len() - && given - .iter() - .zip(expected) - .fold(0u8, |difference, (left, right)| difference | (left ^ right)) - == 0 -} diff --git a/crates/sqlload/src/postgres_source/_helpers/mod.rs b/crates/sqlload/src/postgres_source/_helpers/mod.rs index ca8074c8..d329a9dd 100644 --- a/crates/sqlload/src/postgres_source/_helpers/mod.rs +++ b/crates/sqlload/src/postgres_source/_helpers/mod.rs @@ -2,8 +2,6 @@ pub(crate) mod builders; pub(crate) mod catalog; pub(crate) mod copy_decoder; pub(crate) mod copy_stream; -#[cfg(any(windows, test))] -pub(crate) mod loopback_relay; pub(crate) mod rustls_connect; pub(crate) mod server_reason; pub(crate) mod split_read; diff --git a/crates/sqlload/src/postgres_source/constants.rs b/crates/sqlload/src/postgres_source/constants.rs index 9c6e1cea..e5584da1 100644 --- a/crates/sqlload/src/postgres_source/constants.rs +++ b/crates/sqlload/src/postgres_source/constants.rs @@ -123,21 +123,6 @@ pub const SPLIT_READ_MIN_PAGES: i64 = 16_384; /// Postgres's `SSLRequest`: length 8, then code 80877103. pub const SSL_REQUEST: [u8; 8] = [0, 0, 0, 8, 0x04, 0xd2, 0x16, 0x2f]; -/// How long a TLS relay waits for its replication client to connect. -pub const RELAY_ACCEPT_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30); - -/// How long a loopback relay gives each connection to send its whole startup message, so a silent or slow one is dropped without holding up the replication client. -pub const RELAY_STARTUP_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(5); - -/// The longest startup message a loopback relay reads: Postgres's own limit (`MAX_STARTUP_PACKET_LENGTH`). -pub const STARTUP_MESSAGE_LIMIT: usize = 10_000; - -/// Protocol 3.0, as the replication client's startup message gives it. -pub const PROTOCOL_VERSION: [u8; 4] = [0, 3, 0, 0]; - -/// The application name SQLLoad's sessions show the server. -pub const APPLICATION_NAME: &str = "sqlload"; - /// Signature algorithm OIDs (DER contents) whose certificates bind with SHA-256: sha256WithRSAEncryption, sha1WithRSAEncryption, md5WithRSAEncryption, ecdsa-with-SHA256, ecdsa-with-SHA1. pub const SHA256_SIGNATURES: &[&[u8]] = &[ &[0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0b], diff --git a/crates/sqlload/src/postgres_source/main/relay_tls.rs b/crates/sqlload/src/postgres_source/main/relay_tls.rs index 6d8b10ec..232255c4 100644 --- a/crates/sqlload/src/postgres_source/main/relay_tls.rs +++ b/crates/sqlload/src/postgres_source/main/relay_tls.rs @@ -1,22 +1,15 @@ //! TLS for a client that speaks only plain text: the replication client. -#[cfg(unix)] -use std::os::unix::fs::PermissionsExt; - use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt}; use tokio::net::TcpStream; -#[cfg(unix)] -use tokio::net::UnixListener; use tokio_rustls::TlsConnector; -#[cfg(windows)] -use crate::postgres_source::_helpers::loopback_relay::loopback_relay; use crate::postgres_source::_helpers::tls::client_config; use crate::postgres_source::constants::SSL_REQUEST; -#[cfg(unix)] -use crate::postgres_source::constants::{APPLICATION_NAME, RELAY_ACCEPT_TIMEOUT}; use crate::postgres_source::errors::PostgresError; -use crate::postgres_source::models::{PostgresAddress, RelayEndpoint, SslMode}; +use crate::postgres_source::models::{PostgresAddress, SslMode}; +use crate::session_relay::main::relay_session::relay_session; +use crate::session_relay::models::RelayEndpoint; use crate::ssh_tunnel::main::open_channel::open_channel; /// When `address` may use TLS or goes through a bastion: where the replication client connects (a private socket folder whose `.s.PGSQL.` socket, or on Windows a loopback port, carries the next session opened on it to the server, encrypted, or in plain text when the server refuses TLS under `prefer` (one connection either way, so the server logs no abandoned one), and the application name it must send. `None` to connect directly: `sslmode = "disable"`, a Unix socket host, or a failed handshake under `prefer`, which libpq also retries in plain text. Through a bastion it never connects directly: that retry opens a fresh channel. @@ -35,7 +28,9 @@ pub async fn relay_tls( } } let server = open_channel(tunnel, address.host, address.port).await?; - return Ok(Some(relay(server, address.port).await.map_err(failed)?)); + return Ok(Some( + relay_session(server, address.port).await.map_err(failed)?, + )); } if address.tls.mode == SslMode::Disable || address.host.starts_with('/') { return Ok(None); @@ -73,7 +68,9 @@ where != b'S' { return match address.tls.mode { - SslMode::Prefer => Ok(Some(relay(server, address.port).await.map_err(failed)?)), + SslMode::Prefer => Ok(Some( + relay_session(server, address.port).await.map_err(failed)?, + )), _ => Err(failed("the server does not offer TLS".to_owned())), }; } @@ -84,43 +81,7 @@ where Err(_) if address.tls.mode == SslMode::Prefer => return Ok(None), Err(error) => return Err(failed(format!("TLS handshake failed: {error}"))), }; - Ok(Some(relay(session, address.port).await.map_err(failed)?)) -} - -/// A folder only this user can enter (some systems ignore a socket's own permissions) whose `.s.PGSQL.` socket pipes the first session opened on it through `server`; the folder and its socket go once it is accepted, or after [`RELAY_ACCEPT_TIMEOUT`] without one. -#[cfg(unix)] -async fn relay(mut server: S, port: u16) -> Result -where - S: AsyncRead + AsyncWrite + Unpin + Send + 'static, -{ - let folder = tempfile::Builder::new() - .prefix("sqlload-") - .permissions(std::fs::Permissions::from_mode(0o700)) - .tempdir() - .map_err(|error| error.to_string())?; - let listener = UnixListener::bind(folder.path().join(format!(".s.PGSQL.{port}"))) - .map_err(|error| error.to_string())?; - let host = folder.path().to_string_lossy().into_owned(); - tokio::spawn(async move { - let accepted = tokio::time::timeout(RELAY_ACCEPT_TIMEOUT, listener.accept()).await; - drop(listener); - drop(folder); - if let Ok(Ok((mut local, _))) = accepted { - let _ = tokio::io::copy_bidirectional(&mut local, &mut server).await; - } - }); - Ok(RelayEndpoint { - host, - port, - application_name: crate::auth::models::Secret(APPLICATION_NAME.to_owned()), - }) -} - -/// pgwire-replication reaches no socket file on Windows, only TCP, so the session goes through a loopback port that admits only this relay's secret (see [`loopback_relay`]). -#[cfg(windows)] -async fn relay(server: S, _port: u16) -> Result -where - S: AsyncRead + AsyncWrite + Unpin + Send + 'static, -{ - loopback_relay(server).await + Ok(Some( + relay_session(session, address.port).await.map_err(failed)?, + )) } diff --git a/crates/sqlload/src/postgres_source/models.rs b/crates/sqlload/src/postgres_source/models.rs index c1e8ccb0..950b3013 100644 --- a/crates/sqlload/src/postgres_source/models.rs +++ b/crates/sqlload/src/postgres_source/models.rs @@ -192,13 +192,3 @@ pub struct CopyRequest<'a> { /// More connections to the same database, idle and outside any transaction, to read a large table in parallel; empty reads in one pass. pub helpers: &'a [tokio_postgres::Client], } - -/// Where the replication client reaches a relayed session, and the application name its startup message must carry there. -#[derive(Debug)] -pub struct RelayEndpoint { - /// The relay's socket folder (Unix) or `127.0.0.1` (Windows). - pub host: String, - pub port: u16, - /// `sqlload` on Unix. On Windows `sqlload-`, which the relay checks and replaces with `sqlload` before the server sees it. - pub application_name: crate::auth::models::Secret, -} diff --git a/crates/sqlload/src/postgres_source/tests.rs b/crates/sqlload/src/postgres_source/tests.rs index 03a616d3..934df596 100644 --- a/crates/sqlload/src/postgres_source/tests.rs +++ b/crates/sqlload/src/postgres_source/tests.rs @@ -4,8 +4,6 @@ mod copy_decoder; mod decode_copy; #[path = "tests/helpers.rs"] mod helpers; -#[path = "tests/test_loopback_relay.rs"] -mod loopback_relay; #[path = "tests/test_read_column.rs"] mod read_column; #[path = "tests/test_read_numeric.rs"] diff --git a/crates/sqlload/src/postgres_source/tests/helpers.rs b/crates/sqlload/src/postgres_source/tests/helpers.rs index dbe21b06..4cd42e8b 100644 --- a/crates/sqlload/src/postgres_source/tests/helpers.rs +++ b/crates/sqlload/src/postgres_source/tests/helpers.rs @@ -96,49 +96,3 @@ pub(crate) fn source_column(pg_type: PgType) -> SourceColumn { source_spelling: None, } } - -/// A startup message of protocol `version` with `params`, framed as libpq and pgwire-replication frame it. -pub(crate) fn startup_message(version: [u8; 4], params: &[(&str, &str)]) -> Vec { - let mut body = version.to_vec(); - for (name, value) in params { - body.extend_from_slice(name.as_bytes()); - body.push(0); - body.extend_from_slice(value.as_bytes()); - body.push(0); - } - body.push(0); - let length = u32::try_from(body.len() + 4).unwrap_or(u32::MAX); - let mut message = length.to_be_bytes().to_vec(); - message.extend_from_slice(&body); - message -} - -/// The parameters of a well-formed protocol 3.0 startup message, in order; an error naming what is wrong otherwise. -pub(crate) fn startup_params(message: &[u8]) -> Result, String> { - let declared = message - .get(..4) - .and_then(|bytes| <[u8; 4]>::try_from(bytes).ok()) - .map(u32::from_be_bytes) - .ok_or("no length")?; - if usize::try_from(declared).ok() != Some(message.len()) { - return Err(format!("length {declared} for {} bytes", message.len())); - } - if message.get(4..8) != Some(&[0, 3, 0, 0][..]) { - return Err("not protocol 3.0".to_owned()); - } - let text = String::from_utf8(message[8..].to_vec()).map_err(|error| error.to_string())?; - let mut parts: Vec<&str> = text.split('\0').collect(); - // The pairs' terminators and the final NUL leave two empty parts at the end. - if parts.len() < 2 || parts.pop() != Some("") || parts.pop() != Some("") { - return Err("not NUL-terminated".to_owned()); - } - Ok(parts - .chunks(2) - .map(|pair| { - ( - pair[0].to_owned(), - pair.get(1).copied().unwrap_or_default().to_owned(), - ) - }) - .collect()) -} diff --git a/crates/sqlload/src/postgres_source/tests/test_loopback_relay.rs b/crates/sqlload/src/postgres_source/tests/test_loopback_relay.rs deleted file mode 100644 index 8a2645c2..00000000 --- a/crates/sqlload/src/postgres_source/tests/test_loopback_relay.rs +++ /dev/null @@ -1,304 +0,0 @@ -use std::time::{Duration, Instant}; - -use tokio::io::{AsyncReadExt, AsyncWriteExt, DuplexStream}; -use tokio::net::TcpStream; - -use crate::postgres_source::_helpers::loopback_relay::{admitted_startup, loopback_relay}; -use crate::postgres_source::constants::{PROTOCOL_VERSION, RELAY_STARTUP_TIMEOUT}; -use crate::postgres_source::models::RelayEndpoint; -use crate::postgres_source::tests::helpers::{startup_message, startup_params}; -use crate::postgres_source::tests::test_types::AdmittedStartupTestCase; - -const SECRET: &str = "sqlload-6f1c0d2a9b8e7f6a5b4c3d2e1f0a9b8c7d6e5f4a3b2c1d0e9f8a7b6c5d4e3f2a"; - -fn block_on(body: impl std::future::Future>) -> Result<(), String> { - tokio::runtime::Builder::new_multi_thread() - .worker_threads(2) - .enable_all() - .build() - .map_err(|error| error.to_string())? - .block_on(body) -} - -fn owned(params: &[(&str, &str)]) -> Vec<(String, String)> { - params - .iter() - .map(|(name, value)| ((*name).to_owned(), (*value).to_owned())) - .collect() -} - -/// The replication client's startup message, naming `application_name`. -fn client_startup(application_name: &str) -> Vec { - startup_message( - PROTOCOL_VERSION, - &[ - ("user", "replicator"), - ("database", "shop"), - ("replication", "database"), - ("application_name", application_name), - ], - ) -} - -/// The whole first message the server side of the relay received. -async fn read_forwarded(upstream: &mut DuplexStream) -> Result, String> { - let mut length = [0u8; 4]; - upstream - .read_exact(&mut length) - .await - .map_err(|error| error.to_string())?; - let declared = - usize::try_from(u32::from_be_bytes(length)).map_err(|error| error.to_string())?; - let mut message = length.to_vec(); - message.resize(declared.max(4), 0); - upstream - .read_exact(&mut message[4..]) - .await - .map_err(|error| error.to_string())?; - Ok(message) -} - -fn contains(haystack: &[u8], needle: &str) -> bool { - haystack - .windows(needle.len()) - .any(|window| window == needle.as_bytes()) -} - -async fn connect(endpoint: &RelayEndpoint) -> Result { - TcpStream::connect((endpoint.host.as_str(), endpoint.port)) - .await - .map_err(|error| error.to_string()) -} - -/// Whether the relay closes `stream` without answering, within `wait`. -async fn closed_unanswered(stream: &mut TcpStream, wait: Duration) -> Result { - let mut byte = [0u8; 1]; - match tokio::time::timeout(wait, stream.read(&mut byte)).await { - Ok(Ok(0) | Err(_)) => Ok(true), - Ok(Ok(_)) => Ok(false), - Err(_) => Err("the relay kept the connection open".to_owned()), - } -} - -#[test] -fn given_startup_messages_when_the_relay_checks_them_then_only_the_secret_is_admitted_and_never_forwarded() --> Result<(), String> { - let wrong_last = format!("{}0", &SECRET[..SECRET.len() - 1]); - let mut short_length = client_startup(SECRET); - short_length[3] -= 1; - let mut no_terminator = client_startup(SECRET); - no_terminator.pop(); - no_terminator[3] -= 1; - let test_cases = [ - AdmittedStartupTestCase { - description: "the secret, among the other parameters", - message: client_startup(SECRET), - expected_forwarded: Some(owned(&[ - ("user", "replicator"), - ("database", "shop"), - ("replication", "database"), - ("application_name", "sqlload"), - ])), - }, - AdmittedStartupTestCase { - description: "the secret first, before an empty value", - message: startup_message( - PROTOCOL_VERSION, - &[("application_name", SECRET), ("options", ""), ("user", "u")], - ), - expected_forwarded: Some(owned(&[ - ("application_name", "sqlload"), - ("options", ""), - ("user", "u"), - ])), - }, - AdmittedStartupTestCase { - description: "a secret wrong in its last character", - message: client_startup(&wrong_last), - expected_forwarded: None, - }, - AdmittedStartupTestCase { - description: "a prefix of the secret", - message: client_startup(&SECRET[..SECRET.len() - 1]), - expected_forwarded: None, - }, - AdmittedStartupTestCase { - description: "the secret with more after it", - message: client_startup(&format!("{SECRET}0")), - expected_forwarded: None, - }, - AdmittedStartupTestCase { - description: "the plain application name", - message: client_startup("sqlload"), - expected_forwarded: None, - }, - AdmittedStartupTestCase { - description: "no application name", - message: startup_message(PROTOCOL_VERSION, &[("user", "u"), ("database", "d")]), - expected_forwarded: None, - }, - AdmittedStartupTestCase { - description: "the secret, then a second application name", - message: startup_message( - PROTOCOL_VERSION, - &[("application_name", SECRET), ("application_name", "other")], - ), - expected_forwarded: None, - }, - AdmittedStartupTestCase { - description: "an SSLRequest", - message: vec![0, 0, 0, 8, 0x04, 0xd2, 0x16, 0x2f], - expected_forwarded: None, - }, - AdmittedStartupTestCase { - description: "protocol 2.0 with the secret", - message: startup_message([0, 2, 0, 0], &[("application_name", SECRET)]), - expected_forwarded: None, - }, - AdmittedStartupTestCase { - description: "a length one short of the message", - message: short_length, - expected_forwarded: None, - }, - AdmittedStartupTestCase { - description: "no final NUL", - message: no_terminator, - expected_forwarded: None, - }, - ]; - for test_case in test_cases { - let forwarded = admitted_startup(&test_case.message, SECRET); - let found = forwarded.as_deref().map(startup_params).transpose()?; - if found != test_case.expected_forwarded { - return Err(format!( - "{}: forwarded {found:?}, expected {:?}", - test_case.description, test_case.expected_forwarded - )); - } - if forwarded.is_some_and(|bytes| contains(&bytes, &SECRET["sqlload-".len()..])) { - return Err(format!( - "{}: the secret was forwarded", - test_case.description - )); - } - } - Ok(()) -} - -#[test] -fn given_a_silent_connection_first_when_the_replication_client_connects_then_it_is_relayed_without_waiting() --> Result<(), String> { - block_on(async { - let (server, mut upstream) = tokio::io::duplex(64 * 1024); - let endpoint = loopback_relay(server).await?; - let _silent = connect(&endpoint).await?; - let started = Instant::now(); - let mut client = connect(&endpoint).await?; - client - .write_all(&client_startup(endpoint.application_name.expose())) - .await - .map_err(|error| error.to_string())?; - let forwarded = tokio::time::timeout(RELAY_STARTUP_TIMEOUT, read_forwarded(&mut upstream)) - .await - .map_err(|_| "the client was held up behind the silent connection".to_owned())??; - if started.elapsed() >= RELAY_STARTUP_TIMEOUT { - return Err(format!("relayed after {:?}", started.elapsed())); - } - let name = startup_params(&forwarded)? - .into_iter() - .find(|(name, _)| name == "application_name"); - if name != Some(("application_name".to_owned(), "sqlload".to_owned())) { - return Err(format!("forwarded application name {name:?}")); - } - // The session is piped both ways once admitted. - upstream - .write_all(b"R") - .await - .map_err(|error| error.to_string())?; - let mut answer = [0u8; 1]; - client - .read_exact(&mut answer) - .await - .map_err(|error| error.to_string())?; - client - .write_all(b"p") - .await - .map_err(|error| error.to_string())?; - upstream - .read_exact(&mut answer) - .await - .map_err(|error| error.to_string())?; - match &answer { - b"p" => Ok(()), - other => Err(format!("piped {other:?}")), - } - }) -} - -#[test] -fn given_connections_without_the_secret_when_they_connect_then_they_are_dropped_and_the_server_receives_nothing() --> Result<(), String> { - block_on(async { - let (server, mut upstream) = tokio::io::duplex(64 * 1024); - let endpoint = loopback_relay(server).await?; - let secret = endpoint.application_name.expose().to_owned(); - if format!("{endpoint:?}").contains(&secret) { - return Err("the relay's Debug shows its secret".to_owned()); - } - let wrong = format!("sqlload-{}", "0".repeat(64)); - let attempts: [(&str, Vec); 4] = [ - ("a wrong secret", client_startup(&wrong)), - ("the plain application name", client_startup("sqlload")), - ("an oversize length", vec![0, 1, 0, 0, 0, 3, 0, 0]), - ("an SSLRequest", vec![0, 0, 0, 8, 0x04, 0xd2, 0x16, 0x2f]), - ]; - for (description, bytes) in attempts { - let mut stream = connect(&endpoint).await?; - stream - .write_all(&bytes) - .await - .map_err(|error| error.to_string())?; - if !closed_unanswered(&mut stream, RELAY_STARTUP_TIMEOUT * 2) - .await - .map_err(|error| format!("{description}: {error}"))? - { - return Err(format!("{description}: the relay answered")); - } - } - let mut byte = [0u8; 1]; - if tokio::time::timeout(Duration::from_millis(300), upstream.read(&mut byte)) - .await - .is_ok() - { - return Err("the server received bytes from a refused connection".to_owned()); - } - let mut client = connect(&endpoint).await?; - client - .write_all(&client_startup(&secret)) - .await - .map_err(|error| error.to_string())?; - let forwarded = read_forwarded(&mut upstream).await?; - if contains(&forwarded, &secret["sqlload-".len()..]) { - return Err("the secret reached the server".to_owned()); - } - let expected = owned(&[ - ("user", "replicator"), - ("database", "shop"), - ("replication", "database"), - ("application_name", "sqlload"), - ]); - let found = startup_params(&forwarded)?; - if found != expected { - return Err(format!("forwarded {found:?}")); - } - // Admitted once: the port is closed to every later connection. - match tokio::time::timeout(Duration::from_secs(10), connect(&endpoint)).await { - Ok(Err(_)) => Ok(()), - Ok(Ok(mut late)) => match closed_unanswered(&mut late, Duration::from_secs(2)).await { - Ok(true) => Ok(()), - _ => Err("a connection after the admitted one was kept".to_owned()), - }, - Err(_) => Err("connecting after admission hung".to_owned()), - } - }) -} diff --git a/crates/sqlload/src/postgres_source/tests/test_types.rs b/crates/sqlload/src/postgres_source/tests/test_types.rs index 87899d09..4443f9d2 100644 --- a/crates/sqlload/src/postgres_source/tests/test_types.rs +++ b/crates/sqlload/src/postgres_source/tests/test_types.rs @@ -37,10 +37,3 @@ pub(crate) struct ReadColumnTestCase { /// The select expression, or the error text. pub(crate) expected_result: Result<&'static str, &'static str>, } - -pub(crate) struct AdmittedStartupTestCase { - pub(crate) description: &'static str, - pub(crate) message: Vec, - /// The parameters forwarded to the server, in order; `None` when the connection is refused. - pub(crate) expected_forwarded: Option>, -} diff --git a/crates/sqlload/src/session_relay/_helpers/loopback.rs b/crates/sqlload/src/session_relay/_helpers/loopback.rs new file mode 100644 index 00000000..eb4650a2 --- /dev/null +++ b/crates/sqlload/src/session_relay/_helpers/loopback.rs @@ -0,0 +1,102 @@ +//! A relay on a loopback port, where the client cannot reach a private socket (Windows). Any local user can connect to the port, so the relay forwards the session only to the connection whose startup message names this relay's secret as its application name, and nothing reaches the server before that check passes. + +use std::net::Ipv4Addr; + +use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt}; +use tokio::net::{TcpListener, TcpStream}; +use tokio::task::JoinSet; + +use crate::auth::models::Secret; +use crate::session_relay::_helpers::startup::admitted_startup; +use crate::session_relay::constants::{ + APPLICATION_NAME, RELAY_ACCEPT_TIMEOUT, RELAY_STARTUP_TIMEOUT, STARTUP_MESSAGE_LIMIT, + STARTUP_MESSAGE_MINIMUM, +}; +use crate::session_relay::models::RelayEndpoint; + +/// A port on 127.0.0.1 that pipes `server` to the first connection presenting the returned application name; it closes once that connection is admitted, or after [`RELAY_ACCEPT_TIMEOUT`] without one. Other connections are dropped, each after [`RELAY_STARTUP_TIMEOUT`] at most, without a byte of theirs reaching the server. +pub(crate) async fn loopback_relay(server: S) -> Result +where + S: AsyncRead + AsyncWrite + Unpin + Send + 'static, +{ + let mut secret = [0u8; 32]; + getrandom::fill(&mut secret).map_err(|error| error.to_string())?; + let expected = Secret(format!("{APPLICATION_NAME}-{}", hex::encode(secret))); + let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)) + .await + .map_err(|error| error.to_string())?; + let port = listener + .local_addr() + .map_err(|error| error.to_string())? + .port(); + tokio::spawn(serve(listener, server, expected.clone())); + Ok(RelayEndpoint { + host: Ipv4Addr::LOCALHOST.to_string(), + port, + application_name: expected, + }) +} + +/// Admit connections to `listener` side by side, so a silent one holds up none of the others, and pipe `server` to the first admitted. A refused connection's result is matched inside its arm: a pattern on the arm itself would disable it for the rest of the `select!` after one refusal. +async fn serve(listener: TcpListener, mut server: S, expected: Secret) +where + S: AsyncRead + AsyncWrite + Unpin, +{ + let deadline = tokio::time::Instant::now() + RELAY_ACCEPT_TIMEOUT; + let mut pending = JoinSet::new(); + let (mut local, startup) = loop { + tokio::select! { + accepted = listener.accept() => { + if let Ok((stream, _)) = accepted { + pending.spawn(admit(stream, expected.clone())); + } + } + Some(joined) = pending.join_next() => { + if let Ok(Ok(admitted)) = joined { + break admitted; + } + } + () = tokio::time::sleep_until(deadline) => return, + } + }; + drop(listener); + drop(pending); + let _ = async { + server.write_all(&startup).await?; + tokio::io::copy_bidirectional(&mut local, &mut server).await + } + .await; +} + +/// `stream` and the startup message to forward for it, when it sends one naming `expected` within [`RELAY_STARTUP_TIMEOUT`]; why it is refused otherwise. +async fn admit(mut stream: TcpStream, expected: Secret) -> Result<(TcpStream, Vec), String> { + let message = tokio::time::timeout(RELAY_STARTUP_TIMEOUT, read_startup(&mut stream)) + .await + .map_err(|_| "no startup message in time".to_owned())??; + let forwarded = admitted_startup(&message, expected.expose())?; + stream + .set_nodelay(true) + .map_err(|error| error.to_string())?; + Ok((stream, forwarded)) +} + +/// One startup message: its length, and as many bytes as that gives, from [`STARTUP_MESSAGE_MINIMUM`] to [`STARTUP_MESSAGE_LIMIT`]. +async fn read_startup(stream: &mut R) -> Result, String> { + let mut length = [0u8; 4]; + stream + .read_exact(&mut length) + .await + .map_err(|error| error.to_string())?; + let declared = + usize::try_from(u32::from_be_bytes(length)).map_err(|error| error.to_string())?; + if !(STARTUP_MESSAGE_MINIMUM..=STARTUP_MESSAGE_LIMIT).contains(&declared) { + return Err(format!("a startup message of {declared} bytes")); + } + let mut message = vec![0; declared]; + message[..4].copy_from_slice(&length); + stream + .read_exact(&mut message[4..]) + .await + .map_err(|error| error.to_string())?; + Ok(message) +} diff --git a/crates/sqlload/src/session_relay/_helpers/mod.rs b/crates/sqlload/src/session_relay/_helpers/mod.rs new file mode 100644 index 00000000..acc6568f --- /dev/null +++ b/crates/sqlload/src/session_relay/_helpers/mod.rs @@ -0,0 +1,6 @@ +#[cfg(any(windows, test))] +pub(crate) mod loopback; +#[cfg(unix)] +pub(crate) mod socket_folder; +#[cfg(any(windows, test))] +pub(crate) mod startup; diff --git a/crates/sqlload/src/session_relay/_helpers/socket_folder.rs b/crates/sqlload/src/session_relay/_helpers/socket_folder.rs new file mode 100644 index 00000000..9846e61e --- /dev/null +++ b/crates/sqlload/src/session_relay/_helpers/socket_folder.rs @@ -0,0 +1,36 @@ +use std::os::unix::fs::PermissionsExt; + +use tokio::io::{AsyncRead, AsyncWrite}; +use tokio::net::UnixListener; + +use crate::auth::models::Secret; +use crate::session_relay::constants::{APPLICATION_NAME, RELAY_ACCEPT_TIMEOUT}; +use crate::session_relay::models::RelayEndpoint; + +/// A folder only this user can enter (some systems ignore a socket's own permissions) whose `.s.PGSQL.` socket pipes the first session opened on it through `server`; the folder and its socket go once it is accepted, or after [`RELAY_ACCEPT_TIMEOUT`] without one. +pub(crate) fn socket_folder_relay(mut server: S, port: u16) -> Result +where + S: AsyncRead + AsyncWrite + Unpin + Send + 'static, +{ + let folder = tempfile::Builder::new() + .prefix("sqlload-") + .permissions(std::fs::Permissions::from_mode(0o700)) + .tempdir() + .map_err(|error| error.to_string())?; + let listener = UnixListener::bind(folder.path().join(format!(".s.PGSQL.{port}"))) + .map_err(|error| error.to_string())?; + let host = folder.path().to_string_lossy().into_owned(); + tokio::spawn(async move { + let accepted = tokio::time::timeout(RELAY_ACCEPT_TIMEOUT, listener.accept()).await; + drop(listener); + drop(folder); + if let Ok(Ok((mut local, _))) = accepted { + let _ = tokio::io::copy_bidirectional(&mut local, &mut server).await; + } + }); + Ok(RelayEndpoint { + host, + port, + application_name: Secret(APPLICATION_NAME.to_owned()), + }) +} diff --git a/crates/sqlload/src/session_relay/_helpers/startup.rs b/crates/sqlload/src/session_relay/_helpers/startup.rs new file mode 100644 index 00000000..3e40ec02 --- /dev/null +++ b/crates/sqlload/src/session_relay/_helpers/startup.rs @@ -0,0 +1,51 @@ +use crate::session_relay::constants::{APPLICATION_NAME, PROTOCOL_VERSION}; + +/// `message`, a whole protocol 3.0 startup message (its length, the version, then pairs of NUL-terminated names and values and one more NUL), with its one `application_name` replaced by [`APPLICATION_NAME`]; refused unless that name is `expected`. The forwarded message is rebuilt from the parsed fields, so it is well formed whatever was sent. +pub(crate) fn admitted_startup(message: &[u8], expected: &str) -> Result, &'static str> { + let declared = u32::from_be_bytes(*message.first_chunk::<4>().ok_or("no length")?); + let length = u32::try_from(message.len()).map_err(|_| "too long")?; + if declared != length || message.get(4..8) != Some(&PROTOCOL_VERSION[..]) { + return Err("not one protocol 3.0 startup message"); + } + let fields: Vec<&[u8]> = message[8..] + .strip_suffix(b"\0\0") + .ok_or("not NUL-terminated")? + .split(|byte| *byte == 0) + .collect(); + if !fields.len().is_multiple_of(2) || fields.iter().step_by(2).any(|name| name.is_empty()) { + return Err("not pairs of names and values"); + } + let named: Vec = (0..fields.len()) + .step_by(2) + .filter(|index| fields[*index] == b"application_name") + .collect(); + let [name] = named.as_slice() else { + return Err("not one application name"); + }; + if !same_secret(fields[name + 1], expected.as_bytes()) { + return Err("not this relay's application name"); + } + let mut forwarded = vec![0; 4]; + forwarded.extend_from_slice(&PROTOCOL_VERSION); + for (index, field) in fields.iter().enumerate() { + match index == name + 1 { + true => forwarded.extend_from_slice(APPLICATION_NAME.as_bytes()), + false => forwarded.extend_from_slice(field), + } + forwarded.push(0); + } + forwarded.push(0); + let length = u32::try_from(forwarded.len()).map_err(|_| "too long")?; + forwarded[..4].copy_from_slice(&length.to_be_bytes()); + Ok(forwarded) +} + +/// Whether `given` is `expected`, comparing every byte whatever the first difference, so the time taken tells nothing of the secret. +fn same_secret(given: &[u8], expected: &[u8]) -> bool { + given.len() == expected.len() + && given + .iter() + .zip(expected) + .fold(0u8, |difference, (left, right)| difference | (left ^ right)) + == 0 +} diff --git a/crates/sqlload/src/session_relay/constants.rs b/crates/sqlload/src/session_relay/constants.rs new file mode 100644 index 00000000..de4b46ad --- /dev/null +++ b/crates/sqlload/src/session_relay/constants.rs @@ -0,0 +1,17 @@ +/// How long a relay waits for its client to connect. +pub const RELAY_ACCEPT_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30); + +/// How long a loopback relay gives each connection to send its whole startup message, so a silent or slow one is dropped without holding up the client. +pub const RELAY_STARTUP_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(5); + +/// The shortest startup message: its length and the protocol version. +pub const STARTUP_MESSAGE_MINIMUM: usize = 8; + +/// The longest startup message a loopback relay reads: Postgres's own limit (`MAX_STARTUP_PACKET_LENGTH`). +pub const STARTUP_MESSAGE_LIMIT: usize = 10_000; + +/// Protocol 3.0, as the replication client's startup message gives it. +pub const PROTOCOL_VERSION: [u8; 4] = [0, 3, 0, 0]; + +/// The application name SQLLoad's sessions show the server. +pub const APPLICATION_NAME: &str = "sqlload"; diff --git a/crates/sqlload/src/session_relay/main/mod.rs b/crates/sqlload/src/session_relay/main/mod.rs new file mode 100644 index 00000000..777cfca0 --- /dev/null +++ b/crates/sqlload/src/session_relay/main/mod.rs @@ -0,0 +1 @@ +pub mod relay_session; diff --git a/crates/sqlload/src/session_relay/main/relay_session.rs b/crates/sqlload/src/session_relay/main/relay_session.rs new file mode 100644 index 00000000..bd9803b1 --- /dev/null +++ b/crates/sqlload/src/session_relay/main/relay_session.rs @@ -0,0 +1,24 @@ +//! Open a relay that pipes one session through a connection SQLLoad opened. + +use tokio::io::{AsyncRead, AsyncWrite}; + +#[cfg(windows)] +use crate::session_relay::_helpers::loopback::loopback_relay; +#[cfg(unix)] +use crate::session_relay::_helpers::socket_folder::socket_folder_relay; +use crate::session_relay::models::RelayEndpoint; + +/// Where the client reaches `server` (the session to a Postgres server listening on `port`): on Unix a `.s.PGSQL.` socket in a folder only this user can enter; on Windows, where pgwire-replication reaches no socket file, a 127.0.0.1 port that admits only the connection presenting the returned application name. Either serves the first session admitted and closes, or closes after [`RELAY_ACCEPT_TIMEOUT`](crate::session_relay::constants::RELAY_ACCEPT_TIMEOUT) without one. +pub async fn relay_session(server: S, port: u16) -> Result +where + S: AsyncRead + AsyncWrite + Unpin + Send + 'static, +{ + #[cfg(unix)] + let relayed = socket_folder_relay(server, port); + #[cfg(windows)] + let relayed = { + let _ = port; + loopback_relay(server).await + }; + relayed +} diff --git a/crates/sqlload/src/session_relay/mod.rs b/crates/sqlload/src/session_relay/mod.rs new file mode 100644 index 00000000..aa289d7e --- /dev/null +++ b/crates/sqlload/src/session_relay/mod.rs @@ -0,0 +1,9 @@ +//! Where a client that speaks only plain text, such as the replication client, reaches a session SQLLoad has opened for it: a private socket on Unix, a loopback port that admits only its own client on Windows. + +pub(crate) mod _helpers; +pub mod constants; +pub mod main; +pub mod models; + +#[cfg(test)] +mod tests; diff --git a/crates/sqlload/src/session_relay/models.rs b/crates/sqlload/src/session_relay/models.rs new file mode 100644 index 00000000..667ab0be --- /dev/null +++ b/crates/sqlload/src/session_relay/models.rs @@ -0,0 +1,9 @@ +/// Where the client reaches a relayed session, and the application name its startup message must carry there. +#[derive(Debug)] +pub struct RelayEndpoint { + /// The relay's socket folder (Unix) or `127.0.0.1` (Windows). + pub host: String, + pub port: u16, + /// `sqlload` on Unix. On Windows `sqlload-`, which the relay checks and replaces with `sqlload` before the server sees it. + pub application_name: crate::auth::models::Secret, +} diff --git a/crates/sqlload/src/session_relay/tests.rs b/crates/sqlload/src/session_relay/tests.rs new file mode 100644 index 00000000..af154ab7 --- /dev/null +++ b/crates/sqlload/src/session_relay/tests.rs @@ -0,0 +1,8 @@ +#[path = "tests/test_admitted_startup.rs"] +mod admitted_startup; +#[path = "tests/helpers.rs"] +mod helpers; +#[path = "tests/test_loopback.rs"] +mod loopback; +#[path = "tests/test_types.rs"] +mod test_types; diff --git a/crates/sqlload/src/session_relay/tests/helpers.rs b/crates/sqlload/src/session_relay/tests/helpers.rs new file mode 100644 index 00000000..c2ff3152 --- /dev/null +++ b/crates/sqlload/src/session_relay/tests/helpers.rs @@ -0,0 +1,192 @@ +use std::time::Duration; + +use tokio::io::{AsyncReadExt, AsyncWriteExt, DuplexStream}; +use tokio::net::TcpStream; + +use crate::session_relay::_helpers::loopback::loopback_relay; +use crate::session_relay::constants::{PROTOCOL_VERSION, RELAY_STARTUP_TIMEOUT}; +use crate::session_relay::tests::test_types::RelayOutcome; + +/// A relay secret as the relay makes them: `sqlload-` and 64 hex digits. +pub(crate) const SECRET: &str = + "sqlload-6f1c0d2a9b8e7f6a5b4c3d2e1f0a9b8c7d6e5f4a3b2c1d0e9f8a7b6c5d4e3f2a"; + +/// Postgres's `SSLRequest`. +pub(crate) const SSL_REQUEST: [u8; 8] = [0, 0, 0, 8, 0x04, 0xd2, 0x16, 0x2f]; + +/// What the server receives for the replication client's startup message. +pub(crate) const FORWARDED: &str = + "user=replicator,database=shop,replication=database,application_name=sqlload"; + +pub(crate) fn block_on( + body: impl std::future::Future>, +) -> Result { + tokio::runtime::Builder::new_multi_thread() + .worker_threads(2) + .enable_all() + .build() + .map_err(|error| error.to_string())? + .block_on(body) +} + +/// A message of protocol `version` whose body after the version is `body`, framed with its length. +pub(crate) fn framed(version: [u8; 4], body: &[u8]) -> Vec { + let length = u32::try_from(body.len() + 8).unwrap_or(u32::MAX); + [&length.to_be_bytes()[..], &version, body].concat() +} + +/// A startup message of protocol `version` with `params`, framed as libpq and pgwire-replication frame it. +pub(crate) fn startup_message(version: [u8; 4], params: &[(&str, &str)]) -> Vec { + let mut body = Vec::new(); + for (name, value) in params { + body.extend_from_slice(format!("{name}\0{value}\0").as_bytes()); + } + body.push(0); + framed(version, &body) +} + +/// The replication client's startup message, naming `application_name`. +pub(crate) fn client_startup(application_name: &str) -> Vec { + startup_message( + PROTOCOL_VERSION, + &[ + ("user", "replicator"), + ("database", "shop"), + ("replication", "database"), + ("application_name", application_name), + ], + ) +} + +/// `message` with a length one less than it is. +pub(crate) fn declaring_one_less(mut message: Vec) -> Vec { + message[3] -= 1; + message +} + +/// `message` without its final NUL, its length one less to match. +pub(crate) fn without_final_nul(mut message: Vec) -> Vec { + message.pop(); + declaring_one_less(message) +} + +/// A whole startup message's parameters as `name=value` joined by `,`; `malformed` when it is not one. +pub(crate) fn startup_params(message: &[u8]) -> String { + let declared = message + .first_chunk::<4>() + .and_then(|length| usize::try_from(u32::from_be_bytes(*length)).ok()); + let well_formed = + declared == Some(message.len()) && message.get(4..8) == Some(&PROTOCOL_VERSION[..]); + message + .get(8..) + .and_then(|rest| rest.strip_suffix(b"\0\0")) + .filter(|_| well_formed) + .map(|rest| { + String::from_utf8_lossy(rest) + .split('\0') + .collect::>() + .chunks(2) + .map(|pair| pair.join("=")) + .collect::>() + .join(",") + }) + .unwrap_or_else(|| "malformed".to_owned()) +} + +pub(crate) fn contains(haystack: &[u8], needle: &str) -> bool { + haystack + .windows(needle.len()) + .any(|window| window == needle.as_bytes()) +} + +/// The whole first message the server side of a relay received. +async fn read_forwarded(upstream: &mut DuplexStream) -> Result, String> { + let mut length = [0u8; 4]; + upstream + .read_exact(&mut length) + .await + .map_err(|error| error.to_string())?; + let declared = + usize::try_from(u32::from_be_bytes(length)).map_err(|error| error.to_string())?; + let mut message = length.to_vec(); + message.resize(declared.max(4), 0); + upstream + .read_exact(&mut message[4..]) + .await + .map_err(|error| error.to_string())?; + Ok(message) +} + +/// Whether the relay closes `stream` within two seconds without sending it a byte. +async fn closed_unanswered(stream: &mut TcpStream) -> bool { + let mut byte = [0u8; 1]; + tokio::time::timeout(Duration::from_secs(2), stream.read(&mut byte)) + .await + .is_ok_and(|read| read.unwrap_or_default() == 0) +} + +async fn io(step: impl std::future::Future>) -> Result { + step.await.map_err(|error| error.to_string()) +} + +/// A loopback relay in front of an in-memory server: open a connection for each of `probes`, then send each its bytes at once (so refusals finish together), then open the replication client with the relay's application name, which must be relayed well before a silent connection's startup deadline; then pipe a byte each way, and connect once more. +pub(crate) async fn relay_scenario(probes: &[Vec]) -> Result { + let (server, mut upstream) = tokio::io::duplex(64 * 1024); + let endpoint = loopback_relay(server).await?; + let address = (endpoint.host.clone(), endpoint.port); + let mut opened = Vec::new(); + for _ in probes { + opened.push(io(TcpStream::connect(&address)).await?); + } + tokio::time::sleep(Duration::from_millis(100)).await; + for (stream, bytes) in opened.iter_mut().zip(probes) { + io(stream.write_all(bytes)).await?; + } + tokio::time::sleep(Duration::from_millis(300)).await; + let mut byte = [0u8; 1]; + let upstream_bytes_before_client = + tokio::time::timeout(Duration::from_millis(200), upstream.read(&mut byte)) + .await + .map_or(0, |read| read.unwrap_or_default()); + let mut client = io(TcpStream::connect(&address)).await?; + io(client.write_all(&client_startup(endpoint.application_name.expose()))).await?; + let forwarded = tokio::time::timeout(RELAY_STARTUP_TIMEOUT / 2, read_forwarded(&mut upstream)) + .await + .map_err(|_| "the client was not relayed in time".to_owned())??; + let secret = &endpoint.application_name.expose()["sqlload-".len()..]; + io(upstream.write_all(b"R")).await?; + let mut answer = [0u8; 1]; + io(client.read_exact(&mut answer)).await?; + io(client.write_all(b"p")).await?; + let mut echoed = [0u8; 1]; + io(upstream.read_exact(&mut echoed)).await?; + let mut probes_closed_unanswered = 0; + for stream in &mut opened { + probes_closed_unanswered += usize::from(closed_unanswered(stream).await); + } + let later_connection_refused = + tokio::time::timeout(Duration::from_secs(10), TcpStream::connect(&address)) + .await + .map_err(|_| "connecting after the client hung".to_owned())? + .is_err(); + Ok(RelayOutcome { + upstream_bytes_before_client, + forwarded: startup_params(&forwarded), + secret_forwarded: contains(&forwarded, secret), + piped: (answer, echoed) == (*b"R", *b"p"), + probes_closed_unanswered, + later_connection_refused, + }) +} + +/// What every scenario's relay does once its client connects, having refused `probes` connections. +pub(crate) fn relayed(probes: usize) -> RelayOutcome { + RelayOutcome { + upstream_bytes_before_client: 0, + forwarded: FORWARDED.to_owned(), + secret_forwarded: false, + piped: true, + probes_closed_unanswered: probes, + later_connection_refused: true, + } +} diff --git a/crates/sqlload/src/session_relay/tests/test_admitted_startup.rs b/crates/sqlload/src/session_relay/tests/test_admitted_startup.rs new file mode 100644 index 00000000..9adf6a9b --- /dev/null +++ b/crates/sqlload/src/session_relay/tests/test_admitted_startup.rs @@ -0,0 +1,111 @@ +use crate::session_relay::_helpers::startup::admitted_startup; +use crate::session_relay::constants::PROTOCOL_VERSION; +use crate::session_relay::tests::helpers::{ + FORWARDED, SECRET, SSL_REQUEST, client_startup, contains, declaring_one_less, framed, + startup_message, startup_params, without_final_nul, +}; +use crate::session_relay::tests::test_types::AdmittedStartupTestCase; + +const REFUSED_NAME: &str = "not this relay's application name"; +const NOT_ONE_NAME: &str = "not one application name"; +const NOT_A_STARTUP: &str = "not one protocol 3.0 startup message"; + +#[test] +fn given_startup_messages_when_the_relay_checks_them_then_only_its_secret_is_admitted_and_never_forwarded() + { + let wrong_last = format!("{}0", &SECRET[..SECRET.len() - 1]); + let longer = format!("{SECRET}0"); + let test_cases = [ + AdmittedStartupTestCase { + description: "the secret, among the other parameters", + message: client_startup(SECRET), + expected_forwarded: Ok(FORWARDED), + }, + AdmittedStartupTestCase { + description: "the secret first, before an empty value", + message: startup_message( + PROTOCOL_VERSION, + &[("application_name", SECRET), ("options", ""), ("user", "u")], + ), + expected_forwarded: Ok("application_name=sqlload,options=,user=u"), + }, + AdmittedStartupTestCase { + description: "a secret wrong in its last character", + message: client_startup(&wrong_last), + expected_forwarded: Err(REFUSED_NAME), + }, + AdmittedStartupTestCase { + description: "a prefix of the secret", + message: client_startup(&SECRET[..SECRET.len() - 1]), + expected_forwarded: Err(REFUSED_NAME), + }, + AdmittedStartupTestCase { + description: "the secret with more after it", + message: client_startup(&longer), + expected_forwarded: Err(REFUSED_NAME), + }, + AdmittedStartupTestCase { + description: "the plain application name", + message: client_startup("sqlload"), + expected_forwarded: Err(REFUSED_NAME), + }, + AdmittedStartupTestCase { + description: "no application name", + message: startup_message(PROTOCOL_VERSION, &[("user", "u"), ("database", "d")]), + expected_forwarded: Err(NOT_ONE_NAME), + }, + AdmittedStartupTestCase { + description: "the secret, then a second application name", + message: startup_message( + PROTOCOL_VERSION, + &[("application_name", SECRET), ("application_name", "other")], + ), + expected_forwarded: Err(NOT_ONE_NAME), + }, + AdmittedStartupTestCase { + description: "an SSLRequest", + message: SSL_REQUEST.to_vec(), + expected_forwarded: Err(NOT_A_STARTUP), + }, + AdmittedStartupTestCase { + description: "protocol 2.0 with the secret", + message: startup_message([0, 2, 0, 0], &[("application_name", SECRET)]), + expected_forwarded: Err(NOT_A_STARTUP), + }, + AdmittedStartupTestCase { + description: "a length one short of the message", + message: declaring_one_less(client_startup(SECRET)), + expected_forwarded: Err(NOT_A_STARTUP), + }, + AdmittedStartupTestCase { + description: "no final NUL", + message: without_final_nul(client_startup(SECRET)), + expected_forwarded: Err("not NUL-terminated"), + }, + AdmittedStartupTestCase { + description: "a name without a value", + message: framed(PROTOCOL_VERSION, b"application_name\0\0"), + expected_forwarded: Err("not pairs of names and values"), + }, + AdmittedStartupTestCase { + description: "nothing", + message: Vec::new(), + expected_forwarded: Err("no length"), + }, + ]; + for test_case in test_cases { + let forwarded = admitted_startup(&test_case.message, SECRET); + let found = forwarded.clone().map(|bytes| startup_params(&bytes)); + assert_eq!( + found, + test_case.expected_forwarded.map(str::to_owned), + "{}", + test_case.description + ); + assert!( + !forwarded.is_ok_and(|bytes| contains(&bytes, &SECRET["sqlload-".len()..])), + "{}: the secret was forwarded", + test_case.description + ); + } +} diff --git a/crates/sqlload/src/session_relay/tests/test_loopback.rs b/crates/sqlload/src/session_relay/tests/test_loopback.rs new file mode 100644 index 00000000..532ff27c --- /dev/null +++ b/crates/sqlload/src/session_relay/tests/test_loopback.rs @@ -0,0 +1,54 @@ +use crate::session_relay::constants::PROTOCOL_VERSION; +use crate::session_relay::tests::helpers::{ + SSL_REQUEST, block_on, client_startup, relay_scenario, relayed, startup_message, +}; +use crate::session_relay::tests::test_types::LoopbackRelayTestCase; + +#[test] +fn given_other_local_connections_when_the_client_connects_then_only_it_is_relayed_and_promptly() +-> Result<(), String> { + let wrong = format!("sqlload-{}", "0".repeat(64)); + let test_cases = [ + LoopbackRelayTestCase { + description: "no other connection", + probes: Vec::new(), + expected_outcome: relayed(0), + }, + LoopbackRelayTestCase { + description: "a silent connection first, which must not hold the client up", + probes: vec![Vec::new()], + expected_outcome: relayed(1), + }, + LoopbackRelayTestCase { + description: "two refused connections first, after which the relay must still admit the client", + probes: vec![vec![0; 8], vec![0; 8]], + expected_outcome: relayed(2), + }, + LoopbackRelayTestCase { + description: "a wrong secret, the plain name, an oversize length, an SSLRequest and protocol 2.0 first", + probes: vec![ + client_startup(&wrong), + client_startup("sqlload"), + vec![0, 1, 0, 0, 0, 3, 0, 0], + SSL_REQUEST.to_vec(), + startup_message([0, 2, 0, 0], &[("user", "u")]), + ], + expected_outcome: relayed(5), + }, + LoopbackRelayTestCase { + description: "a startup message with no application name first", + probes: vec![startup_message(PROTOCOL_VERSION, &[("user", "u")])], + expected_outcome: relayed(1), + }, + ]; + for test_case in test_cases { + let found = block_on(relay_scenario(&test_case.probes)) + .map_err(|error| format!("{}: {error}", test_case.description))?; + assert_eq!( + found, test_case.expected_outcome, + "{}", + test_case.description + ); + } + Ok(()) +} diff --git a/crates/sqlload/src/session_relay/tests/test_types.rs b/crates/sqlload/src/session_relay/tests/test_types.rs new file mode 100644 index 00000000..e7544a2f --- /dev/null +++ b/crates/sqlload/src/session_relay/tests/test_types.rs @@ -0,0 +1,29 @@ +pub(crate) struct AdmittedStartupTestCase { + pub(crate) description: &'static str, + pub(crate) message: Vec, + /// The parameters forwarded to the server, as `name=value` joined by `,`; why the message is refused otherwise. + pub(crate) expected_forwarded: Result<&'static str, &'static str>, +} + +pub(crate) struct LoopbackRelayTestCase { + pub(crate) description: &'static str, + /// What each connection opened before the client sends; empty for one that sends nothing. + pub(crate) probes: Vec>, + pub(crate) expected_outcome: RelayOutcome, +} + +/// What a loopback relay did with the connections opened before its client, and with the client. +#[derive(Debug, PartialEq, Eq)] +pub(crate) struct RelayOutcome { + /// Bytes the server received before the client connected. + pub(crate) upstream_bytes_before_client: usize, + /// The startup message the server received, as `name=value` joined by `,`. + pub(crate) forwarded: String, + pub(crate) secret_forwarded: bool, + /// Whether bytes then crossed between the client and the server both ways. + pub(crate) piped: bool, + /// How many of the earlier connections the relay closed without sending them a byte. + pub(crate) probes_closed_unanswered: usize, + /// Whether a connection after the client's is refused. + pub(crate) later_connection_refused: bool, +} diff --git a/crates/sqlload/src/store_paths/_helpers/local_root.rs b/crates/sqlload/src/store_paths/_helpers/local_root.rs index 02fc4e96..520a141e 100644 --- a/crates/sqlload/src/store_paths/_helpers/local_root.rs +++ b/crates/sqlload/src/store_paths/_helpers/local_root.rs @@ -29,14 +29,11 @@ pub(crate) fn local_root(path: &str, windows: bool) -> Option<(String, &str)> { }); } let drive = path.strip_prefix('/').unwrap_or(path); - let bytes = drive.as_bytes(); - if bytes.len() >= 3 - && bytes[0].is_ascii_alphabetic() - && bytes[1] == b':' - && bytes[2] == b'/' + if let [letter, b':', b'/', ..] = drive.as_bytes() + && letter.is_ascii_alphabetic() { return Some(( - format!("{}:/", char::from(bytes[0]).to_ascii_uppercase()), + format!("{}:/", char::from(letter.to_ascii_uppercase())), &drive[3..], )); } @@ -44,12 +41,32 @@ pub(crate) fn local_root(path: &str, windows: bool) -> Option<(String, &str)> { path.strip_prefix('/').map(|rest| ("/".to_owned(), rest)) } -/// The local path a `file://` URL names, given what follows `file://`: on Windows `file:///C:/folder` is `C:/folder` and `file://server/share/folder` is the share `//server/share/folder`. +/// The local path a `file://` URL names, given what follows `file://`: on Windows `file:///C:/folder` and `file://localhost/C:/folder` are `C:/folder`, and `file://server/share/folder` is the share `//server/share/folder`. pub(crate) fn windows_file_url_path(rest: &str, windows: bool) -> Cow<'_, str> { - match (windows, rest.strip_prefix('/')) { - (false, _) => Cow::Borrowed(rest), - (true, Some(path)) if path.as_bytes().get(1) == Some(&b':') => Cow::Borrowed(path), - (true, Some(_)) => Cow::Borrowed(rest), - (true, None) => Cow::Owned(format!("//{rest}")), + let on_localhost = rest + .get(.."localhost/".len()) + .filter(|host| host.eq_ignore_ascii_case("localhost/")) + .map(|host| &rest[host.len()..]); + match (windows, rest.strip_prefix('/'), on_localhost) { + (false, _, _) => Cow::Borrowed(rest), + (true, Some(path), _) if is_drive(path) => Cow::Borrowed(path), + (true, Some(_), _) => Cow::Borrowed(rest), + (true, None, Some(path)) if is_drive(path) => Cow::Borrowed(path), + (true, None, _) => Cow::Owned(format!("//{rest}")), } } + +/// Whether `path` starts with a drive letter and its colon. +fn is_drive(path: &str) -> bool { + matches!(path.as_bytes(), [letter, b':', ..] if letter.is_ascii_alphabetic()) +} + +/// `prefix` without its leading `folders` (both folder prefixes ending in `/`), whose names match ignoring case when `windows`, as NTFS and SMB shares match them; `None` when `prefix` is not below `folders`. +pub(crate) fn strip_folders<'a>(prefix: &'a str, folders: &str, windows: bool) -> Option<&'a str> { + let head = prefix.get(..folders.len())?; + let same = match windows { + true => head.eq_ignore_ascii_case(folders), + false => head == folders, + }; + same.then(|| &prefix[folders.len()..]) +} diff --git a/crates/sqlload/src/store_paths/main/resolve_location.rs b/crates/sqlload/src/store_paths/main/resolve_location.rs index e7e4ccdb..506bc42c 100644 --- a/crates/sqlload/src/store_paths/main/resolve_location.rs +++ b/crates/sqlload/src/store_paths/main/resolve_location.rs @@ -1,12 +1,12 @@ //! Work out which store and folder a path means. use crate::store_paths::_helpers::folder::folder; -use crate::store_paths::_helpers::local_root::{local_root, windows_separators}; +use crate::store_paths::_helpers::local_root::{local_root, strip_folders, windows_separators}; use crate::store_paths::_helpers::locations::{describe, same_store, url_location}; use crate::store_paths::errors::PathError; use crate::store_paths::models::{Location, StoreKind}; -/// The folder `path` names: relative to `connection` when there is one (on an SFTP server, from `/` when it starts with `/`, otherwise from the login folder, which a leading `~/` also names), otherwise from its scheme. +/// The folder `path` names: relative to `connection` when there is one (on an SFTP server, from `/` when it starts with `/`, otherwise from the login folder, which a leading `~/` also names), otherwise from its scheme. Under a filesystem connection, an absolute path must be below its root, matched on the root too unless the connection's root is relative; on Windows roots and folders match ignoring case. pub fn resolve_location( connection: Option<(&str, &StoreKind)>, path: &str, @@ -43,7 +43,6 @@ pub fn resolve_location( (Some((name, StoreKind::Filesystem { root })), Some(location)) => { let windows = cfg!(windows); let separated = windows_separators(root, windows); - // A relative root is matched on its folders alone, whatever the path's root. let (same_root, root_folders) = match (local_root(&separated, windows), &location.store) { (Some((root, folders)), StoreKind::Filesystem { root: found }) => { @@ -52,11 +51,7 @@ pub fn resolve_location( _ => (true, separated.as_ref()), }; let root_prefix = folder(&[root_folders])?; - match location - .prefix - .strip_prefix(&root_prefix) - .filter(|_| same_root) - { + match strip_folders(&location.prefix, &root_prefix, windows).filter(|_| same_root) { Some(relative) => Ok(Location { store: StoreKind::Filesystem { root: root.clone() }, prefix: relative.to_owned(), diff --git a/crates/sqlload/src/store_paths/tests/helpers.rs b/crates/sqlload/src/store_paths/tests/helpers.rs index f1f48844..7b94a39a 100644 --- a/crates/sqlload/src/store_paths/tests/helpers.rs +++ b/crates/sqlload/src/store_paths/tests/helpers.rs @@ -9,3 +9,10 @@ pub(crate) fn bucket(name: &str) -> Option { profile: None, }) } + +/// The root and folders of `path` as `local_root` finds them, written `root|folders`. +pub(crate) fn root_of(path: &str, windows: bool) -> Option { + let separated = crate::store_paths::_helpers::local_root::windows_separators(path, windows); + crate::store_paths::_helpers::local_root::local_root(&separated, windows) + .map(|(root, folders)| format!("{root}|{folders}")) +} diff --git a/crates/sqlload/src/store_paths/tests/test_local_root.rs b/crates/sqlload/src/store_paths/tests/test_local_root.rs index 5615e0c4..91d99a67 100644 --- a/crates/sqlload/src/store_paths/tests/test_local_root.rs +++ b/crates/sqlload/src/store_paths/tests/test_local_root.rs @@ -1,59 +1,52 @@ -use crate::store_paths::_helpers::local_root::{ - local_root, windows_file_url_path, windows_separators, +use crate::store_paths::_helpers::local_root::{strip_folders, windows_file_url_path}; +use crate::store_paths::tests::helpers::root_of; +use crate::store_paths::tests::test_types::{ + FileUrlPathTestCase, LocalRootTestCase, StripFoldersTestCase, }; -/// How a local path reads on Windows, and on Unix, where only `/` starts an absolute path. -struct LocalRootTestCase { - description: &'static str, - path: &'static str, - /// Root and folders on Windows; `None` for a relative path. - expected_windows: Option<(&'static str, &'static str)>, - expected_unix: Option<(&'static str, &'static str)>, -} - #[test] fn given_local_paths_when_finding_their_root_then_drives_and_shares_are_roots_only_on_windows() { let test_cases = [ LocalRootTestCase { description: "a Unix absolute path", path: "/data/exports", - expected_windows: Some(("/", "data/exports")), - expected_unix: Some(("/", "data/exports")), + expected_windows: Some("/|data/exports"), + expected_unix: Some("/|data/exports"), }, LocalRootTestCase { description: "a drive with backslashes, lower-case letter", path: r"c:\Data\exports", - expected_windows: Some(("C:/", "Data/exports")), + expected_windows: Some("C:/|Data/exports"), expected_unix: None, }, LocalRootTestCase { description: "a drive as a file:/// URL gives it", path: "/D:/lake", - expected_windows: Some(("D:/", "lake")), - expected_unix: Some(("/", "D:/lake")), + expected_windows: Some("D:/|lake"), + expected_unix: Some("/|D:/lake"), }, LocalRootTestCase { description: "a UNC share", path: r"\\nas01\exports\daily\orders", - expected_windows: Some(("//nas01/exports/", "daily/orders")), + expected_windows: Some("//nas01/exports/|daily/orders"), expected_unix: None, }, LocalRootTestCase { description: "a UNC share's root alone", path: r"\\nas01\exports", - expected_windows: Some(("//nas01/exports/", "")), + expected_windows: Some("//nas01/exports/|"), expected_unix: None, }, LocalRootTestCase { description: "an extended-length drive path", path: r"\\?\C:\very\long", - expected_windows: Some(("C:/", "very/long")), + expected_windows: Some("C:/|very/long"), expected_unix: None, }, LocalRootTestCase { description: "an extended-length UNC path", path: r"\\?\UNC\nas01\exports\x", - expected_windows: Some(("//nas01/exports/", "x")), + expected_windows: Some("//nas01/exports/|x"), expected_unix: None, }, LocalRootTestCase { @@ -76,35 +69,115 @@ fn given_local_paths_when_finding_their_root_then_drives_and_shares_are_roots_on }, ]; for test_case in test_cases { - for (windows, expected) in [ - (true, test_case.expected_windows), - (false, test_case.expected_unix), - ] { - let separated = windows_separators(test_case.path, windows); - let found = local_root(&separated, windows); - let found = found.as_ref().map(|(root, rest)| (root.as_str(), *rest)); - assert_eq!( - found, expected, - "{} (windows: {windows})", - test_case.description - ); - } + assert_eq!( + root_of(test_case.path, true).as_deref(), + test_case.expected_windows, + "{} on Windows", + test_case.description + ); + assert_eq!( + root_of(test_case.path, false).as_deref(), + test_case.expected_unix, + "{} on Unix", + test_case.description + ); } } #[test] fn given_file_urls_when_reading_their_path_then_windows_drives_and_shares_are_found() { let test_cases = [ - ("/data/x", "/data/x", "/data/x"), - ("/C:/data/x", "C:/data/x", "/C:/data/x"), - ("nas01/exports/x", "//nas01/exports/x", "nas01/exports/x"), + FileUrlPathTestCase { + description: "an absolute path", + rest: "/data/x", + expected_windows: "/data/x", + expected_unix: "/data/x", + }, + FileUrlPathTestCase { + description: "a drive", + rest: "/C:/data/x", + expected_windows: "C:/data/x", + expected_unix: "/C:/data/x", + }, + FileUrlPathTestCase { + description: "a drive on localhost", + rest: "LocalHost/C:/data/x", + expected_windows: "C:/data/x", + expected_unix: "LocalHost/C:/data/x", + }, + FileUrlPathTestCase { + description: "a share on localhost", + rest: "localhost/exports/x", + expected_windows: "//localhost/exports/x", + expected_unix: "localhost/exports/x", + }, + FileUrlPathTestCase { + description: "a share", + rest: "nas01/exports/x", + expected_windows: "//nas01/exports/x", + expected_unix: "nas01/exports/x", + }, + ]; + for test_case in test_cases { + assert_eq!( + windows_file_url_path(test_case.rest, true), + test_case.expected_windows, + "{} on Windows", + test_case.description + ); + assert_eq!( + windows_file_url_path(test_case.rest, false), + test_case.expected_unix, + "{} on Unix", + test_case.description + ); + } +} + +#[test] +fn given_a_folder_prefix_when_stripping_a_connections_folders_then_windows_ignores_case() { + let test_cases = [ + StripFoldersTestCase { + description: "the same folders", + prefix: "lake/scores/", + folders: "lake/", + expected_windows: Some("scores/"), + expected_unix: Some("scores/"), + }, + StripFoldersTestCase { + description: "folders differing in case", + prefix: "Lake/scores/", + folders: "lake/", + expected_windows: Some("scores/"), + expected_unix: None, + }, + StripFoldersTestCase { + description: "a folder that only starts with the connection's", + prefix: "lakehouse/scores/", + folders: "lake/", + expected_windows: None, + expected_unix: None, + }, + StripFoldersTestCase { + description: "a shorter prefix", + prefix: "la/", + folders: "lake/", + expected_windows: None, + expected_unix: None, + }, ]; - for (rest, expected_windows, expected_unix) in test_cases { + for test_case in test_cases { + assert_eq!( + strip_folders(test_case.prefix, test_case.folders, true), + test_case.expected_windows, + "{} on Windows", + test_case.description + ); assert_eq!( - windows_file_url_path(rest, true), - expected_windows, - "{rest}" + strip_folders(test_case.prefix, test_case.folders, false), + test_case.expected_unix, + "{} on Unix", + test_case.description ); - assert_eq!(windows_file_url_path(rest, false), expected_unix, "{rest}"); } } diff --git a/crates/sqlload/src/store_paths/tests/test_resolve_location.rs b/crates/sqlload/src/store_paths/tests/test_resolve_location.rs index 94249d6e..cfb219bf 100644 --- a/crates/sqlload/src/store_paths/tests/test_resolve_location.rs +++ b/crates/sqlload/src/store_paths/tests/test_resolve_location.rs @@ -133,6 +133,14 @@ fn given_windows_paths_when_resolving_then_drives_and_shares_are_local_folders() path: r"daily\orders", expected_location: Ok(r"\\nas01\exports/daily/orders/"), }, + ResolveLocationTestCase { + description: "a folder under a drive connection in another case", + connection: Some(StoreKind::Filesystem { + root: r"C:\Lake".to_owned(), + }), + path: r"c:\lake\scores", + expected_location: Ok(r"C:\Lake/scores/"), + }, ResolveLocationTestCase { description: "another share is not under the connection", connection: share(), @@ -159,14 +167,20 @@ fn given_windows_paths_when_resolving_then_drives_and_shares_are_local_folders() let result = resolve_location(connection, test_case.path) .map(|location| location.to_string()) .map_err(|error| error.to_string()); - match (&result, test_case.expected_location) { - (Ok(found), Ok(expected)) => assert_eq!(found, expected, "{}", test_case.description), - (Err(found), Err(expected)) => assert!( - found.contains(expected), - "{}: {found}", - test_case.description - ), - _ => panic!("{}: {result:?}", test_case.description), - } + assert_eq!( + result.as_deref().ok(), + test_case.expected_location.ok(), + "{}", + test_case.description + ); + assert!( + result + .err() + .unwrap_or_default() + .contains(test_case.expected_location.err().unwrap_or_default()), + "{}: {:?}", + test_case.description, + test_case.path + ); } } diff --git a/crates/sqlload/src/store_paths/tests/test_types.rs b/crates/sqlload/src/store_paths/tests/test_types.rs index c78ea9d4..5864cd2a 100644 --- a/crates/sqlload/src/store_paths/tests/test_types.rs +++ b/crates/sqlload/src/store_paths/tests/test_types.rs @@ -32,3 +32,29 @@ pub(crate) struct PartitionValueTestCase { pub(crate) value: &'static str, pub(crate) expected_escaped: &'static str, } + +/// How a local path reads on Windows, and on Unix, where only `/` starts an absolute path. +pub(crate) struct LocalRootTestCase { + pub(crate) description: &'static str, + pub(crate) path: &'static str, + /// The root and the folders below it, as `root|folders`; `None` for a relative path. + pub(crate) expected_windows: Option<&'static str>, + pub(crate) expected_unix: Option<&'static str>, +} + +/// The local path a `file://` URL names, given what follows `file://`, on Windows and on Unix. +pub(crate) struct FileUrlPathTestCase { + pub(crate) description: &'static str, + pub(crate) rest: &'static str, + pub(crate) expected_windows: &'static str, + pub(crate) expected_unix: &'static str, +} + +/// A folder prefix below a connection's folders, on Windows and on Unix. +pub(crate) struct StripFoldersTestCase { + pub(crate) description: &'static str, + pub(crate) prefix: &'static str, + pub(crate) folders: &'static str, + pub(crate) expected_windows: Option<&'static str>, + pub(crate) expected_unix: Option<&'static str>, +} From 7cd4c00789bd5b1c23c997061c8453a194e82c48 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 12 Nov 2026 12:25:52 +0000 Subject: [PATCH 03/11] fix: build the ssh-agent client and the test kit on Windows russh reaches ssh-agent through SSH_AUTH_SOCK's Unix socket only on Unix; on Windows it now opens the pipe SSH_AUTH_SOCK names, or else the OpenSSH agent service's. The test kit sets Unix key-file permissions only on Unix. --- .../src/ssh/_helpers/key_files.rs | 3 +++ .../sqlload/src/ssh_tunnel/_helpers/session.rs | 18 +++++++++++++++++- crates/sqlload/src/ssh_tunnel/constants.rs | 4 ++++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/crates/sqlload-testkit/src/ssh/_helpers/key_files.rs b/crates/sqlload-testkit/src/ssh/_helpers/key_files.rs index e6e0faf9..586041f5 100644 --- a/crates/sqlload-testkit/src/ssh/_helpers/key_files.rs +++ b/crates/sqlload-testkit/src/ssh/_helpers/key_files.rs @@ -47,8 +47,11 @@ pub(crate) async fn key_files( ] { let path = files.path().join(name); std::fs::write(&path, text).map_err(|error| error.to_string())?; + #[cfg(unix)] std::fs::set_permissions(&path, std::os::unix::fs::PermissionsExt::from_mode(mode)) .map_err(|error| error.to_string())?; + #[cfg(not(unix))] + let _ = mode; } Ok(files) } diff --git a/crates/sqlload/src/ssh_tunnel/_helpers/session.rs b/crates/sqlload/src/ssh_tunnel/_helpers/session.rs index 3b36a30a..35d01c2c 100644 --- a/crates/sqlload/src/ssh_tunnel/_helpers/session.rs +++ b/crates/sqlload/src/ssh_tunnel/_helpers/session.rs @@ -9,6 +9,8 @@ use russh::keys::agent::client::AgentClient; use russh::keys::known_hosts::known_host_keys_path; use russh::keys::{HashAlg, PrivateKeyWithHashAlg, PublicKeyOrCertificate, load_secret_key}; +#[cfg(windows)] +use crate::ssh_tunnel::constants::OPENSSH_AGENT_PIPE; use crate::ssh_tunnel::constants::{CONNECT_TIMEOUT, KEEPALIVE_INTERVAL, KEEPALIVE_MAX}; use crate::ssh_tunnel::models::{HostCheck, SshTunnel}; @@ -234,13 +236,27 @@ async fn password_sign_in( } } +/// ssh-agent: the socket `SSH_AUTH_SOCK` names. +#[cfg(unix)] +async fn connect_agent() -> Result, russh::keys::Error> { + AgentClient::connect_env().await +} + +/// ssh-agent on Windows: the pipe `SSH_AUTH_SOCK` names, or else the OpenSSH agent service's. +#[cfg(windows)] +async fn connect_agent() +-> Result, russh::keys::Error> { + let pipe = std::env::var("SSH_AUTH_SOCK").unwrap_or_else(|_| OPENSSH_AGENT_PIPE.to_owned()); + AgentClient::connect_named_pipe(pipe).await +} + /// `session` signed in as `user` with the first key ssh-agent holds that the bastion accepts. async fn agent_sign_in( mut session: Handle, user: &str, rsa_hash: Option, ) -> Result, String> { - let mut agent = AgentClient::connect_env().await.map_err(|error| { + let mut agent = connect_agent().await.map_err(|error| { format!("neither private_key_file nor password is set and ssh-agent is not reachable ({error}); set private_key_file or password") })?; let identities = agent diff --git a/crates/sqlload/src/ssh_tunnel/constants.rs b/crates/sqlload/src/ssh_tunnel/constants.rs index f9b59a46..a4535409 100644 --- a/crates/sqlload/src/ssh_tunnel/constants.rs +++ b/crates/sqlload/src/ssh_tunnel/constants.rs @@ -3,6 +3,10 @@ use std::time::Duration; /// The bastion's port when the `ssh` table sets none. pub const SSH_PORT: u16 = 22; +/// Where Windows's OpenSSH agent service listens when `SSH_AUTH_SOCK` names no other pipe. +#[cfg(windows)] +pub const OPENSSH_AGENT_PIPE: &str = r"\\.\pipe\openssh-ssh-agent"; + /// How long to wait for the bastion to answer and sign the client in. pub const CONNECT_TIMEOUT: Duration = Duration::from_secs(30); From d34ac80e0cc71365c9cee89d646cdf30dd8877aa Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 12 Nov 2026 12:30:59 +0000 Subject: [PATCH 04/11] test: which pipe ssh-agent is reached on on Windows, and say what is and isn't tested there --- .../src/ssh_tunnel/_helpers/session.rs | 15 ++++++--- crates/sqlload/src/ssh_tunnel/constants.rs | 2 +- crates/sqlload/src/ssh_tunnel/mod.rs | 3 ++ crates/sqlload/src/ssh_tunnel/tests.rs | 4 +++ .../src/ssh_tunnel/tests/test_agent_pipe.rs | 32 +++++++++++++++++++ .../src/ssh_tunnel/tests/test_types.rs | 6 ++++ .../content/docs/docs/sources/postgres.mdx | 5 +++ 7 files changed, 62 insertions(+), 5 deletions(-) create mode 100644 crates/sqlload/src/ssh_tunnel/tests.rs create mode 100644 crates/sqlload/src/ssh_tunnel/tests/test_agent_pipe.rs create mode 100644 crates/sqlload/src/ssh_tunnel/tests/test_types.rs diff --git a/crates/sqlload/src/ssh_tunnel/_helpers/session.rs b/crates/sqlload/src/ssh_tunnel/_helpers/session.rs index 35d01c2c..db230bde 100644 --- a/crates/sqlload/src/ssh_tunnel/_helpers/session.rs +++ b/crates/sqlload/src/ssh_tunnel/_helpers/session.rs @@ -9,7 +9,7 @@ use russh::keys::agent::client::AgentClient; use russh::keys::known_hosts::known_host_keys_path; use russh::keys::{HashAlg, PrivateKeyWithHashAlg, PublicKeyOrCertificate, load_secret_key}; -#[cfg(windows)] +#[cfg(any(windows, test))] use crate::ssh_tunnel::constants::OPENSSH_AGENT_PIPE; use crate::ssh_tunnel::constants::{CONNECT_TIMEOUT, KEEPALIVE_INTERVAL, KEEPALIVE_MAX}; use crate::ssh_tunnel::models::{HostCheck, SshTunnel}; @@ -242,12 +242,19 @@ async fn connect_agent() -> Result, russh::k AgentClient::connect_env().await } -/// ssh-agent on Windows: the pipe `SSH_AUTH_SOCK` names, or else the OpenSSH agent service's. +/// ssh-agent on Windows: see [`agent_pipe`]. #[cfg(windows)] async fn connect_agent() -> Result, russh::keys::Error> { - let pipe = std::env::var("SSH_AUTH_SOCK").unwrap_or_else(|_| OPENSSH_AGENT_PIPE.to_owned()); - AgentClient::connect_named_pipe(pipe).await + AgentClient::connect_named_pipe(agent_pipe(std::env::var_os("SSH_AUTH_SOCK"))).await +} + +/// The pipe ssh-agent listens on, on Windows: the one `SSH_AUTH_SOCK` names when it is set and not empty, or else the OpenSSH agent service's. +#[cfg(any(windows, test))] +pub(crate) fn agent_pipe(auth_sock: Option) -> std::ffi::OsString { + auth_sock + .filter(|pipe| !pipe.is_empty()) + .unwrap_or_else(|| OPENSSH_AGENT_PIPE.into()) } /// `session` signed in as `user` with the first key ssh-agent holds that the bastion accepts. diff --git a/crates/sqlload/src/ssh_tunnel/constants.rs b/crates/sqlload/src/ssh_tunnel/constants.rs index a4535409..70eaead4 100644 --- a/crates/sqlload/src/ssh_tunnel/constants.rs +++ b/crates/sqlload/src/ssh_tunnel/constants.rs @@ -4,7 +4,7 @@ use std::time::Duration; pub const SSH_PORT: u16 = 22; /// Where Windows's OpenSSH agent service listens when `SSH_AUTH_SOCK` names no other pipe. -#[cfg(windows)] +#[cfg(any(windows, test))] pub const OPENSSH_AGENT_PIPE: &str = r"\\.\pipe\openssh-ssh-agent"; /// How long to wait for the bastion to answer and sign the client in. diff --git a/crates/sqlload/src/ssh_tunnel/mod.rs b/crates/sqlload/src/ssh_tunnel/mod.rs index 39298fb9..f4952daf 100644 --- a/crates/sqlload/src/ssh_tunnel/mod.rs +++ b/crates/sqlload/src/ssh_tunnel/mod.rs @@ -5,3 +5,6 @@ pub mod constants; pub mod errors; pub mod main; pub mod models; + +#[cfg(test)] +mod tests; diff --git a/crates/sqlload/src/ssh_tunnel/tests.rs b/crates/sqlload/src/ssh_tunnel/tests.rs new file mode 100644 index 00000000..5b85776e --- /dev/null +++ b/crates/sqlload/src/ssh_tunnel/tests.rs @@ -0,0 +1,4 @@ +#[path = "tests/test_agent_pipe.rs"] +mod agent_pipe; +#[path = "tests/test_types.rs"] +mod test_types; diff --git a/crates/sqlload/src/ssh_tunnel/tests/test_agent_pipe.rs b/crates/sqlload/src/ssh_tunnel/tests/test_agent_pipe.rs new file mode 100644 index 00000000..74f67e37 --- /dev/null +++ b/crates/sqlload/src/ssh_tunnel/tests/test_agent_pipe.rs @@ -0,0 +1,32 @@ +use crate::ssh_tunnel::_helpers::session::agent_pipe; +use crate::ssh_tunnel::tests::test_types::AgentPipeTestCase; + +#[test] +fn given_ssh_auth_sock_when_finding_the_windows_agent_then_its_pipe_or_the_openssh_service_is_used() +{ + let test_cases = [ + AgentPipeTestCase { + description: "SSH_AUTH_SOCK unset", + auth_sock: None, + expected_pipe: r"\\.\pipe\openssh-ssh-agent", + }, + AgentPipeTestCase { + description: "SSH_AUTH_SOCK empty", + auth_sock: Some(""), + expected_pipe: r"\\.\pipe\openssh-ssh-agent", + }, + AgentPipeTestCase { + description: "SSH_AUTH_SOCK naming another agent's pipe", + auth_sock: Some(r"\\.\pipe\pageant.user.1a2b"), + expected_pipe: r"\\.\pipe\pageant.user.1a2b", + }, + ]; + for test_case in test_cases { + assert_eq!( + agent_pipe(test_case.auth_sock.map(std::ffi::OsString::from)), + test_case.expected_pipe, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlload/src/ssh_tunnel/tests/test_types.rs b/crates/sqlload/src/ssh_tunnel/tests/test_types.rs new file mode 100644 index 00000000..58c3c645 --- /dev/null +++ b/crates/sqlload/src/ssh_tunnel/tests/test_types.rs @@ -0,0 +1,6 @@ +pub(crate) struct AgentPipeTestCase { + pub(crate) description: &'static str, + /// `SSH_AUTH_SOCK`, unset when `None`. + pub(crate) auth_sock: Option<&'static str>, + pub(crate) expected_pipe: &'static str, +} diff --git a/website/src/content/docs/docs/sources/postgres.mdx b/website/src/content/docs/docs/sources/postgres.mdx index 4a79727f..ed34af90 100644 --- a/website/src/content/docs/docs/sources/postgres.mdx +++ b/website/src/content/docs/docs/sources/postgres.mdx @@ -194,6 +194,11 @@ and `port` are then the database as the bastion sees it. To set one up, see - It signs in with `private_key_file` (with `private_key_passphrase` if the key has one), or with each key ssh-agent holds when no file is set. From 0.32.0, `password` signs in when no key file is set; before, passwords aren't supported. +- On Windows, ssh-agent is the pipe `SSH_AUTH_SOCK` names, or else the OpenSSH agent service's + (`\\.\pipe\openssh-ssh-agent`). Windows ssh-agent (OpenSSH agent pipe): compiles and + unit-tested, not live-tested. Key-file and password sign-in run the same code as on Linux and + macOS, but on Windows they're not covered by an end-to-end test: no Windows test goes through a + bastion. - The bastion's host key must be in `known_hosts` (`~/.ssh/known_hosts` by default). An unknown or changed key fails with its SHA256 fingerprint and the fix; a key is never accepted on first use. - TLS still runs end to end with the database, so `verify-full` checks the database's own name. From 68e25e222e747c3c6a723328f2f7f6c07ffb38b5 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 12 Nov 2026 12:57:18 +0000 Subject: [PATCH 05/11] ci(publish): a new push to a pull request cancels its older wheel builds; releases never cancel --- .github/workflows/publish.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index eedc1248..4503a2ef 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -19,6 +19,12 @@ on: - crates/sqlload/Cargo.toml - crates/sqlload-python/Cargo.toml +# A new push to a pull request cancels its older wheel builds; a release run is a group of its own, +# so releases never cancel each other. +concurrency: + group: publish-${{ github.event_name == 'pull_request' && github.ref || github.run_id }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + permissions: contents: read From 9ab408f4d5412c9980575174b9d1e1469f261953 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 12 Nov 2026 13:24:57 +0000 Subject: [PATCH 06/11] test: write Windows temp paths as TOML literal strings, and allow the folder-swap gap Windows has A Windows temp path in a TOML basic string reads \U as a unicode escape, so the command tests now quote the root with single quotes. Windows can't swap two folders in one rename, so a reader may briefly see no folder there, as the files target docs now say. --- crates/sqlload/src/adapters/files/tests/helpers.rs | 5 +++++ crates/sqlload/src/adapters/files/tests/test_swap_folder.rs | 3 ++- crates/sqlload/src/command/tests/test_loader_commands.rs | 6 +++--- website/src/content/docs/docs/targets/files.mdx | 2 +- 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/crates/sqlload/src/adapters/files/tests/helpers.rs b/crates/sqlload/src/adapters/files/tests/helpers.rs index 6541236a..63894966 100644 --- a/crates/sqlload/src/adapters/files/tests/helpers.rs +++ b/crates/sqlload/src/adapters/files/tests/helpers.rs @@ -36,6 +36,11 @@ use crate::store_paths::models::{Location, StoreKind}; const NOV_4_PLUS_1_US: i64 = 1_793_750_400_000_001; /// What `watch_folder` records when the folder is missing. pub(crate) const NO_FOLDER: &str = ""; +/// What a reader may also see while a folder is swapped in: nothing on Windows, which cannot swap two folders in one rename, so the old one is moved aside first. +pub(crate) const MID_SWAP: &[&str] = match cfg!(windows) { + true => &[NO_FOLDER], + false => &[], +}; /// How the data files a load writes are named. const PART_NAME_START: &str = "part-"; diff --git a/crates/sqlload/src/adapters/files/tests/test_swap_folder.rs b/crates/sqlload/src/adapters/files/tests/test_swap_folder.rs index f7bef24a..ef0deaab 100644 --- a/crates/sqlload/src/adapters/files/tests/test_swap_folder.rs +++ b/crates/sqlload/src/adapters/files/tests/test_swap_folder.rs @@ -7,7 +7,7 @@ use crate::adapters::files::main::repair_table::repair_table; use crate::adapters::files::main::swap_folder::swap_folder; use crate::adapters::files::models::TablePath; use crate::adapters::files::tests::helpers::{ - NO_FOLDER, block_on, local_files, watch_folder, write_all, + MID_SWAP, NO_FOLDER, block_on, local_files, watch_folder, write_all, }; use crate::adapters::files::tests::test_types::{RemoveStagedTestCase, SwapFolderTestCase}; use crate::file_store::main::open_folder::open_folder; @@ -99,6 +99,7 @@ fn given_a_staged_table_when_swapping_it_in_then_readers_see_the_old_or_new_file .iter() .copied() .chain(test_case.live.is_empty().then_some(NO_FOLDER)) + .chain(MID_SWAP.iter().copied()) .map(str::to_owned) .collect(); let new: Vec = vec!["part-new-0.csv".to_owned()]; diff --git a/crates/sqlload/src/command/tests/test_loader_commands.rs b/crates/sqlload/src/command/tests/test_loader_commands.rs index 3efb4110..2b0bc858 100644 --- a/crates/sqlload/src/command/tests/test_loader_commands.rs +++ b/crates/sqlload/src/command/tests/test_loader_commands.rs @@ -5,7 +5,7 @@ use crate::command::tests::test_types::{ const TARGET: &str = "[connections.wh]\nkind = \"duckdb\"\ndatabase = \":memory:\"\n[targets.dev]\nconnection = \"wh\"\nloader_schema = \"RAW\"\n"; const NO_LOAD_METHOD: &str = "[connections.wh]\nkind = \"snowflake\"\naccount = \"a\"\nuser = \"u\"\nauth = { method = \"pat\", token = \"t\" }\ndatabase = \"D\"\n[targets.dev]\nconnection = \"wh\"\nloader_schema = \"RAW\"\n"; -const REFUSING_TARGET: &str = "[connections.pg]\nkind = \"postgres\"\nhost = \"127.0.0.1\"\nport = 1\nuser = \"loader\"\npassword = \"p\"\ndatabase = \"app\"\nsslmode = \"disable\"\n[targets.dev]\nconnection = \"pg\"\nloader_schema = \"raw\"\n[sources.orders]\nkind = \"files\"\ndescription = \"Test table.\"\nconnection = \"lake\"\npath = \"orders/\"\nformat = \"csv\"\nwrite_strategy = \"append\"\n[connections.lake]\nkind = \"filesystem\"\nroot = \"{root}\"\n"; +const REFUSING_TARGET: &str = "[connections.pg]\nkind = \"postgres\"\nhost = \"127.0.0.1\"\nport = 1\nuser = \"loader\"\npassword = \"p\"\ndatabase = \"app\"\nsslmode = \"disable\"\n[targets.dev]\nconnection = \"pg\"\nloader_schema = \"raw\"\n[sources.orders]\nkind = \"files\"\ndescription = \"Test table.\"\nconnection = \"lake\"\npath = \"orders/\"\nformat = \"csv\"\nwrite_strategy = \"append\"\n[connections.lake]\nkind = \"filesystem\"\nroot = '{root}'\n"; const UNREACHABLE_SOURCE: &str = "[connections.wh]\nkind = \"duckdb\"\ndatabase = \":memory:\"\n[targets.dev]\nconnection = \"wh\"\nloader_schema = \"RAW\"\n[connections.app]\nkind = \"postgres\"\nhost = \"127.0.0.1\"\nport = 1\nuser = \"u\"\npassword = \"p\"\ndatabase = \"app\"\nsslmode = \"disable\"\n[sources.app]\nkind = \"postgres\"\nconnection = \"app\"\ndestination = \"dev\"\ninclude = [\"public.*\"]\n"; #[test] @@ -116,7 +116,7 @@ fn given_a_files_source_into_a_files_target_when_planning_then_an_overlapping_fo ]; for test_case in test_cases { let project = format!( - "[connections.lake]\nkind = \"filesystem\"\nroot = \"{}\"\n[targets.lake]\nconnection = \"lake\"\npath = \"{}\"\nsingle_file = {}\n[sources.orders]\nkind = \"files\"\ndescription = \"Test table.\"\nconnection = \"lake\"\npath = \"data/orders/\"\nformat = \"csv\"\nwrite_strategy = \"replace\"\npartition_columns = {{ day = \"date\" }}\n", + "[connections.lake]\nkind = \"filesystem\"\nroot = '{}'\n[targets.lake]\nconnection = \"lake\"\npath = \"{}\"\nsingle_file = {}\n[sources.orders]\nkind = \"files\"\ndescription = \"Test table.\"\nconnection = \"lake\"\npath = \"data/orders/\"\nformat = \"csv\"\nwrite_strategy = \"replace\"\npartition_columns = {{ day = \"date\" }}\n", root.path().display(), test_case.target_path, test_case.single_file @@ -170,7 +170,7 @@ fn given_a_failure_a_rerun_may_pass_when_loading_then_it_exits_75_and_says_so_el }, ExitCodeTestCase { description: "--json: an unreachable source beside a plan error that is not retryable exits 1", - project: "[connections.wh]\nkind = \"duckdb\"\ndatabase = \":memory:\"\n[targets.dev]\nconnection = \"wh\"\nloader_schema = \"RAW\"\n[connections.app]\nkind = \"postgres\"\nhost = \"127.0.0.1\"\nport = 1\nuser = \"u\"\npassword = \"p\"\ndatabase = \"app\"\nsslmode = \"disable\"\n[sources.app]\nkind = \"postgres\"\nconnection = \"app\"\ndestination = \"dev\"\ninclude = [\"public.*\"]\n[sources.orders]\nkind = \"files\"\ndescription = \"Test table.\"\nconnection = \"lake\"\npath = \"missing/\"\nformat = \"csv\"\nwrite_strategy = \"append\"\ndestination = \"dev\"\n[connections.lake]\nkind = \"filesystem\"\nroot = \"{root}\"\n", + project: "[connections.wh]\nkind = \"duckdb\"\ndatabase = \":memory:\"\n[targets.dev]\nconnection = \"wh\"\nloader_schema = \"RAW\"\n[connections.app]\nkind = \"postgres\"\nhost = \"127.0.0.1\"\nport = 1\nuser = \"u\"\npassword = \"p\"\ndatabase = \"app\"\nsslmode = \"disable\"\n[sources.app]\nkind = \"postgres\"\nconnection = \"app\"\ndestination = \"dev\"\ninclude = [\"public.*\"]\n[sources.orders]\nkind = \"files\"\ndescription = \"Test table.\"\nconnection = \"lake\"\npath = \"missing/\"\nformat = \"csv\"\nwrite_strategy = \"append\"\ndestination = \"dev\"\n[connections.lake]\nkind = \"filesystem\"\nroot = '{root}'\n", argv: &["load", "--json"], expected_output: "\"retryable\": false", expected_exit: "exit 1", diff --git a/website/src/content/docs/docs/targets/files.mdx b/website/src/content/docs/docs/targets/files.mdx index 38818f17..4a50ed77 100644 --- a/website/src/content/docs/docs/targets/files.mdx +++ b/website/src/content/docs/docs/targets/files.mdx @@ -95,7 +95,7 @@ table folder, carry over every file of the table folder that the load doesn't re `_sqlload/runs/` log, the load's lease, and any file you keep beside the data), then swap the two folders in one rename (`renameat2` with `RENAME_EXCHANGE` on Linux, `RENAME_SWAP` on macOS) and delete the old one. A reader sees the old files or the new ones, never both. Where the filesystem can't swap folders in one -rename, SQLLoad moves the old folder aside first and warns, which leaves a moment with no folder. A +rename, as on Windows, SQLLoad moves the old folder aside first and warns, which leaves a moment with no folder. A hidden folder left by a stopped or failed load is deleted by the next load. A table at the root of the folder connection has no folder beside it to swap, so it is replaced as in S3. From ee9f948cd0d7b438301debd52c6126152b386104 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 12 Nov 2026 14:06:33 +0000 Subject: [PATCH 07/11] ci(windows): print sqlload's UTF-8 output from the end-to-end script on a cp1252 console --- .github/scripts/windows_e2e.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/scripts/windows_e2e.py b/.github/scripts/windows_e2e.py index 4a555ce1..c841bd3f 100644 --- a/.github/scripts/windows_e2e.py +++ b/.github/scripts/windows_e2e.py @@ -190,6 +190,9 @@ def cdc_checks(exe, work): def main(): + # sqlload prints UTF-8 (box drawing, check marks); a Windows console or pipe defaults to cp1252. + sys.stdout.reconfigure(encoding="utf-8") + sys.stderr.reconfigure(encoding="utf-8") parser = argparse.ArgumentParser() parser.add_argument("exe") parser.add_argument("work", type=pathlib.Path) From 9d9607064fa302cf1fc66c3a34d20ff782992eb4 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 12 Nov 2026 14:48:47 +0000 Subject: [PATCH 08/11] fix(cli): run a command on a thread with 8 MiB of stack, not the caller's A Windows main thread, sqlload.exe's or python.exe's, has 1 MB of stack, which a debug build's copy into a files target outgrew (0xC00000FD, reproduced on Linux with ulimit -s 1024). Every command now runs on a scoped thread with a Linux main thread's default 8 MiB, whoever calls it. --- crates/sqlload/src/command/constants.rs | 3 + .../src/command/main/run_args_with_host.rs | 65 +++++++++++++------ 2 files changed, 49 insertions(+), 19 deletions(-) diff --git a/crates/sqlload/src/command/constants.rs b/crates/sqlload/src/command/constants.rs index c20fcfef..b3707427 100644 --- a/crates/sqlload/src/command/constants.rs +++ b/crates/sqlload/src/command/constants.rs @@ -11,3 +11,6 @@ pub const FORCED_STOP_EXIT: i32 = 130; /// The exit code of a run that failed only in ways running it again unchanged may fix (`EX_TEMPFAIL`): a dropped or refused connection, throttling or server errors that outlasted their retries, or a table another run is loading. pub const RETRYABLE_EXIT: u8 = 75; + +/// The stack a command runs on: a Linux main thread's default, 8 MiB, on every system. +pub const COMMAND_STACK_BYTES: usize = 8 * 1024 * 1024; diff --git a/crates/sqlload/src/command/main/run_args_with_host.rs b/crates/sqlload/src/command/main/run_args_with_host.rs index 0d2a9034..e0ee9517 100644 --- a/crates/sqlload/src/command/main/run_args_with_host.rs +++ b/crates/sqlload/src/command/main/run_args_with_host.rs @@ -12,6 +12,7 @@ use crate::command::_helpers::limits::runtime; use crate::command::_helpers::loader_commands::{exit_code, json_error, run_loader}; use crate::command::_helpers::logging::init_logging; use crate::command::_helpers::output::{config_text, print_lines}; +use crate::command::constants::COMMAND_STACK_BYTES; use crate::command::errors::CommandError; use crate::command::models::{Cli, Command, LoaderRequest}; use crate::project::main::is_project::is_project; @@ -50,26 +51,38 @@ where Ok(runtime) => runtime, Err(message) => return failed(json, name, &message.into()), }; - let outcome = runtime.block_on(async { - if copies || is_project(&text) { - let request = LoaderRequest { - text: &text, - config: &cli.config, - command: cli.command, - host, - no_color: cli.no_color, - variables: Default::default(), - }; - return match copies { - true => run_copy(request).await, - false => run_loader(request).await, - }; - } - run_exporter_command(&cli.config, cli.command) - .await - .map(|lines| (lines, 0)) - .map_err(CommandError::from) + let outcome = on_command_stack(|| { + runtime.block_on(async { + if copies || is_project(&text) { + let request = LoaderRequest { + text: &text, + config: &cli.config, + command: cli.command, + host, + no_color: cli.no_color, + variables: Default::default(), + }; + return match copies { + true => run_copy(request).await, + false => run_loader(request).await, + }; + } + run_exporter_command(&cli.config, cli.command) + .await + .map(|lines| (lines, 0)) + .map_err(CommandError::from) + }) }); + let outcome = match outcome { + Ok(outcome) => outcome, + Err(error) => { + return failed( + json, + name, + &format!("cannot start the command's thread: {error}").into(), + ); + } + }; match outcome { Ok((lines, code)) => { print_lines(lines, cli.no_color); @@ -92,3 +105,17 @@ fn failed(json: bool, code: &str, error: &CommandError) -> u8 { } exit_code(false, error.retryable) } + +/// `body`'s result, run on a thread of its own with `COMMAND_STACK_BYTES` of stack rather than on the caller's: a Windows main thread, the executable's or python.exe's, has 1 MB, which a debug build's copy into a files target outgrows. A panic in `body` carries on in the caller. +fn on_command_stack(body: impl FnOnce() -> T + Send) -> std::io::Result { + std::thread::scope(|scope| { + let thread = std::thread::Builder::new() + .name("sqlload-command".to_owned()) + .stack_size(COMMAND_STACK_BYTES) + .spawn_scoped(scope, body)?; + Ok(match thread.join() { + Ok(value) => value, + Err(panic) => std::panic::resume_unwind(panic), + }) + }) +} From 996e2e214986b89aa4d29a0439f6ef53cd6bb732 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 12 Nov 2026 15:03:29 +0000 Subject: [PATCH 09/11] ci(publish): check Ctrl-C ends a Python-source load from each smoke-tested wheel A table function yields rows and then blocks; the load gets a real Ctrl-C (SIGINT, or a console Ctrl-C event on Windows) and must end at once as the native executable does, with no traceback and nothing committed, and the next load must write every row once. Commands now run on a thread of their own rather than Python's main thread, so this checks Ctrl-C still reaches them. --- .github/scripts/interrupt_python_load.py | 140 +++++++++++++++++++++++ .github/workflows/publish.yml | 2 + 2 files changed, 142 insertions(+) create mode 100644 .github/scripts/interrupt_python_load.py diff --git a/.github/scripts/interrupt_python_load.py b/.github/scripts/interrupt_python_load.py new file mode 100644 index 00000000..39e5c465 --- /dev/null +++ b/.github/scripts/interrupt_python_load.py @@ -0,0 +1,140 @@ +"""Ctrl-C during a Python-source load, through the installed `sqlload` package's console entry. + +Usage: python interrupt_python_load.py (with the Python that has the sqlload wheel installed) + +A table function yields five rows, then blocks. Once it is blocked, the load gets a real Ctrl-C: +SIGINT on Linux and macOS, and on Windows a console Ctrl-C event in the load's own console. The +load must end within a few seconds as Ctrl-C ends the native executable (the signal's exit status, +or STATUS_CONTROL_C_EXIT on Windows), print no Python traceback, and commit nothing; the next load +must then write every row once. The command runs on a thread of its own, not Python's main thread, +so this checks Ctrl-C still reaches it. +""" + +import os +import signal +import subprocess +import sys +import tempfile +import textwrap +import time +from pathlib import Path + +PROJECT = """\ +python_paths = ["sources"] + +[connections.lake] +kind = "filesystem" +root = "lake" + +[targets.lake] +connection = "lake" +path = "{table}/" +format = "csv" +""" + +MODULE = ''' +import time +from pathlib import Path + +import sqlload as sl + +HERE = Path(__file__).resolve().parent.parent + +shop = sl.source("shop") + + +@shop.table(description="Rows that block on the first load.") +def slow(ctx): + for n in range(5): + yield {"id": n, "name": f"row {n}"} + if not (HERE / "release").exists(): + (HERE / "blocked").write_text("blocked") + time.sleep(120) + +''' + +#: `sqlload load`, run as the wheel's console script runs it. +LOAD = [sys.executable, "-c", "from sqlload._cli import main; main()", "load"] + +#: How Windows ends a process on Ctrl-C when nothing handles it. +STATUS_CONTROL_C_EXIT = 0xC000013A + +#: Attaches to the load's console and sends it a Ctrl-C, ignoring the event itself. +WINDOWS_CTRL_C = """ +import ctypes, sys +kernel32 = ctypes.WinDLL("kernel32", use_last_error=True) +kernel32.FreeConsole() +if not kernel32.AttachConsole(int(sys.argv[1])): + sys.exit(f"AttachConsole failed: {ctypes.get_last_error()}") +kernel32.SetConsoleCtrlHandler(None, True) +if not kernel32.GenerateConsoleCtrlEvent(0, 0): + sys.exit(f"GenerateConsoleCtrlEvent failed: {ctypes.get_last_error()}") +""" + + +def start_load(project): + flags = subprocess.CREATE_NEW_CONSOLE if os.name == "nt" else 0 + return subprocess.Popen(LOAD, cwd=project, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, encoding="utf-8", errors="replace", creationflags=flags) + + +def ctrl_c(process): + if os.name == "nt": + subprocess.run([sys.executable, "-c", WINDOWS_CTRL_C, str(process.pid)], check=True) + else: + process.send_signal(signal.SIGINT) + + +def interrupted_exit(returncode): + if os.name == "nt": + return returncode in (STATUS_CONTROL_C_EXIT, STATUS_CONTROL_C_EXIT - 2**32) + return returncode in (-signal.SIGINT, 128 + signal.SIGINT) + + +def written_rows(project): + lines = [line for path in sorted((project / "lake").rglob("*.csv")) for line in path.read_text(encoding="utf-8").splitlines()[1:]] + return sorted(int(line.split(",")[0]) for line in lines) + + +def check(condition, what, output=""): + if not condition: + sys.exit(f"{what}\n{output}") + print(f"ok: {what}", flush=True) + + +def main(): + sys.stdout.reconfigure(encoding="utf-8") + with tempfile.TemporaryDirectory() as folder: + project = Path(folder) + (project / "sources").mkdir() + (project / "lake").mkdir() + (project / "sqlload.toml").write_text(PROJECT, encoding="utf-8") + (project / "sources" / "shop.py").write_text(textwrap.dedent(MODULE), encoding="utf-8") + process = start_load(project) + deadline = time.monotonic() + 120 + while not (project / "blocked").exists(): + if process.poll() is not None or time.monotonic() > deadline: + process.kill() + out, err = process.communicate() + sys.exit(f"the load never reached the blocked table function (exit {process.returncode})\n{out}{err}") + time.sleep(0.2) + time.sleep(1) + ctrl_c(process) + sent = time.monotonic() + try: + out, err = process.communicate(timeout=15) + except subprocess.TimeoutExpired: + process.kill() + out, err = process.communicate() + sys.exit(f"the load was still running 15 s after Ctrl-C\n{out}{err}") + check(time.monotonic() - sent < 5, f"Ctrl-C ended the load within 5 s ({time.monotonic() - sent:.1f} s)", out + err) + check(interrupted_exit(process.returncode), f"the load ended as Ctrl-C ends it (exit {process.returncode})", out + err) + check("Traceback" not in err, "no Python traceback", err) + check(written_rows(project) == [], "the interrupted load committed nothing", str(written_rows(project))) + (project / "release").write_text("release") + done = subprocess.run(LOAD, cwd=project, check=False, capture_output=True, text=True, encoding="utf-8", errors="replace") + check(done.returncode == 0, "the next load succeeds", done.stdout + done.stderr) + check(written_rows(project) == [0, 1, 2, 3, 4], "the next load writes every row once", str(written_rows(project))) + + +if __name__ == "__main__": + main() diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 4503a2ef..8385bb65 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -13,6 +13,7 @@ on: paths: - .github/workflows/publish.yml - .github/scripts/assert_*_wheel_tags.sh + - .github/scripts/interrupt_python_load.py - packages/*/pyproject.toml - Cargo.toml - Cargo.lock @@ -173,6 +174,7 @@ jobs: "${bin}/python" -m pip install --quiet dist/sqlload-*.whl "${bin}/sqlload" --help "${bin}/python" -c "import sqlload, sqlload._native" + "${bin}/python" .github/scripts/interrupt_python_load.py python -m venv smoke-cli cli=smoke-cli/bin if [[ "${RUNNER_OS}" == "Windows" ]]; then cli=smoke-cli/Scripts; fi From 839b94d3d89ec96b74bbd66dac1f1950ab3a7aba Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 12 Nov 2026 15:27:06 +0000 Subject: [PATCH 10/11] fix(files): open a Windows share on localhost through 127.0.0.1 object_store keeps a local store's root as a file:// URL, and the url crate reads file://localhost/share/... back as a drive path, so every file on a \\localhost\ share failed to convert ("Unable to convert URL ... to filesystem path"). The store now opens such a share as \\127.0.0.1\share, the same server; what SQLLoad prints keeps the path as written. The Windows end-to-end checks also write through the machine's own name, as a share on another server is named. --- .github/scripts/windows_e2e.py | 7 +++ .../src/file_store/main/open_folder.rs | 4 +- .../src/store_paths/_helpers/local_root.rs | 18 ++++++ .../src/store_paths/main/local_store_root.rs | 10 +++ crates/sqlload/src/store_paths/main/mod.rs | 1 + .../src/store_paths/tests/test_local_root.rs | 62 ++++++++++++++++++- .../src/store_paths/tests/test_types.rs | 7 +++ 7 files changed, 106 insertions(+), 3 deletions(-) create mode 100644 crates/sqlload/src/store_paths/main/local_store_root.rs diff --git a/.github/scripts/windows_e2e.py b/.github/scripts/windows_e2e.py index c841bd3f..a722ab7d 100644 --- a/.github/scripts/windows_e2e.py +++ b/.github/scripts/windows_e2e.py @@ -64,6 +64,13 @@ def files_checks(exe, work, share): sys.exit(f"no Parquet files under {lake}") print(f"ok: UNC share holds {written}", flush=True) + # The same share named by this machine's name, as a share on another server is named. + named = "\\\\" + os.environ["COMPUTERNAME"] + "\\" + share.lstrip("\\").split("\\", 1)[1] + "\\lake\\by_name" + sqlload(exe, "copy", str(source), named, "--format", "csv") + if not list((work / "share" / "lake" / "by_name").rglob("*.parquet")): + sys.exit(f"no Parquet files under {named}") + print(f"ok: {named} holds Parquet files", flush=True) + # Read back through a file:// URL naming the share, and through a \\?\UNC\ path. server_share = share.lstrip("\\").replace("\\", "/") sqlload(exe, "copy", f"file://{server_share}/lake/orders", f"duckdb://{database}#raw.from_share", "--format", "parquet") diff --git a/crates/sqlload/src/file_store/main/open_folder.rs b/crates/sqlload/src/file_store/main/open_folder.rs index 21fe9ef7..3141e8f3 100644 --- a/crates/sqlload/src/file_store/main/open_folder.rs +++ b/crates/sqlload/src/file_store/main/open_folder.rs @@ -9,6 +9,7 @@ use crate::file_store::errors::StoreError; use crate::file_store::main::s3_builder::s3_builder; use crate::file_store::models::{OpenFolder, SftpStore}; use crate::http::main::install_tls_provider::install_tls_provider; +use crate::store_paths::main::local_store_root::local_store_root; use crate::store_paths::models::{Location, StoreKind}; /// A client for `location`'s store (an SFTP server is connected to on first use); S3 credentials come from `AWS_*` variables, the shared credentials file, web identity or the instance/container role. @@ -47,7 +48,8 @@ pub fn open_folder(location: &Location) -> Result { source: Box::new(error), }) })?; - Arc::new(LocalFileSystem::new_with_prefix(root).map_err(failed)?) + let root = local_store_root(root); + Arc::new(LocalFileSystem::new_with_prefix(root.as_ref()).map_err(failed)?) } StoreKind::Sftp { server, root } => Arc::new(SftpStore { server: (**server).clone(), diff --git a/crates/sqlload/src/store_paths/_helpers/local_root.rs b/crates/sqlload/src/store_paths/_helpers/local_root.rs index 520a141e..49a4d215 100644 --- a/crates/sqlload/src/store_paths/_helpers/local_root.rs +++ b/crates/sqlload/src/store_paths/_helpers/local_root.rs @@ -56,6 +56,24 @@ pub(crate) fn windows_file_url_path(rest: &str, windows: bool) -> Cow<'_, str> { } } +/// `root` with a share on `localhost` named by `127.0.0.1`, the same server, when `windows` (object_store keeps its root as a URL, and url reads `file://localhost/share` as a drive path); unchanged otherwise. +pub(crate) fn share_by_address(root: &str, windows: bool) -> Cow<'_, str> { + if !windows { + return Cow::Borrowed(root); + } + let separated = windows_separators(root, windows); + let on_localhost = separated.strip_prefix("//").and_then(|share| { + share + .get(.."localhost/".len()) + .filter(|host| host.eq_ignore_ascii_case("localhost/")) + .map(|host| &share[host.len()..]) + }); + match on_localhost { + Some(rest) => Cow::Owned(format!(r"\\127.0.0.1\{}", rest.replace('/', r"\"))), + None => Cow::Borrowed(root), + } +} + /// Whether `path` starts with a drive letter and its colon. fn is_drive(path: &str) -> bool { matches!(path.as_bytes(), [letter, b':', ..] if letter.is_ascii_alphabetic()) diff --git a/crates/sqlload/src/store_paths/main/local_store_root.rs b/crates/sqlload/src/store_paths/main/local_store_root.rs new file mode 100644 index 00000000..659532b3 --- /dev/null +++ b/crates/sqlload/src/store_paths/main/local_store_root.rs @@ -0,0 +1,10 @@ +//! The root a local object store opens a filesystem folder at. + +use std::borrow::Cow; + +use crate::store_paths::_helpers::local_root::share_by_address; + +/// `root` as object_store's local store can open it: on Windows a share on `localhost` is named by `127.0.0.1`, the same server; unchanged otherwise. +pub fn local_store_root(root: &str) -> Cow<'_, str> { + share_by_address(root, cfg!(windows)) +} diff --git a/crates/sqlload/src/store_paths/main/mod.rs b/crates/sqlload/src/store_paths/main/mod.rs index 82df1fed..544cd821 100644 --- a/crates/sqlload/src/store_paths/main/mod.rs +++ b/crates/sqlload/src/store_paths/main/mod.rs @@ -2,6 +2,7 @@ pub mod escape_partition_value; pub mod file_url_path; pub mod hive_values; pub mod local_separators; +pub mod local_store_root; pub mod render_template; pub mod resolve_location; pub mod unescape_partition_value; diff --git a/crates/sqlload/src/store_paths/tests/test_local_root.rs b/crates/sqlload/src/store_paths/tests/test_local_root.rs index 91d99a67..5d82e9f2 100644 --- a/crates/sqlload/src/store_paths/tests/test_local_root.rs +++ b/crates/sqlload/src/store_paths/tests/test_local_root.rs @@ -1,7 +1,9 @@ -use crate::store_paths::_helpers::local_root::{strip_folders, windows_file_url_path}; +use crate::store_paths::_helpers::local_root::{ + share_by_address, strip_folders, windows_file_url_path, +}; use crate::store_paths::tests::helpers::root_of; use crate::store_paths::tests::test_types::{ - FileUrlPathTestCase, LocalRootTestCase, StripFoldersTestCase, + FileUrlPathTestCase, LocalRootTestCase, ShareByAddressTestCase, StripFoldersTestCase, }; #[test] @@ -181,3 +183,59 @@ fn given_a_folder_prefix_when_stripping_a_connections_folders_then_windows_ignor ); } } + +#[test] +fn given_filesystem_roots_when_opening_their_store_then_only_a_windows_share_on_localhost_is_named_by_address() + { + let test_cases = [ + ShareByAddressTestCase { + description: "a share on localhost", + root: r"\\localhost\sqlload-e2e\lake\orders", + expected_windows: r"\\127.0.0.1\sqlload-e2e\lake\orders", + }, + ShareByAddressTestCase { + description: "localhost in another case", + root: r"\\LocalHost\exports", + expected_windows: r"\\127.0.0.1\exports", + }, + ShareByAddressTestCase { + description: "a share on localhost as a file:// URL gives it", + root: "//localhost/exports/x", + expected_windows: r"\\127.0.0.1\exports\x", + }, + ShareByAddressTestCase { + description: "an extended-length share on localhost", + root: r"\\?\UNC\localhost\exports\x", + expected_windows: r"\\127.0.0.1\exports\x", + }, + ShareByAddressTestCase { + description: "a server whose name starts with localhost", + root: r"\\localhostname\exports\x", + expected_windows: r"\\localhostname\exports\x", + }, + ShareByAddressTestCase { + description: "a share on another server", + root: r"\\nas01\exports\x", + expected_windows: r"\\nas01\exports\x", + }, + ShareByAddressTestCase { + description: "a drive folder named localhost", + root: r"C:\localhost\x", + expected_windows: r"C:\localhost\x", + }, + ]; + for test_case in test_cases { + assert_eq!( + share_by_address(test_case.root, true), + test_case.expected_windows, + "{} on Windows", + test_case.description + ); + assert_eq!( + share_by_address(test_case.root, false), + test_case.root, + "{} on Unix", + test_case.description + ); + } +} diff --git a/crates/sqlload/src/store_paths/tests/test_types.rs b/crates/sqlload/src/store_paths/tests/test_types.rs index 5864cd2a..85008233 100644 --- a/crates/sqlload/src/store_paths/tests/test_types.rs +++ b/crates/sqlload/src/store_paths/tests/test_types.rs @@ -58,3 +58,10 @@ pub(crate) struct StripFoldersTestCase { pub(crate) expected_windows: Option<&'static str>, pub(crate) expected_unix: Option<&'static str>, } + +/// A filesystem root, as object_store's local store opens it on Windows and on Unix. +pub(crate) struct ShareByAddressTestCase { + pub(crate) description: &'static str, + pub(crate) root: &'static str, + pub(crate) expected_windows: &'static str, +} From 364297712b440256c3d4b98c12c117222eb9e7f4 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 12 Nov 2026 16:18:58 +0000 Subject: [PATCH 11/11] ci(publish): deliver the Windows Ctrl-C check's event as an interactive console does, with a control The Windows wheel's Ctrl-C check found the load still running 15 s after the event. A plain load installs no Ctrl-C handler of its own, and a Windows child inherits its parent's "ignore Ctrl-C" flag, which the runner's processes may carry. Each interrupted child now clears that flag first, as a process in an interactive console has it clear. Before the load, a plain Python process with Ctrl-C's default action gets the same event: once as the runner starts it (printed) and once with the flag cleared (it must end), so a failing load is the load's fault, not the check's. --- .github/scripts/interrupt_python_load.py | 39 ++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/.github/scripts/interrupt_python_load.py b/.github/scripts/interrupt_python_load.py index 39e5c465..9afbcd07 100644 --- a/.github/scripts/interrupt_python_load.py +++ b/.github/scripts/interrupt_python_load.py @@ -8,6 +8,10 @@ or STATUS_CONTROL_C_EXIT on Windows), print no Python traceback, and commit nothing; the next load must then write every row once. The command runs on a thread of its own, not Python's main thread, so this checks Ctrl-C still reaches it. + +On Windows, a plain Python process gets the same Ctrl-C first. How it ends without clearing an inherited +"ignore Ctrl-C" flag is printed, and it must end once it clears the flag, so a failure of the load is +the load's, not this check's. """ import os @@ -53,9 +57,19 @@ def slow(ctx): ''' +#: On Windows, a child inherits its parent's "ignore Ctrl-C" flag, which a CI runner's processes may +#: carry; a process in an interactive console has it clear, so each interrupted child clears it first. +ACCEPT_CTRL_C = "import ctypes; ctypes.windll.kernel32.SetConsoleCtrlHandler(None, False); " if os.name == "nt" else "" + #: `sqlload load`, run as the wheel's console script runs it. LOAD = [sys.executable, "-c", "from sqlload._cli import main; main()", "load"] +#: The same load, as a process in an interactive console runs it. +INTERRUPTED_LOAD = [sys.executable, "-c", ACCEPT_CTRL_C + "from sqlload._cli import main; main()", "load"] + +#: A Python process doing nothing, with Ctrl-C's default action as `sqlload._cli` sets it. +SLEEPER = "import signal, time; signal.signal(signal.SIGINT, signal.SIG_DFL); time.sleep(120)" + #: How Windows ends a process on Ctrl-C when nothing handles it. STATUS_CONTROL_C_EXIT = 0xC000013A @@ -72,9 +86,23 @@ def slow(ctx): """ -def start_load(project): +def start(command, cwd=None): flags = subprocess.CREATE_NEW_CONSOLE if os.name == "nt" else 0 - return subprocess.Popen(LOAD, cwd=project, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, encoding="utf-8", errors="replace", creationflags=flags) + return subprocess.Popen(command, cwd=cwd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, encoding="utf-8", errors="replace", creationflags=flags) + + +def interrupt_sleeper(code): + """Ctrl-C a Python process running `code`, once it has started; whether it ended as Ctrl-C ends it, and how.""" + process = start([sys.executable, "-c", code]) + time.sleep(3) + ctrl_c(process) + try: + process.communicate(timeout=10) + except subprocess.TimeoutExpired: + process.kill() + process.communicate() + return False, "still running 10 s after Ctrl-C" + return interrupted_exit(process.returncode), f"exit {process.returncode}" def ctrl_c(process): @@ -109,7 +137,12 @@ def main(): (project / "lake").mkdir() (project / "sqlload.toml").write_text(PROJECT, encoding="utf-8") (project / "sources" / "shop.py").write_text(textwrap.dedent(MODULE), encoding="utf-8") - process = start_load(project) + if os.name == "nt": + ended, how = interrupt_sleeper(SLEEPER) + print(f"note: a Python process with Ctrl-C's default action, as the runner starts it: {how}", flush=True) + ended, how = interrupt_sleeper(ACCEPT_CTRL_C + SLEEPER) + check(ended, f"Ctrl-C reaches a Python process that accepts it, so this check can deliver it ({how})") + process = start(INTERRUPTED_LOAD, project) deadline = time.monotonic() + 120 while not (project / "blocked").exists(): if process.poll() is not None or time.monotonic() > deadline: