Skip to content

Commit ff57fe1

Browse files
committed
loading: Delete unsafe normpath where possible
Unix path semantics have a design flaw, where if `a` is a symbolic directory link, `x/a/..` refer's to the link target's parent directory rather than `x/`. As a result, `normpath` is not in general safe to run. We were doing this all over the place (either explicitly, or implicitly in `abspath`) causing us to fail to load files if their paths contained this pattern. Fix this by rm'ing normpath in most places in loading and adding an `normpath`/`abspath` kwarg that skips unsafe normalization. Where paths show up in printing, we can possibly put some back after verifying that the path resolution result does not change using `samefile`, but let's do that on a case-by-case basis.
1 parent cab2c74 commit ff57fe1

File tree

7 files changed

+167
-74
lines changed

7 files changed

+167
-74
lines changed

base/initdefs.jl

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -302,15 +302,15 @@ function load_path_expand(env::AbstractString)::Union{String, Nothing}
302302
path = joinpath(depot, "environments", name)
303303
isdir(path) || continue
304304
for proj in project_names
305-
file = abspath(path, proj)
305+
file = joinpath(path, proj)
306306
isfile_casesensitive(file) && return file
307307
end
308308
end
309309
isempty(DEPOT_PATH) && return nothing
310-
return abspath(DEPOT_PATH[1], "environments", name, project_names[end])
310+
return abspath(DEPOT_PATH[1], "environments", name, project_names[end]; safe=true)
311311
end
312312
# otherwise, it's a path
313-
path = abspath(env)
313+
path = abspath(env; safe=true)
314314
if isdir(path)
315315
# directory with a project file?
316316
for proj in project_names
@@ -337,7 +337,7 @@ function active_project(search_load_path::Bool=true)
337337
# there are backedges here from abspath(::AbstractString, ::String)
338338
project = project::String
339339
if !isfile_casesensitive(project) && basename(project) project_names
340-
project = abspath(project, "Project.toml")
340+
project = abspath(project, "Project.toml"; safe=true)
341341
end
342342
return project
343343
end

base/loading.jl

