Skip to content

Commit 6fc7ee3

Browse files
ORT 1.24.2 release cherry pick round 3 (#27378)
This cherry-picks the following commits for the release: - #27350 [Build] Fix python packaging pipeline - #27349 [Build] Fix DML Nuget Pipeline for Release - #27334 [DNNL] Fix DNNL build - #27374 Enable Robust Symlink Support for External Data (HF Cache Support) --------- Co-authored-by: Hariharan Seshadri <shariharan91@gmail.com>
1 parent 9871182 commit 6fc7ee3

15 files changed

Lines changed: 461 additions & 41 deletions

File tree

cmake/onnxruntime_unittests.cmake

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1212,6 +1212,13 @@ block()
12121212
${TEST_SRC_DIR}/common/tensor_op_test_utils.h
12131213
)
12141214

1215+
if (onnxruntime_USE_DNNL)
1216+
list(APPEND supporting_test_srcs
1217+
${TEST_SRC_DIR}/common/dnnl_op_test_utils.cc
1218+
${TEST_SRC_DIR}/common/dnnl_op_test_utils.h
1219+
)
1220+
endif()
1221+
12151222
list(APPEND onnxruntime_provider_test_srcs
12161223
${supporting_test_srcs}
12171224
${onnxruntime_unittest_main_src}

onnxruntime/core/framework/tensorprotoutils.cc

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -329,22 +329,48 @@ Status TensorProtoWithExternalDataToTensorProto(
329329
}
330330

331331
Status ValidateExternalDataPath(const std::filesystem::path& base_dir,
332-
const std::filesystem::path& location) {
332+
const std::filesystem::path& location,
333+
const std::filesystem::path& model_path) {
333334
// Reject absolute paths
334335
ORT_RETURN_IF(location.is_absolute(),
335336
"Absolute paths not allowed for external data location");
336337
if (!base_dir.empty()) {
337338
// Resolve and verify the path stays within model directory
338339
auto base_canonical = std::filesystem::weakly_canonical(base_dir);
339340
// If the symlink exists, it resolves to the target path;
340-
// so if the symllink is outside the directory it would be caught here.
341+
// so if the symlink is outside the directory it would be caught here.
341342
auto resolved = std::filesystem::weakly_canonical(base_dir / location);
343+
342344
// Check that resolved path starts with base directory
343345
auto [base_end, resolved_it] = std::mismatch(
344346
base_canonical.begin(), base_canonical.end(),
345347
resolved.begin(), resolved.end());
346-
ORT_RETURN_IF(base_end != base_canonical.end(),
347-
"External data path: ", location, " escapes model directory: ", base_dir);
348+
349+
if (base_end != base_canonical.end()) {
350+
// If validation against logical base_dir fails, we check against the
351+
// real (canonical) path of the model file to support symlinked models
352+
// (e.g. models in Hugging Face Hub local cache).
353+
if (!model_path.empty()) {
354+
auto real_model_dir = std::filesystem::weakly_canonical(model_path).parent_path();
355+
356+
auto [real_base_end, real_resolved_it] = std::mismatch(
357+
real_model_dir.begin(), real_model_dir.end(),
358+
resolved.begin(), resolved.end());
359+
360+
if (real_base_end == real_model_dir.end()) {
361+
return Status::OK();
362+
}
363+
364+
return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL,
365+
"External data path: ", location, " (resolved path: ", resolved,
366+
") escapes both model directory: ", base_dir,
367+
" and real model directory: ", real_model_dir);
368+
}
369+
370+
return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL,
371+
"External data path: ", location, " (resolved path: ", resolved,
372+
") escapes model directory: ", base_dir);
373+
}
348374
}
349375
return Status::OK();
350376
}

onnxruntime/core/framework/tensorprotoutils.h

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -526,16 +526,19 @@ Status TensorProtoWithExternalDataToTensorProto(
526526
ONNX_NAMESPACE::TensorProto& new_tensor_proto);
527527

