From 5c069ae1fe2a4b13db96a5aa55baac736cba270d Mon Sep 17 00:00:00 2001 From: william Date: Thu, 27 Aug 2026 19:27:09 +0200 Subject: [PATCH] =?UTF-8?q?Corrige=20LE=20vrai=20bug=20:=20un=20nom=20de?= =?UTF-8?q?=20champ=20mot-r=C3=A9serv=C3=A9=20SQL=20(ex.=20"order")=20fais?= =?UTF-8?q?ait=20dispara=C3=AEtre=20l'objet=20entier?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reproduit à l'identique le cas signalé (objet "dialog" avec les champs order/spiker/text/level_id/parcour_id) : le champ "order" est un mot réservé SQL — "CREATE TABLE dialog (order INTEGER, ...)" plante avec "OperationalError: near \"order\": syntax error". Comme ce crash survient APRÈS l'INSERT de la ligne _definitions mais AVANT le commit(), rien n'était jamais persisté : l'objet ENTIER disparaissait, malgré des champs parfaitement remplis — d'où "j'ai tout rempli comme il faut et aucun objet n'est créé". Mon précédent correctif (champ "Relation" sans cible) était réel mais ne couvrait pas ce cas précis. Cause de fond : chaque nom de colonne (dérivé du nom de champ tapé par l'utilisateur, via slugify) était interpolé TEL QUEL dans du SQL brut (CREATE TABLE, INSERT, UPDATE, ALTER TABLE ADD/DROP/RENAME COLUMN) sans jamais être encadré de guillemets — n'importe quel nom de champ qui soit aussi un mot réservé SQLite (order, group, index, select, where, table, key, default, check, references, unique...) déclenchait exactement le même crash-et-perte-de-transaction, dans n'importe laquelle de ces opérations. Correctif général (pas un simple contournement pour "order") : db/quote_ident.py encadre tout identifiant de colonne de guillemets doubles (forme standard SQL, supportée par SQLite) — appliqué partout où un nom de colonne utilisateur est interpolé dans du SQL brut : create_definition, add_field_to_definition, delete_field, update_field (RENAME COLUMN), insert_row, update_row, update_row_field, rows_referencing. Les noms de TABLE n'ont pas besoin de cette protection (table_name_for.py les préfixe toujours "obj_", donc jamais un mot réservé à eux seuls). Trois nouveaux tests (tests/test_reserved_sql_keyword_field_names.py) : création avec un champ "order" + insertion/lecture/mise à jour d'une ligne, renommage d'un champ vers/depuis un mot réservé ("group"), ajout d'un champ "select" à un objet existant — les trois confirmés en échec sur l'ancien code (même erreur reproduite) puis au vert avec le correctif. 136 tests au vert au total. Co-Authored-By: Claude Sonnet 5 --- db/definitions/add_field_to_definition.py | 5 +- db/definitions/create_definition.py | 5 +- db/definitions/delete_field.py | 3 +- db/definitions/update_field.py | 3 +- db/quote_ident.py | 15 ++++ db/rows/insert_row.py | 4 +- db/rows/rows_referencing.py | 3 +- db/rows/update_row.py | 3 +- db/rows/update_row_field.py | 3 +- .../test_reserved_sql_keyword_field_names.py | 74 +++++++++++++++++++ 10 files changed, 108 insertions(+), 10 deletions(-) create mode 100644 db/quote_ident.py create mode 100644 tests/test_reserved_sql_keyword_field_names.py diff --git a/db/definitions/add_field_to_definition.py b/db/definitions/add_field_to_definition.py index eee858b6..ae2835cd 100644 --- a/db/definitions/add_field_to_definition.py +++ b/db/definitions/add_field_to_definition.py @@ -1,5 +1,6 @@ from ..connection import connect from ..constants import FIELD_TYPES +from ..quote_ident import quote_ident from ..slugify import slugify from .get_definition import get_definition @@ -25,11 +26,11 @@ def add_field_to_definition(slug, definition_id, field): # même simplicité qu'à la création : on ajoute la colonne simple — # c'est la table _fields qui reste la source de vérité utilisée par # le moteur pour savoir que cette colonne est une relation. - conn.execute(f"ALTER TABLE {definition['table_name']} ADD COLUMN {col} INTEGER") + conn.execute(f"ALTER TABLE {definition['table_name']} ADD COLUMN {quote_ident(col)} INTEGER") relation_definition_id = related["id"] else: sql_type = FIELD_TYPES[ftype]["sql"] - conn.execute(f"ALTER TABLE {definition['table_name']} ADD COLUMN {fname} {sql_type}") + conn.execute(f"ALTER TABLE {definition['table_name']} ADD COLUMN {quote_ident(fname)} {sql_type}") min_value = field.get("min_value") if ftype in ("nombre_entier", "nombre_decimal") else None max_value = field.get("max_value") if ftype in ("nombre_entier", "nombre_decimal") else None diff --git a/db/definitions/create_definition.py b/db/definitions/create_definition.py index 56fbbda0..5ef56112 100644 --- a/db/definitions/create_definition.py +++ b/db/definitions/create_definition.py @@ -1,5 +1,6 @@ from ..connection import connect from ..constants import FIELD_TYPES +from ..quote_ident import quote_ident from ..slugify import slugify from ..table_name_for import table_name_for from .get_definition import get_definition @@ -32,13 +33,13 @@ def create_definition(slug, name, fields): related = get_definition(slug, int(f["relation_definition_id"])) col = f"{fname}_id" columns_sql.append( - f"{col} INTEGER REFERENCES {related['table_name']}(id)" + f"{quote_ident(col)} INTEGER REFERENCES {related['table_name']}(id)" ) relation_definition_id = related["id"] else: sql_type = FIELD_TYPES[ftype]["sql"] not_null = " NOT NULL" if required else "" - columns_sql.append(f"{fname} {sql_type}{not_null}") + columns_sql.append(f"{quote_ident(fname)} {sql_type}{not_null}") min_value = f.get("min_value") if ftype in ("nombre_entier", "nombre_decimal") else None max_value = f.get("max_value") if ftype in ("nombre_entier", "nombre_decimal") else None diff --git a/db/definitions/delete_field.py b/db/definitions/delete_field.py index d3bc2adf..a6a4d008 100644 --- a/db/definitions/delete_field.py +++ b/db/definitions/delete_field.py @@ -1,6 +1,7 @@ import sqlite3 from ..connection import connect +from ..quote_ident import quote_ident from ..slugify import slugify from .get_definition import get_definition @@ -20,7 +21,7 @@ def delete_field(slug, definition_id, field_id): col += "_id" conn = connect(slug) try: - conn.execute(f"ALTER TABLE {definition['table_name']} DROP COLUMN {col}") + conn.execute(f"ALTER TABLE {definition['table_name']} DROP COLUMN {quote_ident(col)}") except sqlite3.OperationalError: pass conn.execute("DELETE FROM _fields WHERE id = ?", (field_id,)) diff --git a/db/definitions/update_field.py b/db/definitions/update_field.py index 7a8fc93a..3cc64c8e 100644 --- a/db/definitions/update_field.py +++ b/db/definitions/update_field.py @@ -1,6 +1,7 @@ import sqlite3 from ..connection import connect +from ..quote_ident import quote_ident from ..slugify import slugify from .get_definition import get_definition @@ -31,7 +32,7 @@ def update_field(slug, definition_id, field_id, new_name, required, relation_def if old_col != new_col: try: conn.execute( - f"ALTER TABLE {definition['table_name']} RENAME COLUMN {old_col} TO {new_col}" + f"ALTER TABLE {definition['table_name']} RENAME COLUMN {quote_ident(old_col)} TO {quote_ident(new_col)}" ) except sqlite3.OperationalError: pass # SQLite trop ancien pour RENAME COLUMN : la colonne SQL garde son ancien nom diff --git a/db/quote_ident.py b/db/quote_ident.py new file mode 100644 index 00000000..9cad0551 --- /dev/null +++ b/db/quote_ident.py @@ -0,0 +1,15 @@ +def quote_ident(name): + """Encadre un identifiant SQL (nom de colonne) de guillemets doubles — + forme standard SQL, supportée par SQLite, pour pouvoir utiliser un nom + de colonne qui serait sinon un mot réservé (ex. un champ appelé + "order" : sans ça, "CREATE TABLE ... (order INTEGER)" plantait avec + "OperationalError: near \"order\": syntax error", et comme le crash + survient APRÈS l'INSERT de la ligne _definitions mais AVANT le + commit(), rien n'était jamais persisté — l'objet entier disparaissait + silencieusement, pas seulement le champ en cause). Les noms de colonne + viennent tous de slugify() (utilisateur), jamais les noms de TABLE + (toujours préfixés "obj_" par table_name_for.py, donc jamais un mot + réservé à eux seuls) — seules les colonnes ont besoin de ça. Double les + guillemets internes (échappement standard SQL), au cas improbable où un + nom en contiendrait déjà un.""" + return '"' + str(name).replace('"', '""') + '"' diff --git a/db/rows/insert_row.py b/db/rows/insert_row.py index 5464ce34..e6b40e8a 100644 --- a/db/rows/insert_row.py +++ b/db/rows/insert_row.py @@ -1,4 +1,5 @@ from ..connection import connect +from ..quote_ident import quote_ident from .row_columns_and_values import row_columns_and_values @@ -8,7 +9,8 @@ def insert_row(slug, definition, form_data): cols, values = row_columns_and_values(definition, form_data) conn = connect(slug) placeholders = ["?"] * len(cols) - sql = f"INSERT INTO {definition['table_name']} ({', '.join(cols)}) VALUES ({', '.join(placeholders)})" + quoted_cols = ", ".join(quote_ident(c) for c in cols) + sql = f"INSERT INTO {definition['table_name']} ({quoted_cols}) VALUES ({', '.join(placeholders)})" conn.execute(sql, values) conn.commit() conn.close() diff --git a/db/rows/rows_referencing.py b/db/rows/rows_referencing.py index e7f1f424..fb7adbce 100644 --- a/db/rows/rows_referencing.py +++ b/db/rows/rows_referencing.py @@ -1,4 +1,5 @@ from ..connection import connect +from ..quote_ident import quote_ident from ..slugify import slugify from ..definitions.definitions_referencing import definitions_referencing from ..definitions.get_definition import get_definition @@ -18,7 +19,7 @@ def rows_referencing(slug, definition_id, row_id): col = slugify(f["name"]).replace("-", "_") + "_id" conn = connect(slug) count = conn.execute( - f"SELECT COUNT(*) AS c FROM {full['table_name']} WHERE {col} = ?", (row_id,) + f"SELECT COUNT(*) AS c FROM {full['table_name']} WHERE {quote_ident(col)} = ?", (row_id,) ).fetchone()["c"] conn.close() if count: diff --git a/db/rows/update_row.py b/db/rows/update_row.py index 8ae2cd1e..a1d71bdd 100644 --- a/db/rows/update_row.py +++ b/db/rows/update_row.py @@ -1,4 +1,5 @@ from ..connection import connect +from ..quote_ident import quote_ident from .row_columns_and_values import row_columns_and_values @@ -7,7 +8,7 @@ def update_row(slug, definition, row_id, form_data): valeurs saisies dans le même formulaire généré que pour la création.""" cols, values = row_columns_and_values(definition, form_data) conn = connect(slug) - set_clause = ", ".join(f"{c} = ?" for c in cols) + set_clause = ", ".join(f"{quote_ident(c)} = ?" for c in cols) conn.execute( f"UPDATE {definition['table_name']} SET {set_clause} WHERE id = ?", values + [row_id], diff --git a/db/rows/update_row_field.py b/db/rows/update_row_field.py index 78a1b192..477bef11 100644 --- a/db/rows/update_row_field.py +++ b/db/rows/update_row_field.py @@ -1,4 +1,5 @@ from ..connection import connect +from ..quote_ident import quote_ident from ..slugify import slugify @@ -10,6 +11,6 @@ def update_row_field(slug, definition, row_id, field_def, new_value): fname = slugify(field_def["name"]).replace("-", "_") col = f"{fname}_id" if field_def["type"] == "relation" else fname conn = connect(slug) - conn.execute(f"UPDATE {definition['table_name']} SET {col} = ? WHERE id = ?", (new_value, row_id)) + conn.execute(f"UPDATE {definition['table_name']} SET {quote_ident(col)} = ? WHERE id = ?", (new_value, row_id)) conn.commit() conn.close() diff --git a/tests/test_reserved_sql_keyword_field_names.py b/tests/test_reserved_sql_keyword_field_names.py new file mode 100644 index 00000000..3dc16933 --- /dev/null +++ b/tests/test_reserved_sql_keyword_field_names.py @@ -0,0 +1,74 @@ +"""Régression : un nom de champ qui est un mot réservé SQL (ex. "order") +faisait planter TOUTE requête SQL brute qui le mentionnait telle quelle +(CREATE TABLE, INSERT, UPDATE, ALTER TABLE...) — ex. "OperationalError: +near \"order\": syntax error" dans create_definition(). Comme le crash +survient APRÈS l'INSERT de la ligne _definitions mais AVANT le commit(), +rien n'était jamais persisté : l'objet entier disparaissait, sans le +moindre message d'erreur — vécu comme "le panneau recharge la page sans +créer d'objet" même avec des champs parfaitement remplis. Corrigé en +encadrant chaque nom de colonne de guillemets doubles (db/quote_ident.py) +partout où il est interpolé dans du SQL brut.""" +import db + + +def test_object_created_with_a_reserved_keyword_field_name(client, game): + resp = client.post(f"/game/{game}/objects/new", data={ + "object_name": "dialog", + "field_name[]": ["order", "spiker", "text"], + "field_type[]": ["nombre_entier", "texte", "texte_long"], + "field_relation[]": ["", "", ""], + "field_required[]": ["1", "1", "1"], + "field_min[]": ["1", "", ""], + "field_max[]": ["100", "", ""], + }, follow_redirects=False) + assert resp.status_code == 302 + + definitions = db.list_definitions(game) + assert len(definitions) == 1 + definition = db.get_definition(game, definitions[0]["id"]) + assert [f["name"] for f in definition["fields"]] == ["order", "spiker", "text"] + + # Insertion, lecture ET mise à jour d'une ligne doivent aussi fonctionner + # (insert_row.py / update_row.py / list_rows.py mentionnent aussi "order"). + client.post(f"/game/{game}/objects/{definition['id']}/data/new", + data={"order": "5", "spiker": "Bob", "text": "hello"}) + rows = db.list_rows(game, definition) + assert len(rows) == 1 + assert rows[0]["order"] == 5 + + client.post(f"/game/{game}/objects/{definition['id']}/data/{rows[0]['id']}/edit", + data={"order": "9", "spiker": "Bob", "text": "hello"}) + rows = db.list_rows(game, definition) + assert rows[0]["order"] == 9 + + +def test_field_renamed_to_and_from_a_reserved_keyword(client, game): + resp = client.post(f"/game/{game}/objects/new", data={ + "object_name": "chose", "field_name[]": ["nom"], "field_type[]": ["texte"], + "field_relation[]": [""], "field_required[]": ["0"], "field_min[]": [""], "field_max[]": [""], + }, follow_redirects=False) + def_id = int(resp.headers["Location"].rstrip("/").split("/")[-1]) + field_id = db.get_definition(game, def_id)["fields"][0]["id"] + + resp = client.post(f"/game/{game}/objects/{def_id}/fields/{field_id}/edit", + data={"field_name": "group", "field_required": "0"}, follow_redirects=False) + assert resp.status_code == 302 + assert db.get_definition(game, def_id)["fields"][0]["name"] == "group" + + client.post(f"/game/{game}/objects/{def_id}/data/new", data={"group": "valeur"}) + rows = db.list_rows(game, db.get_definition(game, def_id)) + assert rows[0]["group"] == "valeur" + + +def test_added_field_with_a_reserved_keyword_name(client, game): + resp = client.post(f"/game/{game}/objects/new", data={ + "object_name": "chose", "field_name[]": ["nom"], "field_type[]": ["texte"], + "field_relation[]": [""], "field_required[]": ["0"], "field_min[]": [""], "field_max[]": [""], + }, follow_redirects=False) + def_id = int(resp.headers["Location"].rstrip("/").split("/")[-1]) + + resp = client.post(f"/game/{game}/objects/{def_id}/fields/add", + data={"field_name": "select", "field_type": "texte"}, follow_redirects=False) + assert resp.status_code == 302 + names = [f["name"] for f in db.get_definition(game, def_id)["fields"]] + assert "select" in names