mirror of https://github.com/jenkinsci/jenkins.git
[JENKINS-73161] Ensure file parameters are retained across Jenkins restarts
Save the file as temporary file and store the filename in the value That way we can recover the file
This commit is contained in:
parent
633b9c0975
commit
d01cedafa3
|
@ -26,9 +26,11 @@ package hudson.model;
|
||||||
|
|
||||||
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
|
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
|
||||||
import hudson.EnvVars;
|
import hudson.EnvVars;
|
||||||
|
import hudson.Extension;
|
||||||
import hudson.FilePath;
|
import hudson.FilePath;
|
||||||
import hudson.Launcher;
|
import hudson.Launcher;
|
||||||
import hudson.Util;
|
import hudson.Util;
|
||||||
|
import hudson.model.queue.QueueListener;
|
||||||
import hudson.tasks.BuildWrapper;
|
import hudson.tasks.BuildWrapper;
|
||||||
import hudson.util.VariableResolver;
|
import hudson.util.VariableResolver;
|
||||||
import java.io.File;
|
import java.io.File;
|
||||||
|
@ -39,12 +41,16 @@ import java.io.UnsupportedEncodingException;
|
||||||
import java.nio.charset.Charset;
|
import java.nio.charset.Charset;
|
||||||
import java.nio.file.Files;
|
import java.nio.file.Files;
|
||||||
import java.nio.file.Path;
|
import java.nio.file.Path;
|
||||||
|
import java.util.List;
|
||||||
|
import java.util.logging.Level;
|
||||||
|
import java.util.logging.Logger;
|
||||||
import java.util.regex.Pattern;
|
import java.util.regex.Pattern;
|
||||||
import jenkins.util.SystemProperties;
|
import jenkins.util.SystemProperties;
|
||||||
import org.apache.commons.fileupload2.core.FileItem;
|
import org.apache.commons.fileupload2.core.FileItem;
|
||||||
import org.apache.commons.fileupload2.core.FileItemFactory;
|
import org.apache.commons.fileupload2.core.FileItemFactory;
|
||||||
import org.apache.commons.fileupload2.core.FileItemHeaders;
|
import org.apache.commons.fileupload2.core.FileItemHeaders;
|
||||||
import org.apache.commons.fileupload2.core.FileItemHeadersProvider;
|
import org.apache.commons.fileupload2.core.FileItemHeadersProvider;
|
||||||
|
import org.apache.commons.io.FileUtils;
|
||||||
import org.apache.commons.io.FilenameUtils;
|
import org.apache.commons.io.FilenameUtils;
|
||||||
import org.kohsuke.accmod.Restricted;
|
import org.kohsuke.accmod.Restricted;
|
||||||
import org.kohsuke.accmod.restrictions.NoExternalUse;
|
import org.kohsuke.accmod.restrictions.NoExternalUse;
|
||||||
|
@ -58,6 +64,9 @@ import org.kohsuke.stapler.StaplerResponse2;
|
||||||
* @author Kohsuke Kawaguchi
|
* @author Kohsuke Kawaguchi
|
||||||
*/
|
*/
|
||||||
public class FileParameterValue extends ParameterValue {
|
public class FileParameterValue extends ParameterValue {
|
||||||
|
|
||||||
|
private static final Logger LOGGER = Logger.getLogger(FileParameterValue.class.getName());
|
||||||
|
|
||||||
private static final String FOLDER_NAME = "fileParameters";
|
private static final String FOLDER_NAME = "fileParameters";
|
||||||
private static final Pattern PROHIBITED_DOUBLE_DOT = Pattern.compile(".*[\\\\/]\\.\\.[\\\\/].*");
|
private static final Pattern PROHIBITED_DOUBLE_DOT = Pattern.compile(".*[\\\\/]\\.\\.[\\\\/].*");
|
||||||
private static final long serialVersionUID = -143427023159076073L;
|
private static final long serialVersionUID = -143427023159076073L;
|
||||||
|
@ -71,13 +80,16 @@ public class FileParameterValue extends ParameterValue {
|
||||||
public static /* Script Console modifiable */ boolean ALLOW_FOLDER_TRAVERSAL_OUTSIDE_WORKSPACE =
|
public static /* Script Console modifiable */ boolean ALLOW_FOLDER_TRAVERSAL_OUTSIDE_WORKSPACE =
|
||||||
SystemProperties.getBoolean(FileParameterValue.class.getName() + ".allowFolderTraversalOutsideWorkspace");
|
SystemProperties.getBoolean(FileParameterValue.class.getName() + ".allowFolderTraversalOutsideWorkspace");
|
||||||
|
|
||||||
private final transient FileItem file;
|
private transient FileItem file;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The name of the originally uploaded file.
|
* The name of the originally uploaded file.
|
||||||
*/
|
*/
|
||||||
private final String originalFileName;
|
private final String originalFileName;
|
||||||
|
|
||||||
|
|
||||||
|
private final String tmpFileName;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Overrides the location in the build to place this file. Initially set to {@link #getName()}
|
* Overrides the location in the build to place this file. Initially set to {@link #getName()}
|
||||||
* The location could be directly the filename or also a hierarchical path.
|
* The location could be directly the filename or also a hierarchical path.
|
||||||
|
@ -106,6 +118,14 @@ public class FileParameterValue extends ParameterValue {
|
||||||
|
|
||||||
protected FileParameterValue(String name, FileItem file, String originalFileName) {
|
protected FileParameterValue(String name, FileItem file, String originalFileName) {
|
||||||
super(name);
|
super(name);
|
||||||
|
try {
|
||||||
|
File tmp = Files.createTempFile("jenkins", "parameter").toFile();
|
||||||
|
FileUtils.copyInputStreamToFile(file.getInputStream(), tmp);
|
||||||
|
tmpFileName = tmp.getAbsolutePath();
|
||||||
|
} catch (IOException e) {
|
||||||
|
throw new RuntimeException(e);
|
||||||
|
}
|
||||||
|
|
||||||
this.file = file;
|
this.file = file;
|
||||||
this.originalFileName = originalFileName;
|
this.originalFileName = originalFileName;
|
||||||
setLocation(name);
|
setLocation(name);
|
||||||
|
@ -149,7 +169,17 @@ public class FileParameterValue extends ParameterValue {
|
||||||
return originalFileName;
|
return originalFileName;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private void createFile() {
|
||||||
|
if (file == null && tmpFileName != null) {
|
||||||
|
File tmp = new File(tmpFileName);
|
||||||
|
if (tmp.exists()) {
|
||||||
|
file = new FileItemImpl2(tmp);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
public FileItem getFile2() {
|
public FileItem getFile2() {
|
||||||
|
createFile();
|
||||||
return file;
|
return file;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -163,11 +193,12 @@ public class FileParameterValue extends ParameterValue {
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public BuildWrapper createBuildWrapper(AbstractBuild<?, ?> build) {
|
public BuildWrapper createBuildWrapper(AbstractBuild<?, ?> build) {
|
||||||
|
createFile();
|
||||||
return new BuildWrapper() {
|
return new BuildWrapper() {
|
||||||
@SuppressFBWarnings(value = "NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE", justification = "TODO needs triage")
|
@SuppressFBWarnings(value = "NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE", justification = "TODO needs triage")
|
||||||
@Override
|
@Override
|
||||||
public Environment setUp(AbstractBuild build, Launcher launcher, BuildListener listener) throws IOException, InterruptedException {
|
public Environment setUp(AbstractBuild build, Launcher launcher, BuildListener listener) throws IOException, InterruptedException {
|
||||||
if (location != null && !location.isEmpty() && file.getName() != null && !file.getName().isEmpty()) {
|
if (location != null && !location.isBlank() && file != null && file.getName() != null && !file.getName().isBlank()) {
|
||||||
listener.getLogger().println("Copying file to " + location);
|
listener.getLogger().println("Copying file to " + location);
|
||||||
FilePath ws = build.getWorkspace();
|
FilePath ws = build.getWorkspace();
|
||||||
if (ws == null) {
|
if (ws == null) {
|
||||||
|
@ -186,6 +217,14 @@ public class FileParameterValue extends ParameterValue {
|
||||||
locationFilePath.delete();
|
locationFilePath.delete();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
File tmp = new File(tmpFileName);
|
||||||
|
try {
|
||||||
|
Files.deleteIfExists(tmp.toPath());
|
||||||
|
} catch (IOException e) {
|
||||||
|
LOGGER.log(Level.WARNING, "Unable to delete temporary file {0} for parameter {1} of job {2}",
|
||||||
|
new Object[]{tmp.getAbsolutePath(), getName(), build.getParent().getName()});
|
||||||
|
}
|
||||||
|
|
||||||
locationFilePath.copyFrom(file);
|
locationFilePath.copyFrom(file);
|
||||||
locationFilePath.copyTo(new FilePath(getLocationUnderBuild(build)));
|
locationFilePath.copyTo(new FilePath(getLocationUnderBuild(build)));
|
||||||
}
|
}
|
||||||
|
@ -257,6 +296,33 @@ public class FileParameterValue extends ParameterValue {
|
||||||
return new File(build.getRootDir(), FOLDER_NAME);
|
return new File(build.getRootDir(), FOLDER_NAME);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Extension
|
||||||
|
public static class CancelledQueueListener extends QueueListener {
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public void onLeft(Queue.LeftItem li) {
|
||||||
|
if (li.isCancelled()) {
|
||||||
|
List<ParametersAction> actions = li.getActions(ParametersAction.class);
|
||||||
|
actions.forEach(a -> {
|
||||||
|
a.getAllParameters().stream()
|
||||||
|
.filter(p -> p instanceof FileParameterValue)
|
||||||
|
.map(p -> (FileParameterValue) p)
|
||||||
|
.forEach(p -> {
|
||||||
|
if (p.tmpFileName != null) {
|
||||||
|
File tmp = new File(p.tmpFileName);
|
||||||
|
try {
|
||||||
|
Files.deleteIfExists(tmp.toPath());
|
||||||
|
} catch (IOException e) {
|
||||||
|
LOGGER.log(Level.WARNING, "Unable to delete temporary file {0} for parameter {1} of task {2}",
|
||||||
|
new Object[]{tmp.getAbsolutePath(), p.getName(), li.task.getName()});
|
||||||
|
}
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Default implementation from {@link File}.
|
* Default implementation from {@link File}.
|
||||||
*
|
*
|
||||||
|
|
|
@ -5,21 +5,34 @@ import static org.hamcrest.Matchers.containsString;
|
||||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||||
|
|
||||||
|
import hudson.ExtensionList;
|
||||||
import hudson.Functions;
|
import hudson.Functions;
|
||||||
|
import hudson.model.queue.CauseOfBlockage;
|
||||||
|
import hudson.model.queue.QueueTaskDispatcher;
|
||||||
import hudson.tasks.BatchFile;
|
import hudson.tasks.BatchFile;
|
||||||
import hudson.tasks.Shell;
|
import hudson.tasks.Shell;
|
||||||
import java.io.File;
|
import java.io.File;
|
||||||
|
import java.net.URL;
|
||||||
import java.nio.charset.StandardCharsets;
|
import java.nio.charset.StandardCharsets;
|
||||||
import java.nio.file.Files;
|
import java.nio.file.Files;
|
||||||
import java.nio.file.Path;
|
import java.nio.file.Path;
|
||||||
|
import java.util.Collections;
|
||||||
|
import jenkins.model.Jenkins;
|
||||||
|
import org.apache.commons.io.FileUtils;
|
||||||
|
import org.htmlunit.FormEncodingType;
|
||||||
|
import org.htmlunit.HttpMethod;
|
||||||
|
import org.htmlunit.WebRequest;
|
||||||
import org.htmlunit.html.HtmlForm;
|
import org.htmlunit.html.HtmlForm;
|
||||||
import org.htmlunit.html.HtmlInput;
|
import org.htmlunit.html.HtmlInput;
|
||||||
import org.htmlunit.html.HtmlPage;
|
import org.htmlunit.html.HtmlPage;
|
||||||
|
import org.htmlunit.util.KeyDataPair;
|
||||||
import org.junit.jupiter.api.Test;
|
import org.junit.jupiter.api.Test;
|
||||||
import org.junit.jupiter.api.extension.RegisterExtension;
|
import org.junit.jupiter.api.extension.RegisterExtension;
|
||||||
import org.junit.jupiter.api.io.TempDir;
|
import org.junit.jupiter.api.io.TempDir;
|
||||||
import org.jvnet.hudson.test.Issue;
|
import org.jvnet.hudson.test.Issue;
|
||||||
import org.jvnet.hudson.test.JenkinsRule;
|
import org.jvnet.hudson.test.JenkinsRule;
|
||||||
|
import org.jvnet.hudson.test.MockAuthorizationStrategy;
|
||||||
|
import org.jvnet.hudson.test.TestExtension;
|
||||||
import org.jvnet.hudson.test.junit.jupiter.JenkinsSessionExtension;
|
import org.jvnet.hudson.test.junit.jupiter.JenkinsSessionExtension;
|
||||||
|
|
||||||
class FileParameterValuePersistenceTest {
|
class FileParameterValuePersistenceTest {
|
||||||
|
@ -78,4 +91,38 @@ class FileParameterValuePersistenceTest {
|
||||||
assertThat(page.getWebResponse().getContentAsString(), containsString(FILENAME));
|
assertThat(page.getWebResponse().getContentAsString(), containsString(FILENAME));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Issue("JENKINS-73161")
|
||||||
|
@Test
|
||||||
|
void fileParameterValueIsRetained() throws Throwable {
|
||||||
|
sessions.then(r -> {
|
||||||
|
r.jenkins.setSecurityRealm(r.createDummySecurityRealm());
|
||||||
|
r.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy().grant(Jenkins.ADMINISTER).everywhere().to("admin"));
|
||||||
|
FreeStyleProject p = r.createFreeStyleProject("p");
|
||||||
|
p.addProperty(new ParametersDefinitionProperty(new FileParameterDefinition("FILE")));
|
||||||
|
WebRequest req = new WebRequest(new URL(r.getURL() + "job/p/buildWithParameters"), HttpMethod.POST);
|
||||||
|
File f = File.createTempFile("junit", null, tmp);
|
||||||
|
FileUtils.write(f, "uploaded content here", "UTF-8");
|
||||||
|
req.setEncodingType(FormEncodingType.MULTIPART);
|
||||||
|
req.setRequestParameters(Collections.singletonList(new KeyDataPair("FILE", f, "myfile.txt", "text/plain", "UTF-8")));
|
||||||
|
r.createWebClient().withBasicApiToken("admin").getPage(req);
|
||||||
|
});
|
||||||
|
sessions.then(r -> {
|
||||||
|
ExtensionList.lookupSingleton(Block.class).ready = true;
|
||||||
|
FreeStyleProject p = r.jenkins.getItemByFullName("p", FreeStyleProject.class);
|
||||||
|
r.waitUntilNoActivity();
|
||||||
|
FreeStyleBuild b = p.getBuildByNumber(1);
|
||||||
|
r.assertBuildStatusSuccess(b);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
@TestExtension("fileParameterValueIsRetained")
|
||||||
|
public static final class Block extends QueueTaskDispatcher {
|
||||||
|
private boolean ready;
|
||||||
|
@Override
|
||||||
|
public CauseOfBlockage canTake(Node node, Queue.BuildableItem item) {
|
||||||
|
return ready ? null : new CauseOfBlockage.BecauseNodeIsBusy(node);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue