* fix(export): refuse a target whose password holds an unencoded delimiter
The …sinks read a target with two parsers that split an unencoded password
differently. `urlparse`, which reads every target for its database and
table and is all the InfluxDB sink uses, ends the host part at the first
`/`, `?` or `#`, so `influxdb2://acme:Ab/Cd==@localhost` became port `Ab`
and database `Cd==@localhost`. SQLAlchemy, which the SQL sinks connect
with and CrateDB is handed to, ends the password at the first `@`, so
`postgresql://scott:p@ss:w0rd@db/dwd` raised `invalid literal for int()
with base 10: 'w0rd@db'`. Either way the export failed, and the INFO line
naming the database or the traceback the CLI prints carried pieces of the
password past the redaction #2238 added.
Such a target is now refused with `ExportRefusedError` before anything
connects, and the message names no piece of it, only which characters to
percent-encode. What is refused is what the sink misreads: InfluxDB finds
an `@` in a password, as `urlparse` ends the userinfo at the last one, and
the SQL sinks find a `/`, so those keep working as they did.
The advice is only true if an encoded password arrives decoded. SQLAlchemy
decodes one; `urlparse` does not, so the InfluxDB sink would have sent
`Ab%2FCd==` as the token. `ConnectionString.username` and `.password` now
decode, as SQLAlchemy does.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(export): refuse a password with an `@` before a `/`, and before the first log line
Review of #2263 found three gaps in the refusal.
`influxdb2://acme:Ab@C/d==@localhost` was not refused: both parsers end
the password at the first `@` and agree on host `C`, and the search for a
second `@` stopped where the host part ended. Where such a password ends
cannot be told from the string, so the password is now taken to run to the
last `@`. That also counts an `@` in the query of a target with a password;
the refusal says to write it as `%40`, which the query parser decodes.
The check ran after the INFO line naming the target, whose redaction ends
the password at the first `@` and so printed `C/d==`. It runs first now.
`influxdb://localhost:8086/?table=a@b`, which has no password and which
`urlparse` reads correctly, was refused for its port. Digits alone between
the `:` and the end of the host part are now read as a port.
The refusal lists only the characters the sink misreads, so a SQL target
is no longer told to encode a `/` it reads correctly. The changelog says
that the InfluxDB credentials are now decoded, so a literal `%` is `%25`.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(export)!: end the password at the last `@` before the query
The second review of #2263 found the widest reading, to the last `@` in
the whole target, refusing working targets with an `@` in the query, such
as `postgresql://scott:tiger@db/dwd?application_name=me@host`. The
password now ends at the last `@` before the first `?` or `#` after its
first `@`. That still finds `Ab@C/d==` in
`influxdb2://acme:Ab@C/d==@localhost/?database=dwd`, which the first
review asked for, without counting an `@` in the query.
It also found the port rule letting `root:2024/Winter@localhost` through:
digits are now read as a port only when the first `@` comes after a `?` or
`#`, as in `localhost:8086/?table=a@b`. And `[::1]:5432/dwd?table=a@b` was
refused, the IPv6 colons read as the start of a password; a host in
brackets now has none.
What this still cannot find is written in the docstring: a password with
an `@` and a later `?` or `#`, or of digits followed by a `?` or `#`.
The InfluxDB credentials are now decoded, so a literal `%XX` in an org,
password or token changes meaning. That is breaking, small as it is, and
the changelog now says so in its own entry under Changed.
The refusal test no longer patches SQLAlchemy, which the InfluxDB cases
never reach and a base install lacks, and drops a `%40` check every
message passed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(export): tell the sink apart for the refusal as the dispatch does
The third review of #2263 found the refusal reading the sink from the raw
target while the dispatch reads it with `urlparse`, which strips leading
whitespace. ` influxdb2://acme:Ab/Cd==@localhost` was given the SQL set,
which does not refuse a `/`, and then went to the InfluxDB sink, which
misreads one. Both now read the protocol from the one `ConnectionString`.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(export)!: read every target once, as SQLAlchemy reads it
The refusal so far guessed, per sink, where a password ended. Three
review rounds showed that guess cannot be made from the string: each rule
that caught one target refused a working one that reads the same way.
So the targets are no longer read two ways. `ConnectionString` reads a
server target with the pattern SQLAlchemy's `make_url` uses, and every
sink takes its host, port, credentials, database and table from that one
reading. The SQL sink builds its SQLAlchemy URL from it instead of calling
`make_url`, and the CrateDB sink rebuilds its URL from it, with the
password encoded again, instead of `urlunparse`. A test holds the reading
equal to `make_url`'s.
That password runs to the first `@`, so a `/`, `?`, `#` or `:` in it is
simply read: InfluxDB and CrateDB no longer take part of it for the port
and database, and the SQL sinks no longer take their table from what
`urlparse` cut off. The one shape left that cannot be read is an `@` in the
password, which leaves an `@` in the host or port. That is refused with
`ExportRefusedError`, asking for `%40` and naming no part of the target,
as are a target that is not a URL and a port that is not a number. The
reading comes first in `to_target`, so nothing is logged before it.
The per-sink delimiter sets and `unencoded_password_delimiters`, with its
port and IPv6 rules, are gone. File and DuckDB targets are paths and are
read with `urlparse` as before.
Still breaking for InfluxDB: an unencoded `@` in a token, which `urlparse`
read, is now refused and wants `%40`, and a literal `%XX` is decoded.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(export): refuse a port of non-ASCII digits, and read file targets into the same fields
Review of #2263 after the switch to one reading found:
- `str.isdigit` accepts `²`, which `int` then refuses with a ValueError
naming it, so the port check let it through to a traceback. The port
must now be ASCII digits.
- A file or DuckDB target filled none of the fields a server target did,
so `to_sqlalchemy_url()` on one raised AttributeError and every property
branched on which kind it was. Both now fill the same fields once, in
`__init__`, and the properties just read them.
- The query is handed to SQLAlchemy's own `update_query_string` instead of
being grouped by hand.
- An `@` in the path or query of an InfluxDB or CrateDB target with a
`host:port` and no password now reads as ending a password, as it did
for the SQL sinks. That is the reading this PR chose for every sink; the
breaking entry and the docstring say so and how to write it (`%40`).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(export): let SQLAlchemy read a SQL target's database as its version does
Review of #2263 found that building the SQL sink's URL from
`ConnectionString` decoded the database on every SQLAlchemy version,
where 2.0, which the lockfile installs below Python 3.11, leaves
`data%20base` as written. A SQL export there would have gone to a
different database or file. The SQL sink reads its URL with `make_url`
again, the reading `ConnectionString` copies; the table it takes from
`ConnectionString` is the same either way.
It also found prose the change had made wrong: `redact_password` still
said the non-SQL sinks read a target with `urlparse`. And the InfluxDB 2
and 3 examples now say which characters to percent-encode in the token
and the organization, since an `@` or a literal `%XX` in them now has to
be.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* ci: re-run checks for #2263
The pull_request event for 240f4b48 created no workflow runs, so Lint,
Tests, Typecheck, Unused and CodeQL never ran on it. No change.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* test: name the delimited password in the export tests so CodeQL reads it as test data
CodeQL took the test parameter `password` and the constant `_PASSWORD`
as sensitive sources and traced them through `redact_password`, which
gives back a target it finds no password in unchanged, into the INFO lines
of `to_target`. That failed the PR's CodeQL check with two
py/clear-text-logging-sensitive-data alerts on lines main already has.
The values are test data; the names now say so.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(export): rebuild the CrateDB URL from host and credentials alone
CI's remote `test_export_cratedb` found `crate://localhost/?database=test`
handed to pandas as `crate://localhost/`: `URL.set(database=None)` keeps
the empty database, because `set` reads None as "leave it". The CrateDB
URL is now created from the scheme, credentials, host and port, as
`urlunparse` with an empty path and query gave it before.
That leaves `ConnectionString.to_sqlalchemy_url` unused, so it goes; the
test that holds the reading to `make_url` compares the fields every sink
takes from it instead.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs(changelog): say the CrateDB schema is percent-decoded too
Review of #2263 found the breaking entry naming only the InfluxDB sinks,
while a CrateDB schema taken from the path is now decoded as well: a
`my%20schema` that went to schema `my%20schema` now goes to `my schema`.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>