Do not compute CSP header if disabled (#26255)

* Do not compute CSP header if disabled

* Replace waiting with counting invocations

* Replace waiting with counting

---------

Co-authored-by: Daniel Beck <daniel-beck@users.noreply.github.com>
Co-authored-by: Kris Stern <88480540+krisstern@users.noreply.github.com>
(cherry picked from commit 6227cd1)

Author: daniel-beck

core/src/main/java/jenkins/security/csp/impl/CspFilter.java

@@ -63,12 +63,14 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
         final String headerName = cspDecorator.getContentSecurityPolicyHeaderName();
         final boolean headerShouldBeSet = headerName != null;
 
-        // This is the preliminary value outside Stapler request handling (and providing a context object)
-        final String headerValue = cspDecorator.getContentSecurityPolicyHeaderValue(req);
-
         final boolean isResourceRequest = ResourceDomainConfiguration.isResourceRequest(req);
 
+        String headerValue = "";
+
         if (headerShouldBeSet && !isResourceRequest) {
+            // This is the preliminary value outside Stapler request handling (and providing a context object)
+            headerValue = cspDecorator.getContentSecurityPolicyHeaderValue(req);
+
             // The Filter/Decorator approach needs us to "set" headers rather than "add", so no additional endpoints are supported at the moment.
             final String reportingEndpoints = cspDecorator.getReportingEndpointsHeaderValue(req);
             if (reportingEndpoints != null) {
@@ -80,10 +82,10 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
         try {
             chain.doFilter(req, rsp);
         } finally {
-            if (headerShouldBeSet) {
+            if (headerShouldBeSet && !isResourceRequest) {
                 try {
                     final String actualHeader = rsp.getHeader(headerName);
-                    if (!isResourceRequest && hasUnexpectedDifference(headerValue, actualHeader)) {
+                    if (hasUnexpectedDifference(headerValue, actualHeader)) {
                         LOGGER.log(Level.FINE, "CSP header has unexpected differences: Expected '" + headerValue + "' but got '" + actualHeader + "'");
                     }
                 } catch (RuntimeException e) {

test/src/test/java/jenkins/security/csp/impl/CspFilterTest.java

@@ -4,19 +4,25 @@
 import static org.hamcrest.Matchers.allOf;
 import static org.hamcrest.Matchers.endsWith;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.greaterThanOrEqualTo;
+import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.nullValue;
 import static org.hamcrest.Matchers.startsWith;
 import static org.jvnet.hudson.test.LoggerRule.recorded;
 
+import hudson.ExtensionList;
 import hudson.security.csrf.CrumbExclusion;
 import jakarta.servlet.FilterChain;
 import jakarta.servlet.ServletException;
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
 import java.io.IOException;
 import java.net.URL;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.logging.Level;
 import jenkins.model.JenkinsLocationConfiguration;
+import jenkins.security.csp.Contributor;
+import jenkins.security.csp.CspBuilder;
 import jenkins.util.HttpServletFilter;
 import org.hamcrest.Matcher;
 import org.htmlunit.HttpMethod;
@@ -27,6 +33,7 @@
 import org.jvnet.hudson.test.LoggerRule;
 import org.jvnet.hudson.test.TestExtension;
 import org.jvnet.hudson.test.junit.jupiter.WithJenkins;
+import org.xml.sax.SAXException;
 
 @WithJenkins
 public class CspFilterTest {
@@ -99,6 +106,44 @@ void testFilterWithoutCsp(JenkinsRule j) throws Exception {
                 endsWith(":YW5vbnltb3Vz::L3Rlc3QtZmlsdGVyLXdpdGhvdXQtY3NwL3NvbWUtcGF0aA==' but got 'null'"))));
     }
 
+    @Test
+    void testCspBuilderNotCalledWithHeaderDisabled(JenkinsRule j) throws IOException, SAXException {
+        try (JenkinsRule.WebClient webClient = j.createWebClient().withJavaScriptEnabled(false)) {
+            final AtomicInteger counter = ExtensionList.lookupSingleton(CountingContributor.class).counter;
+            {
+                int start = counter.get();
+                webClient.goTo("");
+                int end = counter.get();
+
+                final int difference = end - start;
+                assertThat(difference, greaterThanOrEqualTo(1));
+            }
+
+            System.setProperty(SystemPropertyHeaderDecider.SYSTEM_PROPERTY_NAME, "");
+            try {
+                int start = counter.get();
+                webClient.goTo("");
+                int end = counter.get();
+
+                // no calls expected
+                final int difference = end - start;
+                assertThat(difference, is(0));
+            } finally {
+                System.clearProperty(SystemPropertyHeaderDecider.SYSTEM_PROPERTY_NAME);
+            }
+        }
+    }
+
+    @TestExtension("testCspBuilderNotCalledWithHeaderDisabled")
+    public static class CountingContributor implements Contributor {
+        AtomicInteger counter = new AtomicInteger(0);
+
+        @Override
+        public void apply(CspBuilder cspBuilder) {
+            counter.incrementAndGet();
+        }
+    }
+
     @TestExtension
     public static class TestFilter implements HttpServletFilter {
         @Override