diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java index cdc057ef9c54..1da338d6e549 100644 --- a/core/src/main/java/hudson/FilePath.java +++ b/core/src/main/java/hudson/FilePath.java @@ -28,6 +28,7 @@ import static hudson.Util.fileToPath; import static hudson.Util.fixEmpty; +import static hudson.Util.fixEmptyAndTrim; import com.google.common.annotations.VisibleForTesting; import com.jcraft.jzlib.GZIPInputStream; @@ -962,7 +963,7 @@ public Void invoke(File dir, VirtualChannel channel) throws IOException { * * * @param archive - * The resource that represents the tgz/zip file. This URL must support the {@code Last-Modified} header. + * The resource that represents the tgz/zip file. This URL must support the {@code Last-Modified} header or the {@code ETag} header. * (For example, you could use {@link ClassLoader#getResource}.) * @param listener * If non-null, a message will be printed to this listener once this method decides to @@ -984,12 +985,18 @@ private boolean installIfNecessaryFrom(@NonNull URL archive, @NonNull TaskListen try { FilePath timestamp = this.child(".timestamp"); long lastModified = timestamp.lastModified(); + // https://httpwg.org/specs/rfc9110.html#field.etag is the ETag specification + // Read previously stored ETag if timestamp is available + String etag = timestamp.exists() ? fixEmptyAndTrim(timestamp.readToString()) : null; URLConnection con; try { con = ProxyConfiguration.open(archive); if (lastModified != 0) { con.setIfModifiedSince(lastModified); } + if (etag != null) { + con.setRequestProperty("If-None-Match", etag); + } con.connect(); } catch (IOException x) { if (this.exists()) { @@ -1016,7 +1023,7 @@ private boolean installIfNecessaryFrom(@NonNull URL archive, @NonNull TaskListen return false; } } - if (lastModified != 0) { + if (lastModified != 0 || etag != null) { if (responseCode == HttpURLConnection.HTTP_NOT_MODIFIED) { return false; } else if (responseCode != HttpURLConnection.HTTP_OK) { @@ -1027,8 +1034,12 @@ private boolean installIfNecessaryFrom(@NonNull URL archive, @NonNull TaskListen } long sourceTimestamp = con.getLastModified(); + String resultEtag = fixEmptyAndTrim(con.getHeaderField("ETag")); if (this.exists()) { + if (equalETags(etag, resultEtag)) { + return false; // already up to date + } if (lastModified != 0 && sourceTimestamp == lastModified) return false; // already up to date this.deleteContents(); @@ -1042,6 +1053,10 @@ private boolean installIfNecessaryFrom(@NonNull URL archive, @NonNull TaskListen // First try to download from the agent machine. try { act(new Unpack(archive)); + if (resultEtag != null && !equalETags(etag, resultEtag)) { + /* Store the ETag value in the timestamp file for later use */ + timestamp.write(resultEtag, "UTF-8"); + } timestamp.touch(sourceTimestamp); return true; } catch (IOException x) { @@ -1061,6 +1076,10 @@ private boolean installIfNecessaryFrom(@NonNull URL archive, @NonNull TaskListen throw new IOException(String.format("Failed to unpack %s (%d bytes read of total %d)", archive, cis.getByteCount(), con.getContentLength()), e); } + if (resultEtag != null && !equalETags(etag, resultEtag)) { + /* Store the ETag value in the timestamp file for later use */ + timestamp.write(resultEtag, "UTF-8"); + } timestamp.touch(sourceTimestamp); return true; } catch (IOException e) { @@ -1068,6 +1087,25 @@ private boolean installIfNecessaryFrom(@NonNull URL archive, @NonNull TaskListen } } + /* Return true if etag1 equals etag2 as defined by the etag specification + https://httpwg.org/specs/rfc9110.html#field.etag + */ + private boolean equalETags(String etag1, String etag2) { + if (etag1 == null || etag2 == null) { + return false; + } + if (etag1.equals(etag2)) { + return true; + } + /* Weak tags are identified by leading characters "W/" as a marker */ + /* Weak tag marker must not be considered in tag comparison. + This implements the weak comparison in the specification at + https://httpwg.org/specs/rfc9110.html#field.etag */ + String opaqueTag1 = etag1.startsWith("W/") ? etag1.substring(2) : etag1; + String opaqueTag2 = etag2.startsWith("W/") ? etag2.substring(2) : etag2; + return opaqueTag1.equals(opaqueTag2); + } + // this reads from arbitrary URL private static final class Unpack extends MasterToSlaveFileCallable { private final URL archive; diff --git a/core/src/main/java/hudson/slaves/Cloud.java b/core/src/main/java/hudson/slaves/Cloud.java index 6ec3ed3b489d..76cf075c2634 100644 --- a/core/src/main/java/hudson/slaves/Cloud.java +++ b/core/src/main/java/hudson/slaves/Cloud.java @@ -331,7 +331,8 @@ public HttpResponse doConfigSubmit(StaplerRequest req, StaplerResponse rsp) thro j.clouds.replace(this, result); j.save(); // take the user back to the cloud top page. - return FormApply.success("."); + return FormApply.success("../" + result.name + '/'); + } public Cloud reconfigure(@NonNull final StaplerRequest req, JSONObject form) throws Descriptor.FormException { diff --git a/core/src/main/java/jenkins/agents/CloudSet.java b/core/src/main/java/jenkins/agents/CloudSet.java index 00a27b04693d..e69a3f9add3c 100644 --- a/core/src/main/java/jenkins/agents/CloudSet.java +++ b/core/src/main/java/jenkins/agents/CloudSet.java @@ -231,7 +231,6 @@ private void handleNewCloudPage(Descriptor descriptor, String name, Stapl checkName(name); JSONObject formData = req.getSubmittedForm(); formData.put("name", name); - formData.put("cloudName", name); // ec2 uses that field name formData.remove("mode"); // Cloud descriptors won't have this field. req.setAttribute("instance", formData); req.setAttribute("descriptor", descriptor); diff --git a/core/src/main/resources/hudson/slaves/Cloud/configure.jelly b/core/src/main/resources/hudson/slaves/Cloud/configure.jelly index 3e122734d601..f7479a743176 100644 --- a/core/src/main/resources/hudson/slaves/Cloud/configure.jelly +++ b/core/src/main/resources/hudson/slaves/Cloud/configure.jelly @@ -40,7 +40,6 @@ THE SOFTWARE. - diff --git a/core/src/test/java/hudson/FilePathTest.java b/core/src/test/java/hudson/FilePathTest.java index fe152a25ba49..54d37c2b3dcd 100644 --- a/core/src/test/java/hudson/FilePathTest.java +++ b/core/src/test/java/hudson/FilePathTest.java @@ -670,6 +670,77 @@ private static void assertValidateAntFileMask(String expected, FilePath d, Strin assertTrue(d.installIfNecessaryFrom(url, null, message)); } + @Issue("JENKINS-72469") + @Test public void installIfNecessaryWithoutLastModifiedStrongValidator() throws Exception { + String strongValidator = "\"An-ETag-strong-validator\""; + installIfNecessaryWithoutLastModified(strongValidator); + } + + @Issue("JENKINS-72469") + @Test public void installIfNecessaryWithoutLastModifiedStrongValidatorNoQuotes() throws Exception { + // This ETag is a violation of the spec at https://httpwg.org/specs/rfc9110.html#field.etag + // However, better safe to handle without quotes as well, just in case + String strongValidator = "An-ETag-strong-validator-without-quotes"; + installIfNecessaryWithoutLastModified(strongValidator); + } + + @Issue("JENKINS-72469") + @Test public void installIfNecessaryWithoutLastModifiedWeakValidator() throws Exception { + String weakValidator = "W/\"An-ETag-weak-validator\""; + installIfNecessaryWithoutLastModified(weakValidator); + } + + @Issue("JENKINS-72469") + @Test public void installIfNecessaryWithoutLastModifiedStrongAndWeakValidators() throws Exception { + String strongValidator = "\"An-ETag-validator\""; + String weakValidator = "W/" + strongValidator; + installIfNecessaryWithoutLastModified(strongValidator, weakValidator); + } + + @Issue("JENKINS-72469") + @Test public void installIfNecessaryWithoutLastModifiedWeakAndStrongValidators() throws Exception { + String strongValidator = "\"An-ETag-validator\""; + String weakValidator = "W/" + strongValidator; + installIfNecessaryWithoutLastModified(weakValidator, strongValidator); + } + + private void installIfNecessaryWithoutLastModified(String validator) throws Exception { + installIfNecessaryWithoutLastModified(validator, validator); + } + + private void installIfNecessaryWithoutLastModified(String validator, String alternateValidator) throws Exception { + final HttpURLConnection con = mock(HttpURLConnection.class); + // getLastModified == 0 when last-modified header is not returned + when(con.getLastModified()).thenReturn(0L); + // An Etag is provided by Azul CDN without last-modified header + when(con.getHeaderField("ETag")).thenReturn(validator); + when(con.getInputStream()).thenReturn(someZippedContent()); + + final URL url = someUrlToZipFile(con); + + File tmp = temp.getRoot(); + final FilePath d = new FilePath(tmp); + + /* Initial download expected to occur */ + assertTrue(d.installIfNecessaryFrom(url, null, "message if failed first download")); + + /* Timestamp last modified == 0 means the header was not provided */ + assertThat(d.child(".timestamp").lastModified(), is(0L)); + + /* Second download should not occur if JENKINS-72469 is fixed and NOT_MODIFIED is returned */ + when(con.getResponseCode()).thenReturn(HttpURLConnection.HTTP_NOT_MODIFIED); + when(con.getInputStream()).thenReturn(someZippedContent()); + when(con.getHeaderField("ETag")).thenReturn(alternateValidator); + assertFalse(d.installIfNecessaryFrom(url, null, "message if failed second download")); + + /* Third download should not occur if JENKINS-72469 is fixed and OK is returned with matching ETag */ + /* Unexpected to receive an OK and a matching ETag from a real web server, but check for safety */ + when(con.getResponseCode()).thenReturn(HttpURLConnection.HTTP_OK); + when(con.getInputStream()).thenReturn(someZippedContent()); + when(con.getHeaderField("ETag")).thenReturn(alternateValidator); + assertFalse(d.installIfNecessaryFrom(url, null, "message if failed third download")); + } + private URL someUrlToZipFile(final URLConnection con) throws IOException { final URLStreamHandler urlHandler = new URLStreamHandler() { diff --git a/test/src/test/java/hudson/tasks/MavenTest.java b/test/src/test/java/hudson/tasks/MavenTest.java index bd366adce71d..dbf745b672d9 100644 --- a/test/src/test/java/hudson/tasks/MavenTest.java +++ b/test/src/test/java/hudson/tasks/MavenTest.java @@ -66,6 +66,7 @@ import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.ToolInstallations; +import org.jvnet.hudson.test.recipes.WithTimeout; import org.kohsuke.stapler.jelly.JellyFacet; /** @@ -190,7 +191,7 @@ private void verify() throws Exception { assertNotNull(isp.installers.get(MavenInstaller.class)); } - @Test public void sensitiveParameters() throws Exception { + @Test @WithTimeout(500) public void sensitiveParameters() throws Exception { FreeStyleProject project = j.createFreeStyleProject(); ParametersDefinitionProperty pdb = new ParametersDefinitionProperty( new StringParameterDefinition("string", "defaultValue", "string description"),