Lines changed: 52 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -964,7 +964,7 @@ function project_file_manifest_path(project_file::String)::Union{Nothing,String}
964964
manifest_path = get(cache.project_file_manifest_path, project_file, missing)
965965
manifest_path === missing || return manifest_path
966966
end
967-
dir = abspath(dirname(project_file))
967+
dir = abspath(dirname(project_file); safe=true)
968968
isfile_casesensitive(project_file) || return nothing
969969
d = parsed_toml(project_file)
970970
base_manifest = workspace_manifest(project_file)
@@ -974,7 +974,7 @@ function project_file_manifest_path(project_file::String)::Union{Nothing,String}
974974
explicit_manifest = get(d, "manifest", nothing)::Union{String, Nothing}
975975
manifest_path = nothing
976976
if explicit_manifest !== nothing
977-
manifest_file = normpath(joinpath(dir, explicit_manifest))
977+
manifest_file = joinpath(dir, explicit_manifest)
978978
if isfile_casesensitive(manifest_file)
979979
manifest_path = manifest_file
980980
end
@@ -998,10 +998,10 @@ end
998998
# given a directory (implicit env from LOAD_PATH) and a name,
999999
# check if it is an implicit package
10001000
function entry_point_and_project_file_inside(dir::String, name::String)::Union{Tuple{Nothing,Nothing},Tuple{String,Nothing},Tuple{String,String}}
1001-
path = normpath(joinpath(dir, "src", "$name.jl"))
1001+
path = joinpath(dir, "src", "$name.jl")
10021002
isfile_casesensitive(path) || return nothing, nothing
10031003
for proj in project_names
1004-
project_file = normpath(joinpath(dir, proj))
1004+
project_file = joinpath(dir, proj)
10051005
isfile_casesensitive(project_file) || continue
10061006
return path, project_file
10071007
end
@@ -1018,7 +1018,7 @@ function entry_point_and_project_file(dir::String, name::String)::Union{Tuple{No
10181018
path, project_file = entry_point_and_project_file_inside(dir_jl, name)
10191019
path === nothing || return path, project_file
10201020
# check for less likely case with a bare file and no src directory last to minimize stat calls
1021-
path = normpath(joinpath(dir, "$name.jl"))
1021+
path = joinpath(dir, "$name.jl")
10221022
isfile_casesensitive(path) && return path, nothing
10231023
return nothing, nothing
10241024
end
@@ -1038,9 +1038,9 @@ end
10381038

10391039
# given a path, name, and possibly an entryfile, return the entry point
10401040
function entry_path(path::String, name::String, entryfile::Union{Nothing,String})::String
1041-
isfile_casesensitive(path) && return normpath(path)
1041+
isfile_casesensitive(path) && return path
10421042
entrypoint = entryfile === nothing ? joinpath("src", "$name.jl") : entryfile
1043-
return normpath(joinpath(path, entrypoint))
1043+
return joinpath(path, entrypoint)
10441044
end
10451045

10461046
## explicit project & manifest API ##
@@ -1204,7 +1204,7 @@ function explicit_manifest_uuid_load_spec(project_file::String, pkg::PkgId)::Uni
12041204
error("failed to find source of parent package: \"$name\"")
12051205
end
12061206
parent_path = parent_load_spec.path
1207-
p = normpath(dirname(parent_path), "..")
1207+
p = joinpath(dirname(parent_path), "..")
12081208
return PkgLoadSpec(find_ext_path(p, pkg.name), parent_load_spec.julia_syntax_version)
12091209
end
12101210
end
@@ -1228,7 +1228,7 @@ function explicit_manifest_entry_load_spec(manifest_file::String, pkg::PkgId, en
12281228
path = get(entry, "path", nothing)::Union{Nothing, String}
12291229
entryfile = get(entry, "entryfile", nothing)::Union{Nothing, String}
12301230
if path !== nothing
1231-
path = entry_path(normpath(abspath(dirname(manifest_file), path)), pkg.name, entryfile)
1231+
path = entry_path(joinpath(dirname(manifest_file), path), pkg.name, entryfile)
12321232
return PkgLoadSpec(path, syntax_version)
12331233
end
12341234
hash = get(entry, "git-tree-sha1", nothing)::Union{Nothing, String}
@@ -1248,7 +1248,7 @@ function explicit_manifest_entry_load_spec(manifest_file::String, pkg::PkgId, en
12481248
for slug in (version_slug(uuid, hash), version_slug(uuid, hash, 4))
12491249
for depot in DEPOT_PATH
12501250
path = joinpath(depot, "packages", pkg.name, slug)
1251-
ispath(path) && return PkgLoadSpec(entry_path(abspath(path), pkg.name, entryfile), syntax_version)
1251+
ispath(path) && return PkgLoadSpec(entry_path(abspath(path; safe=true), pkg.name, entryfile), syntax_version)
12521252
end
12531253
end
12541254
# no depot contains the package, return missing to stop looking
@@ -2394,9 +2394,9 @@ function _include_dependency!(dep_list::Vector{Any}, track_dependencies::Bool,
23942394
track_content::Bool, path_may_be_dir::Bool)
23952395
prev = source_path(nothing)
23962396
if prev === nothing
2397-
path = abspath(_path)
2397+
path = abspath(_path; safe=true)
23982398
else
2399-
path = normpath(joinpath(dirname(prev), _path))
2399+
path = normpath(joinpath(dirname(prev), _path); safe=true)
24002400
end
24012401
if !track_dependencies[]
24022402
if !path_may_be_dir && !isfile(path)
@@ -3003,9 +3003,9 @@ function require_stdlib(package_uuidkey::PkgId, ext::Union{Nothing, String}, fro
30033003
if from_stdlib
30043004
# first since this is a stdlib, try to look there directly first
30053005
if ext === nothing
3006-
sourcepath = normpath(env, this_uuidkey.name, "src", this_uuidkey.name * ".jl")
3006+
sourcepath = joinpath(env, this_uuidkey.name, "src", this_uuidkey.name * ".jl")
30073007
else
3008-
sourcepath = find_ext_path(normpath(joinpath(env, package_uuidkey.name)), ext)
3008+
sourcepath = find_ext_path(joinpath(env, package_uuidkey.name), ext)
30093009
end
30103010
set_pkgorigin_version_path(this_uuidkey, sourcepath)
30113011
newm = _require_search_from_serialized(this_uuidkey, PkgLoadSpec(sourcepath, VERSION), UInt128(0), false; DEPOT_PATH=depot_path)
@@ -3191,13 +3191,14 @@ function evalfile(path::AbstractString, args::Vector{String}=String[])
31913191
end
31923192
evalfile(path::AbstractString, args::Vector) = evalfile(path, String[args...])
31933193

3194+
# Used in Pkg.jl
31943195
function load_path_setup_code(load_path::Bool=true)
31953196
code = """
3196-
append!(empty!(Base.DEPOT_PATH), $(repr(map(abspath, DEPOT_PATH))))
3197-
append!(empty!(Base.DL_LOAD_PATH), $(repr(map(abspath, DL_LOAD_PATH))))
3197+
append!(empty!(Base.DEPOT_PATH), $(repr(map(x -> abspath(x; safe=true), DEPOT_PATH))))
3198+
append!(empty!(Base.DL_LOAD_PATH), $(repr(map(x -> abspath(x; safe=true), DL_LOAD_PATH))))
31983199
"""
31993200
if load_path
3200-
load_path = map(abspath, Base.load_path())
3201+
load_path = map(x -> abspath(x; safe=true), Base.load_path())
32013202
path_sep = Sys.iswindows() ? ';' : ':'
32023203
any(path -> path_sep in path, load_path) &&
32033204
error("LOAD_PATH entries cannot contain $(repr(path_sep))")
@@ -3283,9 +3284,9 @@ function create_expr_cache(pkg::PkgId, input::PkgLoadSpec, output::String, outpu
32833284
@nospecialize internal_stderr internal_stdout
32843285
rm(output, force=true) # Remove file if it exists
32853286
output_o === nothing || rm(output_o, force=true)
3286-
depot_path = String[abspath(x) for x in DEPOT_PATH]
3287-
dl_load_path = String[abspath(x) for x in DL_LOAD_PATH]
3288-
load_path = String[abspath(x) for x in Base.load_path()]
3287+
depot_path = String[abspath(x; safe=true) for x in DEPOT_PATH]
3288+
dl_load_path = String[abspath(x; safe=true) for x in DL_LOAD_PATH]
3289+
load_path = String[abspath(x; safe=true) for x in Base.load_path()]
32893290
# if pkg is a stdlib, append its parent Project.toml to the load path
32903291
triggers = get(EXT_PRIMED, pkg, nothing)
32913292
if triggers !== nothing
@@ -3343,7 +3344,7 @@ function create_expr_cache(pkg::PkgId, input::PkgLoadSpec, output::String, outpu
33433344
Base.track_nested_precomp($(_pkg_str(vcat(Base.precompilation_stack, pkg))))
33443345
Base.loadable_extensions = $(_pkg_str(loadable_exts))
33453346
Base.precompiling_extension = $(loading_extension)
3346-
Base.include_package_for_output($(_pkg_str(pkg)), $(repr(abspath(input.path))), $(repr(input.julia_syntax_version)), $(repr(depot_path)), $(repr(dl_load_path)),
3347+
Base.include_package_for_output($(_pkg_str(pkg)), $(repr(abspath(input.path; safe=true))), $(repr(input.julia_syntax_version)), $(repr(depot_path)), $(repr(dl_load_path)),
33473348
$(repr(load_path)), $(_pkg_str(concrete_deps)), $(repr(source_path(nothing))))
33483349
""")
33493350
close(io.in)
@@ -3372,7 +3373,7 @@ function compilecache_path(pkg::PkgId, prefs_hash::UInt64; flags::CacheFlags=Cac
33723373
cachepath = joinpath(DEPOT_PATH[1], entrypath)
33733374
isdir(cachepath) || mkpath(cachepath)
33743375
if pkg.uuid === nothing
3375-
abspath(cachepath, entryfile) * ".ji"
3376+
joinpath(cachepath, entryfile) * ".ji"
33763377
else
33773378
crc = _crc32c(project)
33783379
crc = _crc32c(unsafe_string(JLOptions().image_file), crc)
@@ -3387,7 +3388,7 @@ function compilecache_path(pkg::PkgId, prefs_hash::UInt64; flags::CacheFlags=Cac
33873388

33883389
crc = _crc32c(prefs_hash, crc)
33893390
project_precompile_slug = slug(crc, 5)
3390-
abspath(cachepath, string(entryfile, "_", project_precompile_slug, ".ji"))
3391+
joinpath(cachepath, string(entryfile, "_", project_precompile_slug, ".ji"))
33913392
end
33923393
end
33933394

@@ -3594,9 +3595,27 @@ function CacheHeaderIncludes(dep_tuple::Tuple{Module, String, UInt64, UInt32, Fl
35943595
return CacheHeaderIncludes(PkgId(dep_tuple[1]), dep_tuple[2:end]..., String[])
35953596
end
35963597

3597-
function replace_depot_path(path::AbstractString, depots::Vector{String}=normalize_depots_for_relocation())
3598+
function replace_depot_path(path::AbstractString, depots::Vector{Union{String, Tuple{String, String}}}=normalize_depots_for_relocation())
3599+
# We must handle several cases:
3600+
# 1. The depot itself is in a symlink'ed path
3601+
# 2. The files were symlinked into the depot
3602+
# 3. A combination of both
3603+
# The normalization looks up `realpath`. The simplest case is tha
3604+
this_realpath = realpath(path)
3605+
realpath_differs = this_realpath != path
35983606
for depot in depots
3599-
if startswith(path, string(depot, Filesystem.pathsep())) || path == depot
3607+
if isa(depot, Tuple{String, String})
3608+
depotrealpath = depot[2]
3609+
depot = depot[1]
3610+
else
3611+
depotrealpath = depot
3612+
end
3613+
if startswith(this_realpath, string(depotrealpath, Filesystem.pathsep())) || this_realpath == depotrealpath
3614+
# Simplest case: The file's realpath is within the depot's realpath, simply replace and go on
3615+
path = replace(this_realpath, depotrealpath => "@depot"; count=1)
3616+
break
3617+
elseif realpath_differs && startswith(path, string(depot, Filesystem.pathsep())) || path == depot
3618+
# The file's original path was loaded from symlinks within the depot - n.b.: the path may still have `..` components
36003619
path = replace(path, depot => "@depot"; count=1)
36013620
break
36023621
end
@@ -3605,14 +3624,20 @@ function replace_depot_path(path::AbstractString, depots::Vector{String}=normali
36053624
end
36063625

36073626
function normalize_depots_for_relocation()
3608-
depots = String[]
3627+
depots = Union{String, Tuple{String, String}}[]
36093628
sizehint!(depots, length(DEPOT_PATH))
36103629
for d in DEPOT_PATH
36113630
isdir(d) || continue
36123631
if isdirpath(d)
36133632
d = dirname(d)
36143633
end
3615-
push!(depots, abspath(d))
3634+
depotrealpath = realpath(d)
3635+
depotabspath = abspath(d)
3636+
if depotrealpath != depotabspath
3637+
push!(depots, (depotabspath, depotrealpath))
3638+
else
3639+
push!(depots, depotrealpath)
3640+
end
36163641
end
36173642
return depots
36183643
end

base/path.jl

Lines changed: 38 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -373,38 +373,55 @@ julia> joinpath(["/home/myuser", "example.jl"])
373373
joinpath
374374

375375
"""
376-
normpath(path::AbstractString)::String
376+
normpath(path::AbstractString; safe=false)::String
377377
378-
Normalize a path, removing "." and ".." entries and changing "/" to the canonical path separator
379-
for the system.
378+
Normalize a path, removing "." changing "/" to the canonical path separator for the system.
379+
If `safe` is false, also removes ".." entries.
380+
381+
!!! warning
382+
Normalization with `safe==false` may change the meaning of a path if the
383+
directory immediately preceding a ".." entry is a symbolic link.
384+
For example, if `a` is a symbolic link in `a/../b`, then this path will
385+
resolve to `b` in the link target's parent directory. However,
386+
`normpath("a/../b")` will return `b`, which will resolve `b` in the
387+
current working directory.
388+
389+
!!! compat "Julia 1.14"
390+
The `safe` keyword argument was added in Julia 1.14.
380391
381392
# Examples
382393
```jldoctest
383394
julia> normpath("/home/myuser/../example.jl")
384395
"/home/example.jl"
385396
397+
julia> normpath("/home/myuser/../example.jl"; safe=true)
398+
"/home/myuser/../example.jl"
399+
386400
julia> normpath("Documents/Julia") == joinpath("Documents", "Julia")
387401
true
388402
```
389403
"""
390-
function normpath(path::String)
404+
function normpath(path::String; safe=false)
391405
isabs = isabspath(path)
392406
isdir = isdirpath(path)
393407
drive, path = splitdrive(path)
394408
parts = split(path, path_separator_re; keepempty=false)
395409
filter!(!=("."), parts)
396-
while true
397-
clean = true
398-
for j = 1:length(parts)-1
399-
if parts[j] != ".." && parts[j+1] == ".."
400-
deleteat!(parts, j:j+1)
401-
clean = false
402-
break
410+
if !safe
411+
while true
412+
clean = true
413+
for j = 1:length(parts)-1
414+
if parts[j] != ".." && parts[j+1] == ".."
415+
deleteat!(parts, j:j+1)
416+
clean = false
417+
break
418+
end
403419
end
420+
clean && break
404421
end
405-
clean && break
406422
end
407423
if isabs
424+
# N.B.: `..` at the beginning of a path may always be removed
408425
while !isempty(parts) && parts[1] == ".."
409426
popfirst!(parts)
410427
end
@@ -427,13 +444,13 @@ end
427444
Convert a set of paths to a normalized path by joining them together and removing
428445
"." and ".." entries. Equivalent to `normpath(joinpath(path, paths...))`.
429446
"""
430-
normpath(a::AbstractString, b::AbstractString...) = normpath(joinpath(a,b...))
447+
normpath(a::AbstractString, b::AbstractString...; kwargs...) = normpath(joinpath(a,b...); kwargs...)
431448

432449
"""
433-
abspath(path::AbstractString)::String
450+
abspath(path::AbstractString; safe=false)::String
434451
435452
Convert a path to an absolute path by adding the current directory if necessary.
436-
Also normalizes the path as in [`normpath`](@ref).
453+
If `normalize` is true, also normalizes the path as in [`normpath`](@ref).
437454
438455
# Examples
439456
@@ -444,8 +461,11 @@ If you are in a directory called `JuliaExample` and the data you are using is tw
444461
Which gives a path like `"/home/JuliaUser/data/"`.
445462
446463
See also [`joinpath`](@ref), [`pwd`](@ref), [`expanduser`](@ref).
464+
465+
!!! compat "Julia 1.14"
466+
The `safe` keyword argument was added in Julia 1.14.
447467
"""
448-
function abspath(a::String)::String
468+
function abspath(a::String; safe=false)::String
449469
if !isabspath(a)
450470
cwd = pwd()
451471
a_drive, a_nodrive = splitdrive(a)
@@ -456,7 +476,7 @@ function abspath(a::String)::String
456476
a = joinpath(cwd, a)
457477
end
458478
end
459-
return normpath(a)
479+
return normpath(a; safe)
460480
end
461481

462482
"""
@@ -465,7 +485,7 @@ end
465485
Convert a set of paths to an absolute path by joining them together and adding the
466486
current directory if necessary. Equivalent to `abspath(joinpath(path, paths...))`.
467487
"""
468-
abspath(a::AbstractString, b::AbstractString...) = abspath(joinpath(a,b...))
488+
abspath(a::AbstractString, b::AbstractString...; kwargs...) = abspath(joinpath(a,b...); kwargs...)
469489

470490
if Sys.iswindows()
471491

src/precompile.c

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,12 @@ void write_srctext(ios_t *f, jl_array_t *udeps, int64_t srctextpos) {
5959
// because these may not be Julia code (and could be huge)
6060
JL_TYPECHK(write_srctext, deptuple, deptuple);
6161
if (depmod != (jl_value_t*)jl_main_module) {
62-
jl_value_t *abspath = jl_fieldref_noalloc(deptuple, 1); // file abspath
63-
const char *abspathstr = jl_string_data(abspath);
64-
if (!abspathstr[0])
62+
jl_value_t *abspath_or_uuid = jl_fieldref_noalloc(deptuple, 1); // file abspath
63+
const char *abspathstr = jl_string_data(abspath_or_uuid);
64+
if (!abspathstr[0]) {
65+
// UUID or empty string
6566
continue;
67+
}
6668
ios_t *srctp = ios_file(&srctext, abspathstr, 1, 0, 0, 0);
6769
if (!srctp) {
6870
jl_printf(JL_STDERR, "WARNING: could not cache source text for \"%s\".\n",
@@ -72,7 +74,7 @@ void write_srctext(ios_t *f, jl_array_t *udeps, int64_t srctextpos) {
7274

7375
jl_value_t *replace_depot_args[3];
7476
replace_depot_args[0] = replace_depot_func;
75-
replace_depot_args[1] = abspath;
77+
replace_depot_args[1] = abspath_or_uuid;
7678
replace_depot_args[2] = depots;
7779
jl_value_t *depalias = (jl_value_t*)jl_apply(replace_depot_args, 3);
7880

0 commit comments

Comments
 (0)