528528
/// <summary>
529-
/// The functions will make sure the 'location' specified in the external data is under the 'base_dir'.
529+
/// Validates if the external data path is under the model directory.
530+
/// If the model is a symlink, it checks against both the logical model directory (base_dir)
531+
/// and the real/canonical directory of the model.
530532
/// If the `base_dir` is empty, the function only ensures that `location` is not an absolute path.
531533
/// </summary>
532-
/// <param name="base_dir">model location directory</param>
533-
/// <param name="location">location is a string retrieved from TensorProto external data that is not
534-
/// an in-memory tag</param>
535-
/// <returns>The function will fail if the resolved full path is not under the model directory
536-
/// or one of the subdirectories</returns>
534+
/// <param name="base_dir">Logical model location directory</param>
535+
/// <param name="location">Location string retrieved from TensorProto external data</param>
536+
/// <param name="model_path">Optional path to the model file, used for canonical path validation if base_dir check fails</param>
537+
/// <returns>The function will fail if the resolved full path is not under the logical model directory
538+
/// nor the real directory of the model path</returns>
537539
Status ValidateExternalDataPath(const std::filesystem::path& base_dir,
538-
const std::filesystem::path& location);
540+
const std::filesystem::path& location,
541+
const std::filesystem::path& model_path = {});
539542

540543
#endif // !defined(SHARED_PROVIDER)
541544

