Skip to content

Invalid q= filter operands return 500 instead of 400 on every endpoint filtering through orm.QuerySetter #23916

Description

@bupd

Expected behavior and actual behavior:

Expected: when a q= filter operand cannot be parsed as the type of the column it targets, the API should reject the request with 400 BadRequest and name the offending parameter, the way other malformed query input is handled.

Actual: the operand is handed to Postgres verbatim, the driver rejects it, and the request fails with 500 and a generic UNKNOWN error. The driver message is written to the core log.

Here is what I get on a 2.15.x deployment. All requests are GET against /api/v2.0, authenticated as admin:

request status response body
projects?q=creation_time=[a~b] 500 {"errors":[{"code":"UNKNOWN","message":"internal server error"}]}
projects?q=creation_time=abc 500 same
projects?q=creation_time={a b} 500 same
projects?q=project_id=abc 500 same
projects?q=project_id=[a~b] 500 same
audit-logs?q=op_time=[x~y] 500 same
repositories?q=creation_time=abc 500 same
users?q=creation_time=abc 500 same
registries?q=creation_time=abc 500 same
replication/policies?q=creation_time=abc 500 same
system/purgeaudit?q=update_time=abc 500 same

Well-formed operands behave correctly, which rules out the filter keys themselves being at fault:

request status
projects?q=creation_time=[2020-01-01~2021-01-01] 200
projects?q=creation_time=~abc 200

The fuzzy-match case survives because it compiles to ILIKE, and Postgres casts the column to text before comparing, so the operand never has to parse as a timestamp.

Two things make this more than cosmetic. The 500 is reachable without credentials on any endpoint that permits anonymous access, so an unauthenticated client can drive an instance's 5xx rate at will:

curl -s -w '\nHTTP %{http_code}\n' -G http://harbor.example.com/api/v2.0/projects \
  --data-urlencode 'q=creation_time=abc'
{"errors":[{"code":"UNKNOWN","message":"internal server error"}]}
HTTP 500

And because these land as genuine server errors, they pollute error-rate dashboards and page on-call for what is really a client typo.

Steps to reproduce the problem:

Any Harbor backed by Postgres reproduces this. Substitute your own credentials and hostname.

HARBOR=http://harbor.example.com/api/v2.0
AUTH='admin:<password>'

# fails with 500
curl -s -u "$AUTH" -w '\nHTTP %{http_code}\n' -G "$HARBOR/projects" --data-urlencode 'q=creation_time=[a~b]'
curl -s -u "$AUTH" -w '\nHTTP %{http_code}\n' -G "$HARBOR/projects" --data-urlencode 'q=creation_time=abc'
curl -s -u "$AUTH" -w '\nHTTP %{http_code}\n' -G "$HARBOR/projects" --data-urlencode 'q=creation_time={a b}'
curl -s -u "$AUTH" -w '\nHTTP %{http_code}\n' -G "$HARBOR/projects" --data-urlencode 'q=project_id=abc'
curl -s -u "$AUTH" -w '\nHTTP %{http_code}\n' -G "$HARBOR/projects" --data-urlencode 'q=project_id=[a~b]'
curl -s -u "$AUTH" -w '\nHTTP %{http_code}\n' -G "$HARBOR/audit-logs" --data-urlencode 'q=op_time=[x~y]'

# succeeds with 200
curl -s -u "$AUTH" -w '\nHTTP %{http_code}\n' -G "$HARBOR/projects" --data-urlencode 'q=creation_time=[2020-01-01~2021-01-01]'
curl -s -u "$AUTH" -w '\nHTTP %{http_code}\n' -G "$HARBOR/projects" --data-urlencode 'q=creation_time=~abc'

Versions:

  • harbor version: reproduced on a 2.15.8 build; the code path described below is unchanged on main as of 3488b6455a7a8cde5cbdf92b243b54567b9e61a9
  • docker engine version: podman 6.1.1
  • docker-compose version: podman-compose 1.6.0
  • postgres version: 16.15

Additional context:

Core log for the six requests in the first block, in order:

[ERROR] [/lib/http/error.go:58]: {"errors":[{"code":"UNKNOWN","message":"unknown: ERROR: invalid input syntax for type timestamp: \"a\" (SQLSTATE 22007)"}]}
[ERROR] [/lib/http/error.go:58]: {"errors":[{"code":"UNKNOWN","message":"unknown: ERROR: invalid input syntax for type timestamp: \"abc\" (SQLSTATE 22007)"}]}
[ERROR] [/lib/http/error.go:58]: {"errors":[{"code":"UNKNOWN","message":"unknown: ERROR: invalid input syntax for type timestamp: \"a\" (SQLSTATE 22007)"}]}
[ERROR] [/lib/http/error.go:58]: {"errors":[{"code":"UNKNOWN","message":"unknown: ERROR: invalid input syntax for type integer: \"abc\" (SQLSTATE 22P02)"}]}
[ERROR] [/lib/http/error.go:58]: {"errors":[{"code":"UNKNOWN","message":"unknown: ERROR: invalid input syntax for type integer: \"a\" (SQLSTATE 22P02)"}]}
[ERROR] [/lib/http/error.go:58]: {"errors":[{"code":"UNKNOWN","message":"unknown: ERROR: invalid input syntax for type timestamp: \"x\" (SQLSTATE 22007)"}]}

SendError in src/lib/http/error.go replaces the payload with a generic message for any status at or above 500, which is why the driver text appears in the log and not in the response.

Root cause looks like setFilters in src/lib/orm/query.go. Once a keyword has been resolved to a filterable field, the value is a plain string taken from the parsed q= expression, and it goes straight onto the beego QuerySeter with no check against the type of the model field it targets:

  • the range branch at lines 205-213 applies r.Min and r.Max as-is
  • the or-list branch at lines 215-222 applies ol.Values as-is
  • the exact-match fallthrough at line 229 applies value as-is

beego assembles the SQL happily, and Postgres is the first component in the chain with an opinion about the type. By then the failure is an opaque driver error that nothing upstream classifies as a client error, so it falls through to GeneralCode and 500.

This shape of bug has been fixed twice at the handler layer, in #23670 for robot accounts and in #23669 for /system/purgeaudit. Both hardened type assertions inside a single handler, so neither covers this path. Every endpoint that builds its query through orm.QuerySetter still has the problem, which is why the matrix above spans projects, repositories, audit logs, users, registries and replication policies. Fixing it once in the ORM layer would cover all of them and remove the need to patch handlers one at a time.

We carry a fix in a downstream fork that validates the operand against the model field type inside the ORM layer and returns BadRequestError naming the parameter, for example invalid value for the query parameter "creation_time": abc. It is deliberately permissive, so anything that resolves today still resolves, including the ILIKE fuzzy path and filters served by a custom FilterFunc. Happy to open a PR against main if that approach sounds acceptable, or to rework it if you would rather see the validation live somewhere else.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions