Skip to content

Commit 848959d

Browse files
committed
shorten unnecessarily long code
1 parent bc4580d commit 848959d

File tree

3 files changed

+8
-304
lines changed

3 files changed

+8
-304
lines changed

src/groups.jl

Lines changed: 2 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -115,19 +115,7 @@ function Base.getindex(g::Group, name::AbstractString)
115115
return resolve_external_link(g.f, link)
116116
elseif isa(link, SoftLink)
117117
# Resolve soft link within the same file
118-
# For relative paths, resolve from the current group
119-
# For absolute paths, resolve from root
120-
if startswith(link.path, '/')
121-
# Absolute path - resolve from root
122-
resolved_path = JLD2.normalize_hdf5_path(link.path)
123-
# Attempt to resolve, let the file's getindex handle KeyError if target doesn't exist
124-
# This approach provides better error messages automatically
125-
return g.f[resolved_path]
126-
else
127-
# Relative path - resolve from current group
128-
# resolve_relative_soft_link handles error cases appropriately
129-
return resolve_relative_soft_link(g, link.path)
130-
end
118+
return resolve_soft_link(g, link.path)
131119
else
132120
throw(ArgumentError("Unknown link type: $(typeof(link))"))
133121
end
@@ -331,7 +319,7 @@ function parse_link_message(wmsg::HmWrap{HmLinkMessage})::AbstractLink
331319
# External link - two null-terminated strings in external_link blob
332320
blob = wmsg.external_link
333321
# Parse two null-terminated strings but skip the first byte which is reserved
334-
strings = split_null_terminated_strings(blob, 2)
322+
strings = split(String(blob), '\0', keepempty=false)
335323
if length(strings) != 2
336324
throw(InvalidDataException("External link must contain exactly two null-terminated strings"))
337325
end
@@ -347,32 +335,6 @@ function parse_link_message(wmsg::HmWrap{HmLinkMessage})::AbstractLink
347335
end
348336
end
349337

350-
"""
351-
split_null_terminated_strings(blob::Vector{UInt8}) -> Vector{String}
352-
353-
Split a byte blob containing null-terminated strings into separate string components.
354-
Used for parsing external link information.
355-
"""
356-
function split_null_terminated_strings(blob::Vector{UInt8}, start_idx=1)::Vector{String}
357-
strings = String[]
358-
359-
for (i, byte) in enumerate(blob)
360-
if byte == 0x00 # null terminator
361-
if i > start_idx # Don't create empty strings from consecutive nulls
362-
push!(strings, String(blob[start_idx:i-1]))
363-
end
364-
start_idx = i + 1
365-
end
366-
end
367-
368-
# Handle case where last string is not null-terminated
369-
if start_idx <= length(blob)
370-
push!(strings, String(blob[start_idx:end]))
371-
end
372-
373-
return strings
374-
end
375-
376338
"""
377339
group_path(g::Group) -> String
378340
@@ -459,54 +421,6 @@ function find_group_path_simple(root::Group, target::Group, current_path::String
459421
return "/"
460422
end
461423

462-
"""
463-
resolve_relative_soft_link(g::Group, relative_path::String)
464-
465-
Resolve a relative soft link path from the current group.
466-
467-
# Supported Relative Paths
468-
- Simple relative paths without ".." (e.g., "temp", "subgroup/data")
469-
- Paths with ".." components have limited support when groups are loaded from disk
470-
471-
# Algorithm
472-
For simple paths, navigates directly from the current group.
473-
For complex paths with "..", attempts to use group hierarchy but may fall back to error.
474-
"""
475-
function resolve_relative_soft_link(g::Group, relative_path::String)
476-
# Parse the relative path components
477-
path_components = split(relative_path, '/', keepempty=false)
478-
current_group = g
479-
480-
# Check if this path contains ".." components
481-
has_upward_navigation = any(c -> c == "..", path_components)
482-
483-
if has_upward_navigation
484-
# Complex relative path with upward navigation
485-
# This requires knowing the current group's path in the hierarchy
486-
# For groups loaded from disk, this may not work perfectly
487-
try
488-
# Try to use the original path resolution approach
489-
current_path = group_path(current_group)
490-
resolved_path = resolve_soft_link_path(current_path, relative_path)
491-
return current_group.f[resolved_path]
492-
catch
493-
# Fallback error message
494-
throw(KeyError("Relative soft link with '..' components cannot be resolved for groups loaded from disk: $relative_path"))
495-
end
496-
else
497-
# Simple relative path - navigate directly from current group
498-
for component in path_components
499-
if component != "." && !isempty(component)
500-
# Navigate to child - this should work with normal getindex
501-
current_group = current_group[component]
502-
end
503-
# Ignore "." and empty components
504-
end
505-
506-
return current_group
507-
end
508-
end
509-
510424
function load_group(f::JLDFile, offset::RelOffset)
511425
# Messages
512426
links = OrderedDict{String,AbstractLink}()

src/path_resolution.jl

Lines changed: 6 additions & 167 deletions
Original file line numberDiff line numberDiff line change
@@ -1,172 +1,11 @@
1-
# Path Resolution for JLD2 External Links
2-
# This module provides secure path resolution functionality for external file links
3-
4-
"""
5-
resolve_external_file_path(current_file_path::String, external_file_path::String) -> String
6-
7-
Resolve an external file path relative to the current file's directory.
8-
9-
# Arguments
10-
- `current_file_path`: Path to the current JLD2 file (used as reference for relative paths)
11-
- `external_file_path`: The external file path from the external link (may be relative or absolute)
12-
13-
# Returns
14-
The resolved absolute path to the external file.
15-
16-
# Path Processing
17-
- Normalizes path separators for cross-platform compatibility
18-
- Resolves relative paths relative to the current file's directory
19-
20-
# Examples
21-
```julia
22-
# Relative external file path
23-
current = "/home/user/data/main.jld2"
24-
external = "./external_data.jld2"
25-
resolved = resolve_external_file_path(current, external)
26-
# Result: "/home/user/data/external_data.jld2"
27-
28-
# Absolute external file path
29-
current = "/home/user/data/main.jld2"
30-
external = "/shared/datasets/public.jld2"
31-
resolved = resolve_external_file_path(current, external)
32-
# Result: "/shared/datasets/public.jld2"
33-
```
34-
"""
351
function resolve_external_file_path(current_file_path::String, external_file_path::String)
36-
# Handle absolute paths directly
37-
if isabspath(external_file_path)
38-
resolved_path = abspath(external_file_path)
39-
else
40-
# Resolve relative to current file's directory
41-
current_dir = dirname(current_file_path)
42-
resolved_path = abspath(joinpath(current_dir, external_file_path))
43-
end
44-
45-
return resolved_path
46-
end
47-
48-
49-
"""
50-
resolve_soft_link_path(group_path::String, soft_link_path::String) -> String
51-
52-
Resolve a soft link path within the same HDF5 file.
53-
54-
# Arguments
55-
- `group_path`: Current group's absolute path within the HDF5 file (e.g., "/data/measurements")
56-
- `soft_link_path`: The soft link target path (may be absolute or relative)
57-
58-
# Returns
59-
The resolved absolute path within the HDF5 file.
60-
61-
# Path Resolution Rules
62-
- Absolute paths (starting with '/') are used as-is
63-
- Relative paths are resolved relative to the containing group
64-
- Path separators are normalized to '/' for HDF5 compatibility
65-
- ".." components navigate up the group hierarchy
66-
67-
# Examples
68-
```julia
69-
# Absolute soft link
70-
group_path = "/data/measurements"
71-
soft_path = "/results/analysis"
72-
resolved = resolve_soft_link_path(group_path, soft_path)
73-
# Result: "/results/analysis"
74-
75-
# Relative soft link
76-
group_path = "/data/measurements"
77-
soft_path = "../calibration/offset"
78-
resolved = resolve_soft_link_path(group_path, soft_path)
79-
# Result: "/data/calibration/offset"
80-
```
81-
"""
82-
function resolve_soft_link_path(group_path::String, soft_link_path::String)
83-
# Normalize path separators to HDF5 standard
84-
normalized_soft_path = replace(soft_link_path, '\\' => '/')
85-
normalized_group_path = replace(group_path, '\\' => '/')
86-
87-
# Handle absolute paths
88-
if startswith(normalized_soft_path, '/')
89-
return normalize_hdf5_path(normalized_soft_path)
90-
end
91-
92-
# Handle relative paths
93-
# Start from the group itself (not the parent directory)
94-
base_path = normalized_group_path
95-
96-
# Handle empty base path (root group case)
97-
if isempty(base_path) || base_path == "."
98-
base_path = "/"
99-
end
100-
101-
# Resolve relative path components
102-
path_components = split(normalized_soft_path, '/')
103-
current_components = split(base_path, '/', keepempty=false)
104-
105-
for component in path_components
106-
if component == ".." && !isempty(current_components)
107-
pop!(current_components) # Go up one level
108-
elseif component != "." && !isempty(component)
109-
push!(current_components, component) # Add component
110-
end
111-
# Ignore "." and empty components
112-
end
113-
114-
# Reconstruct path
115-
if isempty(current_components)
116-
return "/"
117-
else
118-
return "/" * join(current_components, "/")
119-
end
2+
current_dir = dirname(current_file_path)
3+
return abspath(joinpath(current_dir, external_file_path))
1204
end
1215

122-
"""
123-
normalize_hdf5_path(path::String) -> String
124-
125-
Normalize an HDF5 path to standard format.
126-
127-
# Rules
128-
- Ensures path starts with '/' (absolute)
129-
- Normalizes path separators to '/'
130-
- Removes redundant components like "//" or "/./"
131-
- Handles ".." components properly
132-
133-
# Examples
134-
```julia
135-
normalize_hdf5_path("/data//measurements/./temp") # => "/data/measurements/temp"
136-
normalize_hdf5_path("data/measurements") # => "/data/measurements"
137-
```
138-
"""
139-
function normalize_hdf5_path(path::String)
140-
# Normalize separators
141-
normalized = replace(path, '\\' => '/')
142-
143-
# Ensure absolute path
144-
if !startswith(normalized, '/')
145-
normalized = "/" * normalized
146-
end
147-
148-
# Split, process, and rejoin components
149-
components = split(normalized, '/', keepempty=false)
150-
result_components = String[]
151-
152-
for component in components
153-
if component == ".."
154-
# Go up one level (remove last component if any)
155-
if !isempty(result_components)
156-
pop!(result_components)
157-
end
158-
elseif component != "." && !isempty(component)
159-
# Add non-empty, non-current-directory components
160-
push!(result_components, component)
161-
end
162-
# Ignore "." and empty components
163-
end
164-
165-
# Reconstruct path
166-
if isempty(result_components)
167-
return "/"
168-
else
169-
return "/" * join(result_components, "/")
6+
function resolve_soft_link(g::Group, path::String)
7+
if !startswith(path, '/')
8+
path = group_path(g) * "/" * path
1709
end
10+
return path
17111
end
172-

test/links.jl

Lines changed: 0 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -137,35 +137,6 @@ end
137137
end
138138
end
139139

140-
@testset "Link Message Parsing" begin
141-
@testset "split_null_terminated_strings" begin
142-
# Test basic splitting
143-
blob1 = UInt8[0x48, 0x65, 0x6c, 0x6c, 0x6f, 0x00, 0x57, 0x6f, 0x72, 0x6c, 0x64, 0x00] # "Hello\0World\0"
144-
result1 = JLD2.split_null_terminated_strings(blob1)
145-
@test result1 == ["Hello", "World"]
146-
147-
# Test with trailing string (no final null)
148-
blob2 = UInt8[0x48, 0x65, 0x6c, 0x6c, 0x6f, 0x00, 0x57, 0x6f, 0x72, 0x6c, 0x64] # "Hello\0World"
149-
result2 = JLD2.split_null_terminated_strings(blob2)
150-
@test result2 == ["Hello", "World"]
151-
152-
# Test empty string handling
153-
blob3 = UInt8[0x00, 0x48, 0x69, 0x00] # "\0Hi\0"
154-
result3 = JLD2.split_null_terminated_strings(blob3)
155-
@test result3 == ["Hi"]
156-
157-
# Test single string
158-
blob4 = UInt8[0x48, 0x65, 0x6c, 0x6c, 0x6f] # "Hello"
159-
result4 = JLD2.split_null_terminated_strings(blob4)
160-
@test result4 == ["Hello"]
161-
162-
# Test empty input
163-
blob5 = UInt8[]
164-
result5 = JLD2.split_null_terminated_strings(blob5)
165-
@test result5 == String[]
166-
end
167-
end
168-
169140
@testset "Message Size Calculation" begin
170141
@testset "message_size_for_link" begin
171142
# For these tests, we mainly check that the function doesn't throw
@@ -698,26 +669,6 @@ end
698669
end
699670
end
700671

701-
@testset "Complex Path Resolution Scenarios" begin
702-
# Test edge cases in path resolution with internal functions
703-
704-
# Test path normalization
705-
@test JLD2.normalize_hdf5_path("/data//measurements/./temp") == "/data/measurements/temp"
706-
@test JLD2.normalize_hdf5_path("data/measurements") == "/data/measurements"
707-
@test JLD2.normalize_hdf5_path("/data/./measurements/../calibration") == "/data/calibration"
708-
709-
# Test absolute path resolution (these should work in resolve_soft_link_path)
710-
@test JLD2.resolve_soft_link_path("/data/measurements", "/results/analysis") == "/results/analysis"
711-
@test JLD2.resolve_soft_link_path("/", "/data/test") == "/data/test"
712-
713-
# Test simple relative path resolution (without ".." components)
714-
@test JLD2.resolve_soft_link_path("/data/measurements", "temp") == "/data/measurements/temp"
715-
@test JLD2.resolve_soft_link_path("/", "data/test") == "/data/test"
716-
717-
# Note: Complex relative path resolution with ".." components is tested elsewhere
718-
# and has known limitations for groups loaded from disk
719-
end
720-
721672
@testset "Soft Link Display and Inspection" begin
722673
# Test that soft links display correctly in group listings
723674
mktempdir() do dir

0 commit comments

Comments
 (0)