fix(executorch): accept only portable names for external_engine_data

py/torch_tensorrt/executorch/_export.py

@@ -614,7 +614,8 @@ def export(
         external_engine_data (Optional[str]): Write every TensorRT engine to an
             ExecuTorch data file named ``<external_engine_data>.ptd`` instead of into
             the ``.pte``, which then holds only a key per engine. Pass the name without
-            the ``.ptd`` extension. The engine is the same bytes either way. Load the
+            the ``.ptd`` extension, using ASCII letters, digits, ``.``, ``-`` and ``_``. The
+            engine is the same bytes either way. Load the
             program with that file as its data file,
             for example ``Runtime.get().load_program(pte, data_path=ptd)``; without it
             the method fails to load. That loader takes one data file. If another

py/torch_tensorrt/executorch/partitioner.py

@@ -110,6 +110,18 @@ def normalize_use_cuda_graphs(use_cuda_graphs: bool | None) -> bytes | None:
     return b"1" if use_cuda_graphs else b"0"
 
 
+# Characters every file system accepts in a file name, with no path separator or drive colon. A
+# leading dot or dash would make a hidden file or a name that reads as a command line option.
+_EXTERNAL_ENGINE_DATA_NAME = re.compile(r"[A-Za-z0-9_][A-Za-z0-9_.-]*")
+# ExecuTorch writes <name>.ptd, and most file systems limit a file name to 255 bytes.
+_MAX_EXTERNAL_ENGINE_DATA_LENGTH = 255 - len(".ptd")
+# Windows opens these as devices, with any extension.
+_WINDOWS_DEVICE_NAMES = frozenset(
+    ["CON", "PRN", "AUX", "NUL"]
+    + [f"{port}{i}" for port in ("COM", "LPT") for i in range(10)]
+)
+
+
 def normalize_external_engine_data(external_engine_data: str | None) -> bytes | None:
     """Validate the .ptd file name, given without its extension; None embeds every engine."""
     if external_engine_data is None:
@@ -119,18 +131,28 @@ def normalize_external_engine_data(external_engine_data: str | None) -> bytes |
             "external_engine_data must be a str or None, got "
             f"{type(external_engine_data).__name__}."
         )
-    # ExecuTorch writes <name>.ptd beside the .pte, so the name may not leave that directory; on
-    # Windows "D:engines" is relative to drive D, hence ":". It appends .ptd only when missing, so
-    # "x.ptd" would share x.ptd with another backend's "x" data and the second write would replace
-    # the first.
-    if (
-        external_engine_data in ("", ".", "..")
-        or any(c in external_engine_data for c in "/\\:\0")
-        or external_engine_data.endswith(".ptd")
-    ):
+    if not _EXTERNAL_ENGINE_DATA_NAME.fullmatch(external_engine_data):
+        raise ValueError(
+            "external_engine_data must use only ASCII letters, digits, '.', '-' and '_', and "
+            f"start with a letter, a digit or '_', got {external_engine_data!r}."
+        )
+    if len(external_engine_data) > _MAX_EXTERNAL_ENGINE_DATA_LENGTH:
+        raise ValueError(
+            f"external_engine_data must be at most {_MAX_EXTERNAL_ENGINE_DATA_LENGTH} "
+            "characters, so the .ptd file name fits the 255 byte limit, got "
+            f"{len(external_engine_data)}."
+        )
+    # ExecuTorch adds .ptd only when it is missing, so "x.ptd" would share x.ptd with another
+    # backend's "x" data and the second write would replace the first.
+    if external_engine_data.endswith(".ptd"):
+        raise ValueError(
+            "external_engine_data must not end with .ptd, which ExecuTorch adds, got "
+            f"{external_engine_data!r}."
+        )
+    if external_engine_data.split(".")[0].upper() in _WINDOWS_DEVICE_NAMES:
         raise ValueError(
-            "external_engine_data must be a non-empty file name without a directory, a drive or the "
-            f".ptd extension, got {external_engine_data!r}."
+            "external_engine_data must not be a Windows device name such as NUL or CON, got "
+            f"{external_engine_data!r}."
         )
     return external_engine_data.encode()
 

tests/py/dynamo/executorch/test_backend.py

@@ -917,8 +917,8 @@ def test_preprocess_without_the_spec_writes_no_external_data():
     "specs, message",
     [
         ([_external_engine_spec(), _external_engine_spec()], "at most once"),
-        ([_external_engine_spec(b"")], "must be a non-empty file name"),
-        ([_external_engine_spec(b"../outside")], "without a directory"),
+        ([_external_engine_spec(b"")], "must use only ASCII letters"),
+        ([_external_engine_spec(b"../outside")], "must use only ASCII letters"),
         ([_external_engine_spec(5)], "not int"),
         ([_external_engine_spec(b"\xff\xfe")], "not valid UTF-8"),
     ],

tests/py/dynamo/executorch/test_external_engine_data.py

@@ -99,14 +99,38 @@ def test_export_leaves_engines_embedded_by_default(program, lowering):
         "a\0b",
         "D:engines",
         "aoti_cuda_blob.ptd",
+        "a b",
+        "a?b",
+        "a*b",
+        "a|b",
+        "a<b",
+        'a"b',
+        "a\nb",
+        "-engines",
+        "engines\u00e9",
+        "x" * 252,
+        "NUL",
+        "con",
+        "Com1",
+        "lpt9.engines",
     ],
 )
 def test_export_rejects_a_name_that_is_not_a_plain_file_name(program, lowering, value):
-    with pytest.raises(ValueError, match="external_engine_data must be"):
+    with pytest.raises(ValueError, match="external_engine_data must"):
         torch_tensorrt.executorch.export(program, external_engine_data=value)
     lowering.assert_not_called()
 
 
+@pytest.mark.unit
+@pytest.mark.parametrize(
+    "value", ["engines", "my-model_v2.1", "_x", "COM10", "x" * 251]
+)
+def test_export_accepts_a_plain_file_name(program, lowering, value):
+    torch_tensorrt.executorch.export(program, external_engine_data=value)
+    specs = _trt_specs(lowering)["forward"]
+    assert [s.value for s in specs if s.key == _KEY] == [value.encode()]
+
+
 @pytest.mark.unit
 def test_the_name_the_docstring_gives_for_sharing_with_cuda_is_accepted(
     program, lowering
@@ -130,7 +154,7 @@ def test_export_rejects_a_name_that_is_not_a_str(program, lowering, value):
 def test_the_partitioner_rejects_a_name_that_is_not_a_plain_file_name(value):
     from torch_tensorrt.executorch.partitioner import TensorRTPartitioner
 
-    with pytest.raises(ValueError, match="external_engine_data must be"):
+    with pytest.raises(ValueError, match="external_engine_data must"):
         TensorRTPartitioner([CompileSpec(_KEY, value.encode())])
 
 
@@ -177,7 +201,7 @@ def test_save_forwards_the_name_to_export(monkeypatch, tmp_path, program, path):
 
 @pytest.mark.unit
 def test_save_checks_the_name_before_the_model_and_input_checks(tmp_path):
-    with pytest.raises(ValueError, match="external_engine_data must be"):
+    with pytest.raises(ValueError, match="external_engine_data must"):
         torch_tensorrt.save(
             torch.fx.symbolic_trace(torch.nn.Identity()),
             str(tmp_path / "model.pte"),