bug(sqla): get_one() raises InvalidRequestError (HTTP 500) when id contains a comma on a single-PK model
Summary
ModelView.get_one() in flask_admin.contrib.sqla.view calls
session.get(self.model, tools.iterdecode(id)) unconditionally. For a model
with a single primary key, passing an id query string that contains a comma
(e.g. ?id=1,2) makes iterdecode return a 2-tuple, which SQLAlchemy 2.x
rejects in Session.get(). The exception is not caught by details_view /
edit_view, so it surfaces as an unhandled HTTP 500.
This was previously raised in #2788, but the discussion stalled without a
minimal reproducer. Opening a fresh issue with a PEP 723 MRE so the failure
mode is unambiguous, and so the right fix can be discussed separately from
that PR's approach.
Expected behavior
A malformed id parameter on a single-PK model should produce a
user-facing error (e.g. 400 Bad Request or the existing "Record does not
exist." flash + redirect to the list view) — not an unhandled 500.
Actual behavior
sqlalchemy.exc.InvalidRequestError: Incorrect number of values in identifier
to formulate primary key for session.get(); primary key columns are 'widget.id'
…rendered as a 500 Internal Server Error response.
Minimal reproducer (PEP 723)
Save as repro.py and run with uv run repro.py. No venv setup needed.
# /// script
# requires-python = ">=3.10"
# dependencies = [
# "flask-admin",
# "flask-sqlalchemy",
# ]
# ///
"""MRE for flask-admin get_one() crash on `?id=1,2` with a single-PK model."""
from flask import Flask
from flask_admin import Admin
from flask_admin.contrib.sqla import ModelView
from flask_sqlalchemy import SQLAlchemy
app = Flask(__name__)
app.config["SQLALCHEMY_DATABASE_URI"] = "sqlite:///:memory:"
app.config["SECRET_KEY"] = "x"
app.config["WTF_CSRF_ENABLED"] = False
db = SQLAlchemy(app)
class Widget(db.Model):
id = db.Column(db.Integer, primary_key=True)
name = db.Column(db.String(50))
class WidgetView(ModelView):
can_view_details = True
admin = Admin(app)
admin.add_view(WidgetView(Widget, db.session, endpoint="widget"))
with app.app_context():
db.create_all()
db.session.add_all([Widget(name="alpha"), Widget(name="beta")])
db.session.commit()
client = app.test_client()
print("=== id=1 (valid) ===")
rv = client.get("/admin/widget/details/?id=1")
print("status:", rv.status_code)
print("\n=== id=1,2 (malformed for single-PK model) ===")
rv = client.get("/admin/widget/details/?id=1,2")
print("status:", rv.status_code)
print("body head:", rv.data[:120])Output
=== id=1 (valid) ===
status: 200
=== id=1,2 (malformed for single-PK model) ===
status: 500
body head: b'<!doctype html>\n<html lang=en>\n<title>500 Internal Server Error</title>\n<h1>Internal Server Error</h1>...'
The same crash is reachable on /admin/widget/edit/?id=1,2 and any other view
that calls get_one().
Root cause
flask_admin/contrib/sqla/view.py:
def get_one(self, id):
session = _get_deprecated_session(self.session)
return session.get(self.model, tools.iterdecode(id))tools.iterdecode("1,2") returns ("1", "2"). SQLAlchemy 2.x's
Session.get() validates the identifier length against the model's primary
key columns and raises InvalidRequestError when they don't match. Compare
with get_pk_value() (same file), which already branches on
isinstance(self._primary_key, tuple) for the write side — the read side
should mirror that.
Notes on PR #2788
The PR proposes a fix but:
- Uses
tools.escape(id) for the single-PK branch, which encodes the value
in the iterencode escape format (e.g. "1,2" → "1\,2") — a value
that can never match an integer primary key. The crash is avoided only
because session.get returns None and the existing
"record not found → redirect" path takes over.
- Contains a dead
_id = tools.iterdecode(id) line immediately overwritten
in both branches of the if/else.
- The added test uses
follow_redirects=True and asserts on strings
(test1_val_1, test2_val_1) that appear on the list view after the
redirect — not on the details page. It would pass even if get_one
returned None, so it does not actually verify the multi-id case.
Environment
flask-admin from master (commit at time of writing).SQLAlchemy 2.x.- Python 3.10+.
Possible fix sketch (for discussion, not part of this issue)
def get_one(self, id):
session = _get_deprecated_session(self.session)
if isinstance(self._primary_key, tuple):
return session.get(self.model, tools.iterdecode(id))
return session.get(self.model, (id,))…and a test that asserts the status code / flash message on the malformed
input rather than scraping the redirected list page.
bug(sqla):
get_one()raisesInvalidRequestError(HTTP 500) whenidcontains a comma on a single-PK modelSummary
ModelView.get_one()inflask_admin.contrib.sqla.viewcallssession.get(self.model, tools.iterdecode(id))unconditionally. For a modelwith a single primary key, passing an
idquery string that contains a comma(e.g.
?id=1,2) makesiterdecodereturn a 2-tuple, which SQLAlchemy 2.xrejects in
Session.get(). The exception is not caught bydetails_view/edit_view, so it surfaces as an unhandled HTTP 500.This was previously raised in #2788, but the discussion stalled without a
minimal reproducer. Opening a fresh issue with a PEP 723 MRE so the failure
mode is unambiguous, and so the right fix can be discussed separately from
that PR's approach.
Expected behavior
A malformed
idparameter on a single-PK model should produce auser-facing error (e.g.
400 Bad Requestor the existing "Record does notexist." flash + redirect to the list view) — not an unhandled
500.Actual behavior
…rendered as a
500 Internal Server Errorresponse.Minimal reproducer (PEP 723)
Save as
repro.pyand run withuv run repro.py. No venv setup needed.Output
The same crash is reachable on
/admin/widget/edit/?id=1,2and any other viewthat calls
get_one().Root cause
flask_admin/contrib/sqla/view.py:tools.iterdecode("1,2")returns("1", "2"). SQLAlchemy 2.x'sSession.get()validates the identifier length against the model's primarykey columns and raises
InvalidRequestErrorwhen they don't match. Comparewith
get_pk_value()(same file), which already branches onisinstance(self._primary_key, tuple)for the write side — the read sideshould mirror that.
Notes on PR #2788
The PR proposes a fix but:
tools.escape(id)for the single-PK branch, which encodes the valuein the
iterencodeescape format (e.g."1,2"→"1\,2") — a valuethat can never match an integer primary key. The crash is avoided only
because
session.getreturnsNoneand the existing"record not found → redirect" path takes over.
_id = tools.iterdecode(id)line immediately overwrittenin both branches of the
if/else.follow_redirects=Trueand asserts on strings(
test1_val_1,test2_val_1) that appear on the list view after theredirect — not on the details page. It would pass even if
get_onereturned
None, so it does not actually verify the multi-id case.Environment
flask-adminfrommaster(commit at time of writing).SQLAlchemy2.x.Possible fix sketch (for discussion, not part of this issue)
…and a test that asserts the status code / flash message on the malformed
input rather than scraping the redirected list page.