mirror of
https://github.com/jenkinsci/jenkins.git
synced 2026-11-10 10:36:21 +00:00
[JENKINS-72469] Avoid repeated tool downloads from misconfigured HTTP servers (#8814)
* [JENKINS-72469] Avoid repeated tool downloads from misconfigured HTTP servers The Azul Systems content delivery network stopped providing the last-modified header in their URL responses. They only provide the ETag header. Add ETag support to the Jenkins FilePath URL download method so that if ETag is provided, we use the ETag value. If last-modified is provided and matches, we continue to honor it as well. https://issues.jenkins.io/browse/JENKINS-72469 has more details. https://community.jenkins.io/t/job-stuck-on-unpacking-global-jdk-tool/11272 also includes more details. Testing done * Automated test added to FilePathTest for code changes on the controller. The automated test confirms that even without a last-modified value, the later downloads are skipped if a matching ETag is received. The automated test also confirms that download is skipped if OK is received with a matching ETag. No automated test was added to confirm download on the agent because that path is not tested by any of the other test automation of this class. * Interactive test with the Azul Systems JDK installer on the controller. I created a tool installer for the Azul JDK. I verified that before this change it was downloaded each time the job was run. I verified that after the change it was downloaded only once. * Interactive test with the Azul Systems JDK installer on an agent. I created a tool installer for the Azul JDK. I verified that before this change it was downloaded each time the job was run. I verified that after the change it was downloaded only once. * Interactive test on the controller with a file download from an NGINX web server confirmed that the tool is downloaded once and then later runs of the job did not download the file again. * Use equals instead of contains to check ETag Don't risk that a substring of an earlier ETag might cause a later ETag to incorrectly assume it does not need to download a modified installer. * Use weak comparison for ETag values https://httpwg.org/specs/rfc9110.html#field.etag describes weak comparison cases and notes that content providers may provide weak or strong entity tags. Updated code to correctly compare weak and strong entity tags. Also improves the null checks based on the suggestions from @mawinter69 in https://github.com/jenkinsci/jenkins/pull/8814#discussion_r1438909824 * Test comparison of weak and strong validators * Do not duplicate test args, more readable * Use better variable names in test Cover more branches in the equalEtags method as well * Fix variable declaration order
This commit is contained in:
1 parent
3597db8e13
commit
c8156d41f2
2 files changed
+111
-2
No files matched your search
@@ -28,6 +28,7 @@ package hudson;
|
||||
|
||||
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 final class FilePath implements SerializableOnlyOverRemoting {
|
||||
* </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
|
||||
@@ -984,12 +985,18 @@ public final class FilePath implements SerializableOnlyOverRemoting {
|
||||
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 @@ public final class FilePath implements SerializableOnlyOverRemoting {
|
||||
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 @@ public final class FilePath implements SerializableOnlyOverRemoting {
|
||||
}
|
||||
|
||||
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 @@ public final class FilePath implements SerializableOnlyOverRemoting {
|
||||
// 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 @@ public final class FilePath implements SerializableOnlyOverRemoting {
|
||||
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 @@ public final class FilePath implements SerializableOnlyOverRemoting {
|
||||
}
|
||||
}
|
||||
|
||||
/* 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<Void> {
|
||||
private final URL archive;
|
||||
|
||||
Reference in new issue
Block a user