Skip to content

Commit

Permalink
Merge pull request #8843 from krisstern/feat/stable-2.426/backporting…
Browse files Browse the repository at this point in the history 
…-2.426.3-1

Backporting LTS 2.426.3
  • Loading branch information
@NotMyFault
NotMyFault authored Jan 9, 2024
2 parents 2bc5292 + 914f0d4 commit 96dc95a
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 6 deletions.
42 changes: 40 additions & 2 deletions core/src/main/java/hudson/FilePath.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -962,7 +963,7 @@ public Void invoke(File dir, VirtualChannel channel) throws IOException {
* </ul>
*
* @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
Expand All @@ -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()) {
Expand All @@ -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) {
Expand All @@ -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();
Expand All @@ -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");

Check warning on line 1058 in core/src/main/java/hudson/FilePath.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines 

Lines 1056-1058 are not covered by tests
}
timestamp.touch(sourceTimestamp);
return true;
} catch (IOException x) {
Expand All @@ -1061,13 +1076,36 @@ 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)) {

Check warning on line 1079 in core/src/main/java/hudson/FilePath.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line 

Line 1079 is only partially covered, one branch is missing
/* Store the ETag value in the timestamp file for later use */
timestamp.write(resultEtag, "UTF-8");
}
timestamp.touch(sourceTimestamp);
return true;
} catch (IOException e) {
throw new IOException("Failed to install " + archive + " to " + remote, e);
}
}

/* 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) {

Check warning on line 1094 in core/src/main/java/hudson/FilePath.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line 

Line 1094 is only partially covered, one branch is missing
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<Void> {
private final URL archive;
Expand Down
3 changes: 2 additions & 1 deletion core/src/main/java/hudson/slaves/Cloud.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 + '/');

Check warning on line 334 in core/src/main/java/hudson/slaves/Cloud.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line 

Line 334 is not covered by tests

}

public Cloud reconfigure(@NonNull final StaplerRequest req, JSONObject form) throws Descriptor.FormException {
Expand Down
1 change: 0 additions & 1 deletion core/src/main/java/jenkins/agents/CloudSet.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,6 @@ private void handleNewCloudPage(Descriptor<Cloud> 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);
Expand Down
1 change: 0 additions & 1 deletion core/src/main/resources/hudson/slaves/Cloud/configure.jelly
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ THE SOFTWARE.
<l:isAdmin>
<f:bottomButtonBar>
<f:submit value="${%Save}"/>
<f:apply />
</f:bottomButtonBar>
</l:isAdmin>
</f:form>
Expand Down
71 changes: 71 additions & 0 deletions core/src/test/java/hudson/FilePathTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
3 changes: 2 additions & 1 deletion test/src/test/java/hudson/tasks/MavenTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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"),
Expand Down

0 comments on commit 96dc95a

Please sign in to comment.