diff --git a/dimod/constrained/constrained.py b/dimod/constrained/constrained.py index 86c4fc93c..dbddb7e07 100644 --- a/dimod/constrained/constrained.py +++ b/dimod/constrained/constrained.py @@ -1785,6 +1785,16 @@ def to_file(self, *, # put everything in a constraints/label/ directory lstr = json.dumps(serialize_variable(label)) + # We want to disallow invalid filenames. We do this via a blacklist + # rather than a whitelist to be as permissive as possible. If we find enough + # other edge cases, we can switch in the future. + # Also, if we find that raising an error places an undue burden on users, we can + # switch to a scheme where invalid names are saved as files with generic directory + # labels. + if "/" in lstr: + # NULL actually passes fine because of the JSON dumps + raise ValueError("Cannot serialize constraint labels containing '/'") + with zf.open(f'constraints/{lstr}/lhs', "w", force_zip64=True) as fdst: constraint.lhs._into_file(fdst) diff --git a/releasenotes/notes/fix-constraint-labels-and-serialization-c0eb5e410293a63c.yaml b/releasenotes/notes/fix-constraint-labels-and-serialization-c0eb5e410293a63c.yaml new file mode 100644 index 000000000..995b0e1ef --- /dev/null +++ b/releasenotes/notes/fix-constraint-labels-and-serialization-c0eb5e410293a63c.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Raise an exception when serializing constrained quadratic models with constraint + labels containing ``"/"``. Previously they would be serialized but would subsequently + break deserialization. + See `#1358 `_. diff --git a/tests/test_constrained.py b/tests/test_constrained.py index 54941b012..8c9d1398f 100644 --- a/tests/test_constrained.py +++ b/tests/test_constrained.py @@ -1334,6 +1334,34 @@ def test_unused_variable(self): self.assertEqual(new.lower_bound(v), cqm.lower_bound(v)) self.assertEqual(new.upper_bound(v), cqm.upper_bound(v)) + def test_unusual_constraint_labels(self): + x, y = dimod.Binaries("xy") + + with self.subTest("/"): + cqm = dimod.CQM() + cqm.add_constraint(x + y <= 5, label="hello/world") + with self.assertRaises(ValueError): + cqm.to_file() + + # NULL actually works because it's passed through JSON + with self.subTest("NULL"): + cqm = dimod.CQM() + cqm.add_constraint(x + y <= 5, label="hello\0world") + with cqm.to_file() as f: + new = dimod.CQM.from_file(f) + + self.assertEqual(list(new.constraints), ["hello\0world"]) + + # a few other potentially tricky characters, not exhaustive + for c in ";,\\>|😜+-&": + with self.subTest(c): + cqm = dimod.CQM() + cqm.add_constraint(x + y <= 5, label=f"hello{c}world") + with cqm.to_file() as f: + new = dimod.CQM.from_file(f) + + self.assertEqual(list(new.constraints), [f"hello{c}world"]) + class TestSetObjective(unittest.TestCase): def test_bqm(self):