onnxruntime/core/graph/graph.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3771,7 +3771,7 @@ Status Graph::ConvertInitializersIntoOrtValues() {
37713771
std::unique_ptr<onnxruntime::ExternalDataInfo> external_data_info;
37723772
ORT_RETURN_IF_ERROR(onnxruntime::ExternalDataInfo::Create(tensor_proto.external_data(), external_data_info));
37733773
const auto& location = external_data_info->GetRelPath();
3774-
auto st = utils::ValidateExternalDataPath(model_dir, location);
3774+
auto st = utils::ValidateExternalDataPath(model_dir, location, model_path);
37753775
if (!st.IsOK()) {
37763776
return ORT_MAKE_STATUS(ONNXRUNTIME, FAIL,
37773777
"External data path validation failed for initializer: ", tensor_proto.name(),

onnxruntime/core/providers/shared_library/provider_api.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -453,11 +453,6 @@ inline bool HasExternalDataInMemory(const ONNX_NAMESPACE::TensorProto& ten_proto
453453
return g_host->Utils__HasExternalDataInMemory(ten_proto);
454454
}
455455

456-
inline Status ValidateExternalDataPath(const std::filesystem::path& base_dir,
457-
const std::filesystem::path& location) {
458-
return g_host->Utils__ValidateExternalDataPath(base_dir, location);
459-
}
460-
461456
} // namespace utils
462457

463458
namespace graph_utils {

onnxruntime/core/providers/shared_library/provider_interfaces.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1004,9 +1004,6 @@ struct ProviderHost {
10041004

10051005
virtual bool Utils__HasExternalDataInMemory(const ONNX_NAMESPACE::TensorProto& ten_proto) = 0;
10061006

1007-
virtual Status Utils__ValidateExternalDataPath(const std::filesystem::path& base_path,
1008-
const std::filesystem::path& location) = 0;
1009-
10101007
// Model
10111008
virtual std::unique_ptr<Model> Model__construct(ONNX_NAMESPACE::ModelProto&& model_proto, const PathString& model_path,
10121009
const IOnnxRuntimeOpSchemaRegistryList* local_registries,

onnxruntime/core/session/provider_bridge_ort.cc

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1286,11 +1286,6 @@ struct ProviderHostImpl : ProviderHost {
12861286
return onnxruntime::utils::HasExternalDataInMemory(ten_proto);
12871287
}
12881288

1289-
Status Utils__ValidateExternalDataPath(const std::filesystem::path& base_path,
1290-
const std::filesystem::path& location) override {
1291-
return onnxruntime::utils::ValidateExternalDataPath(base_path, location);
1292-
}
1293-
12941289
// Model (wrapped)
12951290
std::unique_ptr<Model> Model__construct(ONNX_NAMESPACE::ModelProto&& model_proto, const PathString& model_path,
12961291
const IOnnxRuntimeOpSchemaRegistryList* local_registries,
Lines changed: 250 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,250 @@
1+
# Copyright (c) Microsoft Corporation. All rights reserved.
2+
# Licensed under the MIT License.
3+
4+
import os
5+
import shutil
6+
import struct
7+
import tempfile
8+
import unittest
9+
10+
import numpy as np
11+
from onnx import TensorProto, helper, save
12+
13+
import onnxruntime as ort
14+
15+
16+
class TestSymLinkOnnxModelExternalData(unittest.TestCase):
17+
def test_symlink_model_and_data_under_same_directory(self):
18+
# The following directory structure simulates huggingface hub local cache:
19+
# temp_dir/ (This corresponds to .cache/huggingface/hub/model_id/)
20+
# blobs/
21+
# guid1
22+
# guid2
23+
# snapshots/version/
24+
# model.onnx -> ../../blobs/guid1
25+
# data.bin -> ../../blobs/guid2
26+
27+
self.temp_dir = tempfile.mkdtemp()
28+
try:
29+
blobs_dir = os.path.join(self.temp_dir, "blobs")
30+
os.makedirs(blobs_dir)
31+
32+
snapshots_dir = os.path.join(self.temp_dir, "snapshots", "version")
33+
os.makedirs(snapshots_dir)
34+
35+
# Create real files in blobs
36+
# We'll use the helper to create the model, but we need to control where files end up.
37+
# Let's manually create the data file in blobs
38+
data_blob_path = os.path.join(blobs_dir, "guid2")
39+
vals = [float(i) for i in range(10)]
40+
with open(data_blob_path, "wb") as f:
41+
f.writelines(struct.pack("f", v) for v in vals)
42+
43+
# Create model in blobs (referencing "data.bin" as external data)
44+
# When loaded from snapshots/version/model.onnx, ORT looks for snapshots/version/data.bin
45+
46+
input_ = helper.make_tensor_value_info("input", TensorProto.FLOAT, [10])
47+
output = helper.make_tensor_value_info("output", TensorProto.FLOAT, [10])
48+
tensor = helper.make_tensor("external_data", TensorProto.FLOAT, [10], vals)
49+
tensor.data_location = TensorProto.EXTERNAL
50+
tensor.ClearField("float_data")
51+
tensor.ClearField("raw_data")
52+
53+
k = tensor.external_data.add()
54+
k.key = "location"
55+
k.value = "data.bin" # Relative path
56+
57+
offset = tensor.external_data.add()
58+
offset.key = "offset"
59+
offset.value = "0"
60+
61+
length = tensor.external_data.add()
62+
length.key = "length"
63+
length.value = str(len(vals) * 4)
64+
65+
const_node = helper.make_node("Constant", [], ["const_out"], value=tensor)
66+
add_node = helper.make_node("Add", ["input", "const_out"], ["output"])
67+
graph = helper.make_graph([const_node, add_node], "test_graph", [input_], [output])
68+
model = helper.make_model(graph)
69+
70+
model_blob_path = os.path.join(blobs_dir, "guid1")
71+
save(model, model_blob_path)
72+
73+
# Now create symlinks in snapshots
74+
model_symlink_path = os.path.join(snapshots_dir, "model.onnx")
75+
data_symlink_path = os.path.join(snapshots_dir, "data.bin")
76+
77+
try:
78+
os.symlink(model_blob_path, model_symlink_path)
79+
os.symlink(data_blob_path, data_symlink_path)
80+
except (OSError, NotImplementedError) as e:
81+
self.skipTest(f"Skipping symlink test: symlink creation is not supported in this environment: {e}")
82+
83+
sess = ort.InferenceSession(model_symlink_path, providers=["CPUExecutionProvider"])
84+
85+
input_data = np.zeros(10, dtype=np.float32)
86+
res = sess.run(["output"], {"input": input_data})
87+
expected = np.array([float(i) for i in range(10)], dtype=np.float32)
88+
np.testing.assert_allclose(res[0], expected)
89+
90+
finally:
91+
shutil.rmtree(self.temp_dir)
92+
93+
def test_symlink_with_data_in_model_sub_dir(self):
94+
# working directory structure (data is in model sub directory):
95+
# temp_dir/
96+
# blobs/
97+
# guid1
98+
# data/guid2
99+
# snapshots/version/
100+
# model.onnx -> ../../blobs/guid1
101+
# data.bin -> ../../blobs/data/guid2
102+
103+
self.temp_dir = tempfile.mkdtemp()
104+
try:
105+
blobs_dir = os.path.join(self.temp_dir, "blobs")
106+
os.makedirs(blobs_dir)
107+
data_dir = os.path.join(blobs_dir, "data")
108+
os.makedirs(data_dir)
109+
110+
snapshots_dir = os.path.join(self.temp_dir, "snapshots", "version")
111+
os.makedirs(snapshots_dir)
112+
113+
# Create real files in blobs
114+
# We'll use the helper to create the model, but we need to control where files end up.
115+
# Let's manually create the data file in blobs
116+
data_blob_path = os.path.join(data_dir, "guid2")
117+
vals = [float(i) for i in range(10)]
118+
with open(data_blob_path, "wb") as f:
119+
f.writelines(struct.pack("f", v) for v in vals)
120+
121+
# Create model in blobs (referencing "data.bin" as external data)
122+
# When loaded from snapshots/version/model.onnx, ORT looks for snapshots/version/data.bin
123+
124+
input_ = helper.make_tensor_value_info("input", TensorProto.FLOAT, [10])
125+
output = helper.make_tensor_value_info("output", TensorProto.FLOAT, [10])
126+
tensor = helper.make_tensor("external_data", TensorProto.FLOAT, [10], vals)
127+
tensor.data_location = TensorProto.EXTERNAL
128+
tensor.ClearField("float_data")
129+
tensor.ClearField("raw_data")
130+
131+
k = tensor.external_data.add()
132+
k.key = "location"
133+
k.value = "data.bin" # Relative path
134+
135+
offset = tensor.external_data.add()
136+
offset.key = "offset"
137+
offset.value = "0"
138+
139+
length = tensor.external_data.add()
140+
length.key = "length"
141+
length.value = str(len(vals) * 4)
142+
143+
const_node = helper.make_node("Constant", [], ["const_out"], value=tensor)
144+
add_node = helper.make_node("Add", ["input", "const_out"], ["output"])
145+
graph = helper.make_graph([const_node, add_node], "test_graph", [input_], [output])
146+
model = helper.make_model(graph)
147+
148+
model_blob_path = os.path.join(blobs_dir, "guid1")
149+
save(model, model_blob_path)
150+
151+
# Now create symlinks in snapshots
152+
model_symlink_path = os.path.join(snapshots_dir, "model.onnx")
153+
data_symlink_path = os.path.join(snapshots_dir, "data.bin")
154+
155+
try:
156+
os.symlink(model_blob_path, model_symlink_path)
157+
os.symlink(data_blob_path, data_symlink_path)
158+
except (OSError, NotImplementedError) as e:
159+
self.skipTest(f"Skipping symlink test: symlink creation is not supported in this environment: {e}")
160+
161+
sess = ort.InferenceSession(model_symlink_path, providers=["CPUExecutionProvider"])
162+
163+
input_data = np.zeros(10, dtype=np.float32)
164+
res = sess.run(["output"], {"input": input_data})
165+
expected = np.array([float(i) for i in range(10)], dtype=np.float32)
166+
np.testing.assert_allclose(res[0], expected)
167+
168+
finally:
169+
shutil.rmtree(self.temp_dir)
170+
171+
def test_symlink_with_data_not_in_model_sub_dir(self):
172+
# working directory structure (data is not in model directory or its sub directories):
173+
# temp_dir/
174+
# model/
175+
# guid1
176+
# data/
177+
# guid2
178+
# snapshots/version/
179+
# model.onnx -> ../../model/guid1
180+
# data.bin -> ../../data/guid2
181+
182+
self.temp_dir = tempfile.mkdtemp()
183+
try:
184+
model_dir = os.path.join(self.temp_dir, "model")
185+
os.makedirs(model_dir)
186+
data_dir = os.path.join(self.temp_dir, "data")
187+
os.makedirs(data_dir)
188+
189+
snapshots_dir = os.path.join(self.temp_dir, "snapshots", "version")
190+
os.makedirs(snapshots_dir)
191+
192+
# Create real files in data_dir
193+
# We'll use the helper to create the model, but we need to control where files end up.
194+
# Let's manually create the data file in data_dir
195+
data_blob_path = os.path.join(data_dir, "guid2")
196+
vals = [float(i) for i in range(10)]
197+
with open(data_blob_path, "wb") as f:
198+
f.writelines(struct.pack("f", v) for v in vals)
199+
200+
# Create model in model_dir (referencing "data.bin" as external data)
201+
# When loaded from snapshots/version/model.onnx, ORT looks for snapshots/version/data.bin
202+
203+
input_ = helper.make_tensor_value_info("input", TensorProto.FLOAT, [10])
204+
output = helper.make_tensor_value_info("output", TensorProto.FLOAT, [10])
205+
tensor = helper.make_tensor("external_data", TensorProto.FLOAT, [10], vals)
206+
tensor.data_location = TensorProto.EXTERNAL
207+
tensor.ClearField("float_data")
208+
tensor.ClearField("raw_data")
209+
210+
k = tensor.external_data.add()
211+
k.key = "location"
212+
k.value = "data.bin" # Relative path
213+
214+
offset = tensor.external_data.add()
215+
offset.key = "offset"
216+
offset.value = "0"
217+
218+
length = tensor.external_data.add()
219+
length.key = "length"
220+
length.value = str(len(vals) * 4)
221+
222+
const_node = helper.make_node("Constant", [], ["const_out"], value=tensor)
223+
add_node = helper.make_node("Add", ["input", "const_out"], ["output"])
224+
graph = helper.make_graph([const_node, add_node], "test_graph", [input_], [output])
225+
model = helper.make_model(graph)
226+
227+
model_blob_path = os.path.join(model_dir, "guid1")
228+
save(model, model_blob_path)
229+
230+
# Now create symlinks in snapshots
231+
model_symlink_path = os.path.join(snapshots_dir, "model.onnx")
232+
data_symlink_path = os.path.join(snapshots_dir, "data.bin")
233+
234+
try:
235+
os.symlink(model_blob_path, model_symlink_path)
236+
os.symlink(data_blob_path, data_symlink_path)
237+
except (OSError, NotImplementedError) as e:
238+
self.skipTest(f"Skipping symlink test: symlink creation is not supported in this environment: {e}")
239+
240+
with self.assertRaises(Exception) as cm:
241+
ort.InferenceSession(model_symlink_path, providers=["CPUExecutionProvider"])
242+
243+
# We expect an error about external data not under model directory or the real model directory.
244+
self.assertIn("External data path validation failed", str(cm.exception))
245+
finally:
246+
shutil.rmtree(self.temp_dir)
247+
248+
249+
if __name__ == "__main__":
250+
unittest.main()

tools/ci_build/build.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1818,6 +1818,9 @@ def run_onnxruntime_tests(args, source_dir, ctest_path, build_dir, configs):
18181818
onnx_test = False
18191819

18201820
if onnx_test:
1821+
log.info("Testing Symlink ONNX Model and External Data")
1822+
run_subprocess([sys.executable, "onnxruntime_test_python_symlink_data.py"], cwd=cwd, dll_path=dll_path)
1823+
18211824
# Disable python onnx tests for TensorRT and CANN EP, because many tests are
18221825
# not supported yet.
18231826
if args.use_tensorrt or args.use_cann:

0 commit comments

Comments
 (0)