[FIXED JENKINS-17028]

Some browsers appear to cache 302 requests in violation of RFC (I'm looking at you, Chrome: http://code.google.com/p/chromium/issues/detail?id=103458) I also saw this behavior with Firefox, even though I couldn't locate any bug report. Sine Chrome alone is a big enough browser share, in this change I modified the code to avoid 302 redirects and instead to service the request with 200. To avoid excessive data transfer, ETag is used to detect that the browser has the image in cache.
jenkinsci · Mar 1, 2013 · 1bac74e · 1bac74e
1 parent 6dfe95e
commit 1bac74e
Show file tree

Hide file tree

Showing 3 changed files with 112 additions and 20 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/badge/BadgeAction.java b/src/main/java/org/jenkinsci/plugins/badge/BadgeAction.java
@@ -3,15 +3,28 @@
 import hudson.model.AbstractProject;
 import hudson.model.Action;
 import hudson.util.HttpResponses;
+import hudson.util.IOUtils;
 import jenkins.model.Jenkins;
 import org.kohsuke.stapler.HttpResponse;
+import org.kohsuke.stapler.StaplerRequest;
+import org.kohsuke.stapler.StaplerResponse;
+
+import javax.servlet.ServletException;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URL;
+
+import static javax.servlet.http.HttpServletResponse.SC_NOT_MODIFIED;
 
 /**
 * @author Kohsuke Kawaguchi
 */
 public class BadgeAction implements Action {
+    private final BadgeActionFactory factory;
     public final AbstractProject project;
-    public BadgeAction(AbstractProject project) {
+
+    public BadgeAction(BadgeActionFactory factory, AbstractProject project) {
+        this.factory = factory;
         this.project = project;
     }
 
@@ -31,23 +44,6 @@ public String getUrlName() {
      * Serves the badge image.
      */
     public HttpResponse doIcon() {
-        String file;
-        switch (project.getIconColor().noAnime()) {
-        case RED:
-        case ABORTED:
-            file = "failure.png";
-            break;
-        case YELLOW:
-            file = "unstable.png";
-            break;
-        case BLUE:
-            file = "success.png";
-            break;
-        default:
-            file = "running.png";
-            break;
-        }
-
-        return HttpResponses.redirectViaContextPath(Jenkins.RESOURCE_PATH + "/plugin/embeddable-build-status/status/" + file);
+        return factory.getImage(project.getIconColor());
     }
 }
diff --git a/src/main/java/org/jenkinsci/plugins/badge/BadgeActionFactory.java b/src/main/java/org/jenkinsci/plugins/badge/BadgeActionFactory.java
@@ -3,8 +3,10 @@
 import hudson.Extension;
 import hudson.model.AbstractProject;
 import hudson.model.Action;
+import hudson.model.BallColor;
 import hudson.model.TransientProjectActionFactory;
 
+import java.io.IOException;
 import java.util.Collection;
 import java.util.Collections;
 
@@ -13,9 +15,32 @@
  */
 @Extension
 public class BadgeActionFactory extends TransientProjectActionFactory {
+    private final StatusImage[] images = new StatusImage[4];
+
+    public BadgeActionFactory() throws IOException {
+        images[0] = new StatusImage("failure.png");
+        images[1] = new StatusImage("unstable.png");
+        images[2] = new StatusImage("success.png");
+        images[3] = new StatusImage("running.png");
+    }
+
     @Override
     public Collection<? extends Action> createFor(AbstractProject target) {
-        return Collections.singleton(new BadgeAction(target));
+        return Collections.singleton(new BadgeAction(this,target));
+    }
+
+    public StatusImage getImage(BallColor color) {
+        switch (color.noAnime()) {
+        case RED:
+        case ABORTED:
+            return images[0];
+        case YELLOW:
+            return images[1];
+        case BLUE:
+            return images[2];
+        default:
+            return images[3];
+        }
     }
 
 }
diff --git a/src/main/java/org/jenkinsci/plugins/badge/StatusImage.java b/src/main/java/org/jenkinsci/plugins/badge/StatusImage.java
@@ -0,0 +1,71 @@
+package org.jenkinsci.plugins.badge;
+
+import hudson.util.IOUtils;
+import jenkins.model.Jenkins;
+import org.kohsuke.stapler.HttpResponse;
+import org.kohsuke.stapler.StaplerRequest;
+import org.kohsuke.stapler.StaplerResponse;
+
+import javax.servlet.ServletException;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URL;
+
+import static javax.servlet.http.HttpServletResponse.*;
+
+/**
+ * Status image as an {@link HttpResponse}, with proper cache handling.
+ *
+ * <p>
+ * Originally we used 302 redirects to map the status URL to a proper permanent image URL,
+ * but it turns out that some browsers cache 302 redirects in violation of RFC
+ * (see http://code.google.com/p/chromium/issues/detail?id=103458)
+ *
+ * <p>
+ * So this version directly serves the image at the status URL. Since the status
+ * can change any time, we use ETag to skip the actual data transfer if possible.
+ *
+ * @author Kohsuke Kawaguchi
+ */
+class StatusImage implements HttpResponse {
+    private final byte[] payload;
+
+    /**
+     * To improve the caching, compute unique ETag.
+     *
+     * This needs to differentiate different image types, and possible future image changes
+     * in newer versions of this plugin.
+     */
+    private final String etag;
+
+    private final String length;
+
+    StatusImage(String fileName) throws IOException {
+        etag = Jenkins.RESOURCE_PATH+'/'+fileName;
+
+        URL image = new URL(
+            Jenkins.getInstance().pluginManager.getPlugin("embeddable-build-status").baseResourceURL,
+            "status/"+fileName);
+        InputStream s = image.openStream();
+        try {
+            payload = IOUtils.toByteArray(s);
+        } finally {
+            IOUtils.closeQuietly(s);
+        }
+        length = Integer.toString(payload.length);
+    }
+
+    public void generateResponse(StaplerRequest req, StaplerResponse rsp, Object node) throws IOException, ServletException {
+        String v = req.getHeader("If-None-Match");
+        if (etag.equals(v)) {
+            rsp.setStatus(SC_NOT_MODIFIED);
+            return;
+        }
+
+        rsp.setHeader("ETag",etag);
+        rsp.setHeader("Expires","Fri, 01 Jan 1984 00:00:00 GMT");
+        rsp.setHeader("Content-Type", "image/png");
+        rsp.setHeader("Content-Length", length);
+        rsp.getOutputStream().write(payload);
+    }
+}