[ADD] migration: decode percent-encoded page anchors before the 13 bump
OpenUpgrade's website post-migration gathers the href of every page
anchor and glues them into a CSS selector:
"selector": ", ".join([link.attrib["href"] for link in links])
An href is not a selector. A French anchor written #principes-mn%C3%A9-
moniques puts a % in it, which no CSS identifier may hold, and the whole
migration dies on SelectorSyntaxError.
Proven by replaying that exact code on the database: three views failed
before, one of them with the % at position 53 -- the very position in
the log. After the fix, three views processed, none failed. Decoding is
enough: the parser accepts #principes-mnémoniques and refuses the
encoded form.
Only page views, only anchor hrefs, only %XX in hex. A % elsewhere in a
URL is legitimate and stays. The decoder lives in pg_temp and leaves
nothing behind.
--- FR ---
La post-migration website d'OpenUpgrade ramasse les href des ancres
d'une page et les recolle en sélecteur CSS. Or un href n'est pas un
sélecteur : une ancre française encodée y met un %, interdit dans un
identifiant CSS, et la migration meurt.
Prouvé en rejouant ce code exact sur la base : trois vues échouaient,
dont une avec le % en position 53 — celle du journal. Après correctif,
trois vues traitées, zéro échec. Décoder suffit : le parseur accepte
#principes-mnémoniques et refuse la forme encodée.
Seulement les vues de page, seulement les href d'ancre, seulement les
%XX hexadécimaux. Un % ailleurs dans une URL est légitime et reste. Le
décodeur vit dans pg_temp et ne laisse rien derrière lui.
Assisted-by: Claude Opus 5
This commit is contained in:
parent
5da0ebaed3
commit
803f1ea8ce
2 changed files with 312 additions and 0 deletions
87
script/odoo/migration/fix_migration_odoo120_to_odoo130.sql
Normal file
87
script/odoo/migration/fix_migration_odoo120_to_odoo130.sql
Normal file
|
|
@ -0,0 +1,87 @@
|
||||||
|
-- © 2021-2026 TechnoLibre (http://www.technolibre.ca)
|
||||||
|
-- License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl)
|
||||||
|
--
|
||||||
|
-- Correctifs à appliquer AVANT qu'OpenUpgrade ne migre vers Odoo 13.
|
||||||
|
--
|
||||||
|
-- En SQL et non en Python : ce fichier tourne sur une base encore en 12,
|
||||||
|
-- que le code de la 13 ne saurait pas charger.
|
||||||
|
|
||||||
|
-- Ancres de page encodées en pourcentage.
|
||||||
|
--
|
||||||
|
-- Le script d'OpenUpgrade `website/migrations/13.0.1.0/post-migration.py`
|
||||||
|
-- ramasse les `href` des ancres d'une page et les RECOLLE en sélecteur CSS :
|
||||||
|
--
|
||||||
|
-- links = doc.cssselect(r"a[href^=\#]:not([href=\#])")
|
||||||
|
-- "selector": ", ".join([link.attrib["href"] for link in links])
|
||||||
|
--
|
||||||
|
-- Or un `href` n'est pas un sélecteur. Une ancre française encodée —
|
||||||
|
-- `#principes-mn%C3%A9moniques` — y introduit un `%`, interdit dans un
|
||||||
|
-- identifiant CSS, et la migration meurt :
|
||||||
|
--
|
||||||
|
-- cssselect.parser.SelectorSyntaxError: Expected selector, got <DELIM '%'>
|
||||||
|
--
|
||||||
|
-- Éprouvé avec le parseur d'Odoo 13 : `#principes-mnémoniques` passe,
|
||||||
|
-- `#principes-mn%C3%A9moniques` non. Décoder suffit donc, et un accent
|
||||||
|
-- reste un identifiant CSS valide.
|
||||||
|
--
|
||||||
|
-- On ne touche QUE les vues qu'OpenUpgrade parcourt — celles qui portent
|
||||||
|
-- une page — et QUE les `href` d'ancre. Un `%` ailleurs dans une URL est
|
||||||
|
-- légitime et reste intact.
|
||||||
|
--
|
||||||
|
-- Rejouable : une fois décodé il n'y a plus de `%XX` à trouver.
|
||||||
|
|
||||||
|
-- Le décodeur vit dans `pg_temp` : il disparaît avec la session psql, et
|
||||||
|
-- ne laisse rien derrière lui dans la base du client.
|
||||||
|
CREATE FUNCTION pg_temp.el_url_decode(entree text) RETURNS text AS $decode$
|
||||||
|
DECLARE
|
||||||
|
octets bytea = '';
|
||||||
|
morceau text;
|
||||||
|
BEGIN
|
||||||
|
-- Deux à deux : « %C3 » devient un octet, tout autre caractère se
|
||||||
|
-- recopie tel quel. On rassemble en bytea AVANT de convertir, car un
|
||||||
|
-- caractère accenté tient sur deux octets et les décoder séparément
|
||||||
|
-- rendrait deux caractères illisibles.
|
||||||
|
FOR morceau IN
|
||||||
|
SELECT (regexp_matches(entree, '(%[0-9A-Fa-f]{2}|.)', 'g'))[1]
|
||||||
|
LOOP
|
||||||
|
IF length(morceau) = 3 AND left(morceau, 1) = '%' THEN
|
||||||
|
octets = octets || decode(substring(morceau, 2, 2), 'hex');
|
||||||
|
ELSE
|
||||||
|
octets = octets || convert_to(morceau, 'UTF8');
|
||||||
|
END IF;
|
||||||
|
END LOOP;
|
||||||
|
RETURN convert_from(octets, 'UTF8');
|
||||||
|
END
|
||||||
|
$decode$ LANGUAGE plpgsql IMMUTABLE STRICT;
|
||||||
|
|
||||||
|
DO $$
|
||||||
|
DECLARE
|
||||||
|
ancre RECORD;
|
||||||
|
combien integer := 0;
|
||||||
|
BEGIN
|
||||||
|
IF to_regclass('ir_ui_view') IS NULL THEN
|
||||||
|
RETURN;
|
||||||
|
END IF;
|
||||||
|
FOR ancre IN
|
||||||
|
SELECT DISTINCT trouve[1] AS brut
|
||||||
|
FROM ir_ui_view v
|
||||||
|
JOIN website_page p ON p.view_id = v.id,
|
||||||
|
LATERAL regexp_matches(
|
||||||
|
v.arch_db, 'href="(#[^"]*%[0-9A-Fa-f]{2}[^"]*)"', 'g'
|
||||||
|
) AS trouve
|
||||||
|
LOOP
|
||||||
|
UPDATE ir_ui_view
|
||||||
|
SET arch_db = replace(
|
||||||
|
arch_db,
|
||||||
|
'href="' || ancre.brut || '"',
|
||||||
|
'href="' || pg_temp.el_url_decode(ancre.brut) || '"'
|
||||||
|
)
|
||||||
|
WHERE position('href="' || ancre.brut || '"' IN arch_db) > 0;
|
||||||
|
combien := combien + 1;
|
||||||
|
RAISE NOTICE 'ancre decodee : % -> %',
|
||||||
|
ancre.brut, pg_temp.el_url_decode(ancre.brut);
|
||||||
|
END LOOP;
|
||||||
|
IF combien > 0 THEN
|
||||||
|
RAISE NOTICE '% ancre(s) de page decodee(s)', combien;
|
||||||
|
END IF;
|
||||||
|
END $$;
|
||||||
225
test/test_fix_migration_120_to_130.py
Normal file
225
test/test_fix_migration_120_to_130.py
Normal file
|
|
@ -0,0 +1,225 @@
|
||||||
|
#!/usr/bin/env python3
|
||||||
|
# © 2021-2026 TechnoLibre (http://www.technolibre.ca)
|
||||||
|
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl)
|
||||||
|
|
||||||
|
"""Le correctif de palier 12 → 13, exécuté contre un vrai PostgreSQL.
|
||||||
|
|
||||||
|
Une assertion sur le TEXTE d'un fichier SQL ne voit pas ce qu'il fait.
|
||||||
|
Celui-ci décode des séquences UTF-8 en pourcentage — `%C3%A9` tient sur
|
||||||
|
DEUX octets — et seule l'exécution prouve qu'on rassemble les octets
|
||||||
|
avant de convertir, plutôt que de produire deux caractères illisibles.
|
||||||
|
|
||||||
|
Ce qu'il répare : OpenUpgrade recolle les `href` d'ancres en sélecteur
|
||||||
|
CSS, où un `%` est interdit. La migration mourait là.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import os
|
||||||
|
import shutil
|
||||||
|
import subprocess
|
||||||
|
import sys
|
||||||
|
import unittest
|
||||||
|
|
||||||
|
sys.path.append(
|
||||||
|
os.path.normpath(os.path.join(os.path.dirname(__file__), ".."))
|
||||||
|
)
|
||||||
|
|
||||||
|
RACINE = os.path.normpath(os.path.join(os.path.dirname(__file__), ".."))
|
||||||
|
SQL = os.path.join(
|
||||||
|
RACINE,
|
||||||
|
"script",
|
||||||
|
"odoo",
|
||||||
|
"migration",
|
||||||
|
"fix_migration_odoo120_to_odoo130.sql",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
class Base(unittest.TestCase):
|
||||||
|
BASE = "tmp_fix_120_130_test"
|
||||||
|
|
||||||
|
@classmethod
|
||||||
|
def setUpClass(cls):
|
||||||
|
if not shutil.which("psql") or not shutil.which("createdb"):
|
||||||
|
raise unittest.SkipTest("PostgreSQL absent")
|
||||||
|
subprocess.run(
|
||||||
|
["dropdb", "--if-exists", cls.BASE], capture_output=True
|
||||||
|
)
|
||||||
|
if subprocess.run(
|
||||||
|
["createdb", cls.BASE], capture_output=True
|
||||||
|
).returncode:
|
||||||
|
raise unittest.SkipTest("createdb impossible")
|
||||||
|
|
||||||
|
@classmethod
|
||||||
|
def tearDownClass(cls):
|
||||||
|
if shutil.which("dropdb"):
|
||||||
|
subprocess.run(
|
||||||
|
["dropdb", "--if-exists", cls.BASE], capture_output=True
|
||||||
|
)
|
||||||
|
|
||||||
|
def sql(self, requete):
|
||||||
|
done = subprocess.run(
|
||||||
|
["psql", "-X", "-w", "-q", "-d", self.BASE, "-tAc", requete],
|
||||||
|
capture_output=True,
|
||||||
|
text=True,
|
||||||
|
)
|
||||||
|
self.assertEqual(done.returncode, 0, done.stderr)
|
||||||
|
return done.stdout.strip()
|
||||||
|
|
||||||
|
def prepare(self, lignes):
|
||||||
|
"""lignes = [(arch, a_une_page)]"""
|
||||||
|
self.sql("DROP TABLE IF EXISTS website_page, ir_ui_view")
|
||||||
|
self.sql(
|
||||||
|
"CREATE TABLE ir_ui_view (id serial PRIMARY KEY, arch_db text)"
|
||||||
|
)
|
||||||
|
self.sql(
|
||||||
|
"CREATE TABLE website_page (id serial PRIMARY KEY, view_id integer)"
|
||||||
|
)
|
||||||
|
for arch, avec_page in lignes:
|
||||||
|
vid = self.sql(
|
||||||
|
"INSERT INTO ir_ui_view (arch_db) VALUES ("
|
||||||
|
+ "'"
|
||||||
|
+ arch.replace("'", "''")
|
||||||
|
+ "') RETURNING id"
|
||||||
|
)
|
||||||
|
if avec_page:
|
||||||
|
self.sql(f"INSERT INTO website_page (view_id) VALUES ({vid})")
|
||||||
|
|
||||||
|
def applique(self):
|
||||||
|
done = subprocess.run(
|
||||||
|
[
|
||||||
|
"psql",
|
||||||
|
"-X",
|
||||||
|
"-w",
|
||||||
|
"-v",
|
||||||
|
"ON_ERROR_STOP=1",
|
||||||
|
"-d",
|
||||||
|
self.BASE,
|
||||||
|
"-f",
|
||||||
|
SQL,
|
||||||
|
],
|
||||||
|
capture_output=True,
|
||||||
|
text=True,
|
||||||
|
)
|
||||||
|
self.assertEqual(done.returncode, 0, done.stdout + done.stderr)
|
||||||
|
return done.stdout + done.stderr
|
||||||
|
|
||||||
|
def arch(self, vid=1):
|
||||||
|
return self.sql(f"SELECT arch_db FROM ir_ui_view WHERE id={vid}")
|
||||||
|
|
||||||
|
|
||||||
|
class TestDecodingTheAnchors(Base):
|
||||||
|
def test_an_accented_anchor_is_decoded(self):
|
||||||
|
self.prepare([('<a href="#principes-mn%C3%A9moniques">x</a>', True)])
|
||||||
|
self.applique()
|
||||||
|
self.assertEqual(self.arch(), '<a href="#principes-mnémoniques">x</a>')
|
||||||
|
|
||||||
|
def test_two_byte_sequences_are_assembled_before_converting(self):
|
||||||
|
# `%C3%A9` est UN caractère sur DEUX octets. Les décoder
|
||||||
|
# séparément rendrait deux caractères illisibles.
|
||||||
|
self.prepare([('<a href="#caf%C3%A9-th%C3%A9">x</a>', True)])
|
||||||
|
self.applique()
|
||||||
|
self.assertEqual(self.arch(), '<a href="#café-thé">x</a>')
|
||||||
|
|
||||||
|
def test_several_anchors_in_one_view(self):
|
||||||
|
self.prepare(
|
||||||
|
[('<a href="#a%C3%A9">1</a><a href="#b%C3%A8">2</a>', True)]
|
||||||
|
)
|
||||||
|
self.applique()
|
||||||
|
self.assertIn("#aé", self.arch())
|
||||||
|
self.assertIn("#bè", self.arch())
|
||||||
|
|
||||||
|
def test_it_is_replayable(self):
|
||||||
|
self.prepare([('<a href="#r%C3%A9seau">x</a>', True)])
|
||||||
|
self.applique()
|
||||||
|
premier = self.arch()
|
||||||
|
sortie = self.applique()
|
||||||
|
self.assertEqual(self.arch(), premier)
|
||||||
|
self.assertNotIn("decodee", sortie)
|
||||||
|
|
||||||
|
|
||||||
|
class TestWhatItMustNotTouch(Base):
|
||||||
|
def test_a_percent_outside_an_anchor_is_left_alone(self):
|
||||||
|
# Un `%` dans une vraie URL est légitime : le décoder changerait
|
||||||
|
# une adresse qui fonctionne.
|
||||||
|
arch = '<a href="/page?q=a%20b">x</a>'
|
||||||
|
self.prepare([(arch, True)])
|
||||||
|
self.applique()
|
||||||
|
self.assertEqual(self.arch(), arch)
|
||||||
|
|
||||||
|
def test_a_view_without_a_page_is_left_alone(self):
|
||||||
|
# OpenUpgrade ne parcourt que les vues qui portent une page :
|
||||||
|
# toucher plus large modifierait du contenu sans raison.
|
||||||
|
arch = '<a href="#r%C3%A9seau">x</a>'
|
||||||
|
self.prepare([(arch, False)])
|
||||||
|
self.applique()
|
||||||
|
self.assertEqual(self.arch(), arch)
|
||||||
|
|
||||||
|
def test_a_lone_percent_is_not_an_encoding(self):
|
||||||
|
# « 100% » n'est pas une séquence : seuls `%XX` hexadécimaux le
|
||||||
|
# sont, et confondre les deux abîmerait du texte.
|
||||||
|
arch = '<a href="#remise-100%-ici">x</a>'
|
||||||
|
self.prepare([(arch, True)])
|
||||||
|
sortie = self.applique()
|
||||||
|
self.assertEqual(self.arch(), arch)
|
||||||
|
# Et il ne doit pas ANNONCER un décodage qui n'a rien changé :
|
||||||
|
# un rapport qui se félicite à vide fait douter du reste.
|
||||||
|
self.assertNotIn("decodee", sortie)
|
||||||
|
|
||||||
|
def test_an_anchor_without_any_percent_is_untouched(self):
|
||||||
|
arch = '<a href="#simple">x</a>'
|
||||||
|
self.prepare([(arch, True)])
|
||||||
|
self.applique()
|
||||||
|
self.assertEqual(self.arch(), arch)
|
||||||
|
|
||||||
|
|
||||||
|
class TestItLeavesNothingBehind(Base):
|
||||||
|
def test_the_decoder_does_not_survive_the_session(self):
|
||||||
|
# La fonction vit dans `pg_temp` : elle disparaît avec psql et ne
|
||||||
|
# laisse rien dans la base du client.
|
||||||
|
self.prepare([('<a href="#a%C3%A9">x</a>', True)])
|
||||||
|
self.applique()
|
||||||
|
reste = self.sql(
|
||||||
|
"SELECT count(*) FROM pg_proc WHERE proname = 'el_url_decode'"
|
||||||
|
)
|
||||||
|
self.assertEqual(reste, "0")
|
||||||
|
|
||||||
|
def test_a_database_without_the_tables_does_not_crash(self):
|
||||||
|
self.sql("DROP TABLE IF EXISTS website_page, ir_ui_view")
|
||||||
|
done = subprocess.run(
|
||||||
|
[
|
||||||
|
"psql",
|
||||||
|
"-X",
|
||||||
|
"-w",
|
||||||
|
"-v",
|
||||||
|
"ON_ERROR_STOP=1",
|
||||||
|
"-d",
|
||||||
|
self.BASE,
|
||||||
|
"-f",
|
||||||
|
SQL,
|
||||||
|
],
|
||||||
|
capture_output=True,
|
||||||
|
text=True,
|
||||||
|
)
|
||||||
|
self.assertEqual(done.returncode, 0, done.stdout + done.stderr)
|
||||||
|
|
||||||
|
|
||||||
|
class TestTheFileIsWiredIn(unittest.TestCase):
|
||||||
|
def test_the_name_matches_what_the_driver_looks_for(self):
|
||||||
|
# Le pilote compose « fix_migration_odoo{(v-1)*10}_to_odoo{v*10} ».
|
||||||
|
self.assertTrue(os.path.isfile(SQL), SQL)
|
||||||
|
self.assertTrue(SQL.endswith("fix_migration_odoo120_to_odoo130.sql"))
|
||||||
|
|
||||||
|
def test_it_runs_before_openupgrade(self):
|
||||||
|
# Il agit sur une base encore en 12 : après OpenUpgrade, il
|
||||||
|
# serait trop tard, la migration aurait déjà échoué.
|
||||||
|
with open(
|
||||||
|
os.path.join(RACINE, "script", "todo", "todo_upgrade.py"),
|
||||||
|
encoding="utf-8",
|
||||||
|
) as handle:
|
||||||
|
src = handle.read()
|
||||||
|
self.assertLess(
|
||||||
|
src.index("- Fix migrate code"), src.index("- Migrate database")
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
Loading…
Reference in a new issue