Skip to content

Path related warnings #7075

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
XingY merged 7 commits into from
Sep 29, 2025
Merged

Path related warnings #7075

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
407fe75
Path related warnings
XingY Sep 25, 2025
57f5160
Path related warnings
XingY Sep 26, 2025
a5476a1
Fix import using path
XingY Sep 26, 2025
d6bd718
Merge branch 'develop' into fb_filePathWarning
XingY Sep 26, 2025
8ff2a67
File path warning
XingY Sep 26, 2025
c125e9a
don't allow non dav path for import action.
XingY Sep 27, 2025
7e92439
allow checking against pipeline root
XingY Sep 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
import org.labkey.api.study.StudyService;
import org.labkey.api.study.publish.StudyPublishService;
import org.labkey.api.study.assay.ParticipantVisitResolverType;
import org.labkey.api.util.FileUtil;
import org.labkey.api.util.GUID;
import org.labkey.api.view.ActionURL;
import org.labkey.api.view.NotFoundException;
Expand Down Expand Up @@ -360,7 +361,7 @@ public Map<DomainProperty, FileLike> getAdditionalPostedFiles(List<? extends Dom
String previousFileName = request.getParameter(fileParam);
if (null != previousFileName)
{
previousFile = new File(getAssayDirectory(getContainer(), null).getAbsolutePath(), previousFileName);
previousFile = FileUtil.appendName(getAssayDirectory(getContainer(), null), previousFileName);

MultipartFile multiFile = ((MultipartHttpServletRequest)request).getFileMap().get(UploadWizardAction.getInputName(fileParameters.get(fileParam)));

Expand Down
9 changes: 5 additions & 4 deletions api/src/org/labkey/api/attachments/FileAttachmentFile.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.labkey.api.attachments;

import org.apache.logging.log4j.Logger;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.labkey.api.util.PageFlowUtil;
import org.labkey.api.util.logging.LogHelper;
Expand All @@ -40,22 +41,22 @@ public class FileAttachmentFile implements AttachmentFile

private InputStream _in;

public FileAttachmentFile(File file)
public FileAttachmentFile(@NotNull File file)
{
this(file, null);
}

public FileAttachmentFile(FileLike file)
public FileAttachmentFile(@NotNull FileLike file)
{
this(file.toNioPathForRead().toFile(), null);
}

public FileAttachmentFile(FileLike file, @Nullable String originalName)
public FileAttachmentFile(@NotNull FileLike file, @Nullable String originalName)
{
this(file.toNioPathForRead().toFile(), originalName);
}

public FileAttachmentFile(File file, @Nullable String originalName)
public FileAttachmentFile(@NotNull File file, @Nullable String originalName)
{
_file = file;
_filename = null != originalName ? originalName : file.getName();
Expand Down
11 changes: 11 additions & 0 deletions api/src/org/labkey/api/exp/AbstractFileXarSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
import org.fhcrc.cpas.exp.xml.ExperimentArchiveDocument;
import org.jetbrains.annotations.Nullable;
import org.labkey.api.data.Container;
import org.labkey.api.pipeline.PipeRoot;
import org.labkey.api.pipeline.PipelineJob;
import org.labkey.api.pipeline.PipelineService;
import org.labkey.api.security.User;
import org.labkey.api.util.FileUtil;
import org.labkey.api.util.NetworkDrive;
Expand All @@ -30,6 +32,7 @@
import java.io.InputStream;
import java.net.URI;
import java.nio.file.Files;
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
import java.util.Map;

Expand Down Expand Up @@ -113,6 +116,14 @@ public String canonicalizeDataFileURL(String dataFileURL)
if (!uri.isAbsolute())
{
Path path = xarDirectory.resolve(dataFileURL);
if (!path.normalize().startsWith(xarDirectory.normalize()))
{
// check alternative - relative to container pipeline root
PipeRoot root = PipelineService.get().findPipelineRoot(getXarContext().getContainer());
boolean valid = root != null && root.isUnderRoot(path);
if (!valid)
throw new InvalidPathException(dataFileURL, "Path to parent not allowed");
}
String result = _dataFileURLs.get(FileUtil.getAbsolutePath(getXarContext().getContainer(), path));
if (result != null)
{
Expand Down
2 changes: 1 addition & 1 deletion api/src/org/labkey/api/files/FileContentService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ enum ContentType {
* relative to the first parent container with an override
*/
File getDefaultRoot(Container c, boolean createDir);
Path getDefaultRootPath(Container c, boolean createDir);
Path getDefaultRootPath(@NotNull Container c, boolean createDir);

class DefaultRootInfo
{
Expand Down
3 changes: 2 additions & 1 deletion api/src/org/labkey/api/pipeline/AnalyzeForm.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.labkey.api.pipeline.file.AbstractFileAnalysisProtocol;
import org.labkey.api.security.User;
import org.labkey.api.util.FileType;
import org.labkey.api.util.FileUtil;

import java.nio.file.Path;

Expand Down Expand Up @@ -96,7 +97,7 @@ private String initStatusFile(AbstractFileAnalysisProtocol protocol, Path dirDat
}
else if (fileInputName != null)
{
Path fileInput = dirData.resolve(fileInputName);
Path fileInput = FileUtil.appendName(dirData, fileInputName);
FileType ft = protocol.findInputType(fileInput);
if (ft != null)
fileStatus = PipelineJob.FT_LOG.newFile(dirAnalysis, ft.getBaseName(fileInput));
Expand Down
4 changes: 2 additions & 2 deletions api/src/org/labkey/api/pipeline/LocalDirectory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ public Path determineLogFile()
{
try
{
Path remoteLogFilePath = _remoteDir.resolve(_logFile.getFileName().toString());
Path remoteLogFilePath = FileUtil.appendName(_remoteDir, _logFile.getFileName().toString());
if (Files.exists(remoteLogFilePath))
{
Files.copy(remoteLogFilePath, _logFile);
Expand Down Expand Up @@ -307,6 +307,6 @@ public Path getRemoteLogFilePath()
{
if (_remoteDir == null)
return _logFile;
return _remoteDir.resolve(_logFile.getFileName().toString());
return FileUtil.appendName(_remoteDir, _logFile.getFileName().toString());
}
}
20 changes: 0 additions & 20 deletions api/src/org/labkey/api/query/AbstractQueryImportAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,26 +472,6 @@ else if (StringUtils.isNotBlank(path))
file = resource.getFileStream(user);
originalName = resource.getName();
}
else
{
// Resolve file under pipeline root
PipeRoot root = PipelineService.get().findPipelineRoot(getContainer());
if (root != null)
{
// Attempt absolute path first, then relative path from pipeline root
File f = new File(path);
if (!root.isUnderRoot(f))
f = root.resolvePath(path);

if (NetworkDrive.exists(f) && root.isUnderRoot(f) && root.hasPermission(getContainer(), getUser(), ReadPermission.class))
{
hasPostData = true;
loader = DataLoader.get().createLoader(f, null, _hasColumnHeaders, null, null);
file = new FileAttachmentFile(dataFile, f.getName());
originalName = f.getName();
}
}
}

if (!hasPostData)
{
Expand Down
6 changes: 3 additions & 3 deletions api/src/org/labkey/api/util/FileType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,15 +244,15 @@ private String tryName(Path parentDir, String name)
{
if (_supportGZ.booleanValue()) // TPP treats xml.gz as a native format
{ // in the case of existing files, non-gz copy wins if present
Path f = parentDir!=null ? parentDir.resolve(name) : Path.of(name);
Path f = parentDir!=null ? FileUtil.appendName(parentDir, name) : Path.of(name);
if (!NetworkDrive.exists(f))
{ // non-gz copy doesn't exist - how about .gz version?
String gzname = name + ".gz";
if (_preferGZ.booleanValue())
{ // we like .gz for new filenames, so don't care if exists
return gzname;
}
f = parentDir!=null ? parentDir.resolve(gzname) : Path.of(gzname);
f = parentDir!=null ? FileUtil.appendName(parentDir, gzname) : Path.of(gzname);
if (NetworkDrive.exists(f))
{ // we don't prefer .gz, but we support it if it exists
return gzname;
Expand Down Expand Up @@ -464,7 +464,7 @@ else if (_supportGZ.booleanValue()) // TPP treats .xml.gz as a native read forma

public File newFile(File parent, String basename)
{
return new File(parent, getName(parent, basename));
return FileUtil.appendName(parent, getName(parent, basename));
Copy link

Copilot AI Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The return type is File but FileUtil.appendName(File, String) likely returns a Path or different type. This will cause a compilation error.

Suggested change
return FileUtil.appendName(parent, getName(parent, basename));
return FileUtil.appendName(parent, getName(parent, basename)).toFile();

Copilot uses AI. Check for mistakes.
}

public Path newFile(Path parent, String basename)
Expand Down
4 changes: 2 additions & 2 deletions experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6758,15 +6758,15 @@ public Object execute(Object o, BindException errors) throws Exception
{
userDirName = GUEST_DIRECTORY_NAME;
}
Path userDir = uploadDir.resolve(userDirName);
Path userDir = FileUtil.appendName(uploadDir, userDirName);
FileUtil.createDirectories(userDir);
if (!Files.isDirectory(userDir))
{
errors.reject(ERROR_MSG, "Unable to create an 'UploadedXARs/" + userDirName + "' directory under the pipeline root");
return false;
}

Path xarFile = userDir.resolve(formFile.getOriginalFilename());
Path xarFile = FileUtil.appendName(userDir, formFile.getOriginalFilename());

// As this is multi-part will need to use finally to close, to prevent a stream closure exception
try (OutputStream out = new BufferedOutputStream(Files.newOutputStream(xarFile)))
Expand Down
4 changes: 2 additions & 2 deletions experiment/src/org/labkey/experiment/pipeline/ExperimentPipelineProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,12 @@ public void initSystemDirectory(Path rootDir, Path systemDir)

public void locateSystemDir(Path systemDir, String name)
{
Path path = systemDir.resolve(name);
Path path = FileUtil.appendName(systemDir, name);
if (Files.exists(path))
{
try
{
Path dest = getExperimentDirectory(systemDir, name).resolve(FileUtil.getFileName(path));
Path dest = FileUtil.appendName(getExperimentDirectory(systemDir, name), FileUtil.getFileName(path));
Files.move(path, dest);
}
catch (IOException e)
Expand Down
8 changes: 4 additions & 4 deletions filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ public File getDefaultRoot(Container c, boolean createDir)
}

@Override
public java.nio.file.Path getDefaultRootPath(Container c, boolean createDir)
public java.nio.file.Path getDefaultRootPath(@NotNull Container c, boolean createDir)
{
Container firstOverride = getFirstAncestorWithOverride(c);

Expand All @@ -325,7 +325,7 @@ public java.nio.file.Path getDefaultRootPath(Container c, boolean createDir)
parentRoot = getFileRootPath(firstOverride);
}

if (parentRoot != null && c != null && firstOverride != null)
if (parentRoot != null && firstOverride != null)
{
java.nio.file.Path fileRootPath;
if (FileUtil.hasCloudScheme(parentRoot))
Expand All @@ -336,7 +336,7 @@ public java.nio.file.Path getDefaultRootPath(Container c, boolean createDir)
else
{
// For local, the path may be several directories deep (since it matches the LK folder path), so we should create the directories for that path
fileRootPath = new File(parentRoot.toFile(), getRelativePath(c, firstOverride)).toPath();
fileRootPath = FileUtil.appendPath(parentRoot.toFile(), Path.parse(getRelativePath(c, firstOverride))).toPath();
Copy link

Copilot AI Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code converts Path to File, then back to Path unnecessarily. Should use FileUtil.appendPath directly with Path objects or use a consistent approach.

Suggested change
fileRootPath = FileUtil.appendPath(parentRoot.toFile(), Path.parse(getRelativePath(c, firstOverride))).toPath();
fileRootPath = FileUtil.appendPath(parentRoot, Path.parse(getRelativePath(c, firstOverride)));

Copilot uses AI. Check for mistakes.

try
{
Expand Down Expand Up @@ -395,7 +395,7 @@ public FileRoot getDefaultFileRoot(Container c)
return null;
}

private String getRelativePath(Container c, Container ancestor)
private @NotNull String getRelativePath(Container c, Container ancestor)
{
return c.getPath().replaceAll("^" + Pattern.quote(ancestor.getPath()), "");
}
Expand Down
2 changes: 1 addition & 1 deletion filecontent/src/org/labkey/filecontent/FileSystemAttachmentParent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ else if (isRelative())

if (_contentType != null && !svc.isCloudRoot(container)) // don't need @files in cloud
{
Path root = dir.resolve(svc.getFolderName(_contentType));
Path root = FileUtil.appendName(dir, svc.getFolderName(_contentType));

// Issue 49963: avoid FileAlreadyExistsExceptions on certain file systems
if (!Files.exists(root) && Files.isWritable(dir) && create)
Expand Down
2 changes: 1 addition & 1 deletion pipeline/src/org/labkey/pipeline/api/PipelineServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -986,7 +986,7 @@ public String startFileAnalysis(AnalyzeForm form, @Nullable Map<String, String>
{
try
{
Files.move(inputFile, dirData.resolve(inputFile.getFileName().toString()));
Files.move(inputFile, FileUtil.appendName(dirData, inputFile.getFileName().toString()));
}
catch (IOException e)
{
Expand Down
2 changes: 1 addition & 1 deletion study/src/org/labkey/study/assay/StudyPublishManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -958,7 +958,7 @@ public UploadLog saveUploadData(User user, Dataset dsd, FileStream tsv, String f
String extra = id++ == 0 ? "" : String.valueOf(id);
String fileName = dsd.getStudy().getLabel() + "-" + dsd.getLabel() + "-" + dateString + extra + "." + extension;
fileName = fileName.replace('\\', '_').replace('/', '_').replace(':', '_');
file = dir.resolve(fileName);
file = FileUtil.appendName(dir, fileName);
}
while (Files.exists(file));

Expand Down
2 changes: 1 addition & 1 deletion study/src/org/labkey/study/importer/StudyImportContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ private Path getStudyFile(VirtualFile root, VirtualFile dir, String name) throws
{
Path rootFile = FileUtil.stringToPath(getContainer(), root.getLocation());
Path dirFile = FileUtil.stringToPath(getContainer(), dir.getLocation());
Path file = dirFile.resolve(name);
Path file = FileUtil.appendName(dirFile, name);
String source = "study.xml";

if (!Files.exists(file))
Expand Down