From 803f1ea8ce28d5795c5fad2d9155e8b126cc12fc Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Sun, 23 Aug 2026 00:00:22 -0400 Subject: [PATCH] [ADD] migration: decode percent-encoded page anchors before the 13 bump MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../fix_migration_odoo120_to_odoo130.sql | 87 +++++++ test/test_fix_migration_120_to_130.py | 225 ++++++++++++++++++ 2 files changed, 312 insertions(+) create mode 100644 script/odoo/migration/fix_migration_odoo120_to_odoo130.sql create mode 100644 test/test_fix_migration_120_to_130.py diff --git a/script/odoo/migration/fix_migration_odoo120_to_odoo130.sql b/script/odoo/migration/fix_migration_odoo120_to_odoo130.sql new file mode 100644 index 0000000..24ea722 --- /dev/null +++ b/script/odoo/migration/fix_migration_odoo120_to_odoo130.sql @@ -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 +-- +-- É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 $$; diff --git a/test/test_fix_migration_120_to_130.py b/test/test_fix_migration_120_to_130.py new file mode 100644 index 0000000..1cbc425 --- /dev/null +++ b/test/test_fix_migration_120_to_130.py @@ -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([('x', True)]) + self.applique() + self.assertEqual(self.arch(), 'x') + + 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([('x', True)]) + self.applique() + self.assertEqual(self.arch(), 'x') + + def test_several_anchors_in_one_view(self): + self.prepare( + [('12', True)] + ) + self.applique() + self.assertIn("#aé", self.arch()) + self.assertIn("#bè", self.arch()) + + def test_it_is_replayable(self): + self.prepare([('x', 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 = 'x' + 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 = 'x' + 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 = 'x' + 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 = 'x' + 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([('x', 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()