Corrige LE vrai bug : un nom de champ mot-réservé SQL (ex. "order") faisait disparaître l'objet entier
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
c14f7e9ba5
commit
5c069ae1fe
@@ -1,5 +1,6 @@
|
|||||||
from ..connection import connect
|
from ..connection import connect
|
||||||
from ..constants import FIELD_TYPES
|
from ..constants import FIELD_TYPES
|
||||||
|
from ..quote_ident import quote_ident
|
||||||
from ..slugify import slugify
|
from ..slugify import slugify
|
||||||
from .get_definition import get_definition
|
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 —
|
# 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
|
# 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.
|
# 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"]
|
relation_definition_id = related["id"]
|
||||||
else:
|
else:
|
||||||
sql_type = FIELD_TYPES[ftype]["sql"]
|
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
|
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
|
max_value = field.get("max_value") if ftype in ("nombre_entier", "nombre_decimal") else None
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
from ..connection import connect
|
from ..connection import connect
|
||||||
from ..constants import FIELD_TYPES
|
from ..constants import FIELD_TYPES
|
||||||
|
from ..quote_ident import quote_ident
|
||||||
from ..slugify import slugify
|
from ..slugify import slugify
|
||||||
from ..table_name_for import table_name_for
|
from ..table_name_for import table_name_for
|
||||||
from .get_definition import get_definition
|
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"]))
|
related = get_definition(slug, int(f["relation_definition_id"]))
|
||||||
col = f"{fname}_id"
|
col = f"{fname}_id"
|
||||||
columns_sql.append(
|
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"]
|
relation_definition_id = related["id"]
|
||||||
else:
|
else:
|
||||||
sql_type = FIELD_TYPES[ftype]["sql"]
|
sql_type = FIELD_TYPES[ftype]["sql"]
|
||||||
not_null = " NOT NULL" if required else ""
|
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
|
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
|
max_value = f.get("max_value") if ftype in ("nombre_entier", "nombre_decimal") else None
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
import sqlite3
|
import sqlite3
|
||||||
|
|
||||||
from ..connection import connect
|
from ..connection import connect
|
||||||
|
from ..quote_ident import quote_ident
|
||||||
from ..slugify import slugify
|
from ..slugify import slugify
|
||||||
from .get_definition import get_definition
|
from .get_definition import get_definition
|
||||||
|
|
||||||
@@ -20,7 +21,7 @@ def delete_field(slug, definition_id, field_id):
|
|||||||
col += "_id"
|
col += "_id"
|
||||||
conn = connect(slug)
|
conn = connect(slug)
|
||||||
try:
|
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:
|
except sqlite3.OperationalError:
|
||||||
pass
|
pass
|
||||||
conn.execute("DELETE FROM _fields WHERE id = ?", (field_id,))
|
conn.execute("DELETE FROM _fields WHERE id = ?", (field_id,))
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
import sqlite3
|
import sqlite3
|
||||||
|
|
||||||
from ..connection import connect
|
from ..connection import connect
|
||||||
|
from ..quote_ident import quote_ident
|
||||||
from ..slugify import slugify
|
from ..slugify import slugify
|
||||||
from .get_definition import get_definition
|
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:
|
if old_col != new_col:
|
||||||
try:
|
try:
|
||||||
conn.execute(
|
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:
|
except sqlite3.OperationalError:
|
||||||
pass # SQLite trop ancien pour RENAME COLUMN : la colonne SQL garde son ancien nom
|
pass # SQLite trop ancien pour RENAME COLUMN : la colonne SQL garde son ancien nom
|
||||||
|
|||||||
@@ -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('"', '""') + '"'
|
||||||
@@ -1,4 +1,5 @@
|
|||||||
from ..connection import connect
|
from ..connection import connect
|
||||||
|
from ..quote_ident import quote_ident
|
||||||
from .row_columns_and_values import row_columns_and_values
|
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)
|
cols, values = row_columns_and_values(definition, form_data)
|
||||||
conn = connect(slug)
|
conn = connect(slug)
|
||||||
placeholders = ["?"] * len(cols)
|
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.execute(sql, values)
|
||||||
conn.commit()
|
conn.commit()
|
||||||
conn.close()
|
conn.close()
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
from ..connection import connect
|
from ..connection import connect
|
||||||
|
from ..quote_ident import quote_ident
|
||||||
from ..slugify import slugify
|
from ..slugify import slugify
|
||||||
from ..definitions.definitions_referencing import definitions_referencing
|
from ..definitions.definitions_referencing import definitions_referencing
|
||||||
from ..definitions.get_definition import get_definition
|
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"
|
col = slugify(f["name"]).replace("-", "_") + "_id"
|
||||||
conn = connect(slug)
|
conn = connect(slug)
|
||||||
count = conn.execute(
|
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"]
|
).fetchone()["c"]
|
||||||
conn.close()
|
conn.close()
|
||||||
if count:
|
if count:
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
from ..connection import connect
|
from ..connection import connect
|
||||||
|
from ..quote_ident import quote_ident
|
||||||
from .row_columns_and_values import row_columns_and_values
|
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."""
|
valeurs saisies dans le même formulaire généré que pour la création."""
|
||||||
cols, values = row_columns_and_values(definition, form_data)
|
cols, values = row_columns_and_values(definition, form_data)
|
||||||
conn = connect(slug)
|
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(
|
conn.execute(
|
||||||
f"UPDATE {definition['table_name']} SET {set_clause} WHERE id = ?",
|
f"UPDATE {definition['table_name']} SET {set_clause} WHERE id = ?",
|
||||||
values + [row_id],
|
values + [row_id],
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
from ..connection import connect
|
from ..connection import connect
|
||||||
|
from ..quote_ident import quote_ident
|
||||||
from ..slugify import slugify
|
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("-", "_")
|
fname = slugify(field_def["name"]).replace("-", "_")
|
||||||
col = f"{fname}_id" if field_def["type"] == "relation" else fname
|
col = f"{fname}_id" if field_def["type"] == "relation" else fname
|
||||||
conn = connect(slug)
|
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.commit()
|
||||||
conn.close()
|
conn.close()
|
||||||
|
|||||||
@@ -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
|
||||||
Reference in New Issue
Block a user