Copilot commented on code in PR #1174:
URL: https://github.com/apache/struts/pull/1174#discussion_r2328562896
##########
core/src/main/java/org/apache/struts2/interceptor/csp/DefaultCspSettings.java:
##########
@@ -43,63 +45,102 @@
*/
public class DefaultCspSettings implements CspSettings {
- private final static Logger LOG =
LogManager.getLogger(DefaultCspSettings.class);
+ private static final Logger LOG =
LogManager.getLogger(DefaultCspSettings.class);
+ private static final String NONCE_KEY = "nonce";
private final SecureRandom sRand = new SecureRandom();
+ private CspNonceSource nonceSource = CspNonceSource.SESSION;
+
protected String reportUri;
protected String reportTo;
// default to reporting mode
protected String cspHeader = CSP_REPORT_HEADER;
+ @Inject(value = StrutsConstants.STRUTS_CSP_NONCE_SOURCE, required = false)
+ public void setNonceSource(String nonceSource) {
+ if (StringUtils.isBlank(nonceSource)) {
+ this.nonceSource = CspNonceSource.SESSION;
+ } else {
+ this.nonceSource =
CspNonceSource.valueOf(nonceSource.toUpperCase());
+ }
+ }
+
@Override
public void addCspHeaders(HttpServletRequest request, HttpServletResponse
response) {
+ if (this.nonceSource == CspNonceSource.SESSION) {
+ addCspHeadersWithSession(request, response);
+ } else if (this.nonceSource == CspNonceSource.REQUEST) {
+ addCspHeadersWithRequest(request, response);
+ } else {
+ LOG.warn("Unknown nonce source: {}, ignoring CSP settings",
nonceSource);
+ }
+ }
+
+ private void addCspHeadersWithSession(HttpServletRequest request,
HttpServletResponse response) {
if (isSessionActive(request)) {
LOG.trace("Session is active, applying CSP settings");
- associateNonceWithSession(request);
- response.setHeader(cspHeader, createPolicyFormat(request));
+ String nonceValue = generateNonceValue();
+ request.getSession().setAttribute(NONCE_KEY, nonceValue);
+ response.setHeader(cspHeader, createPolicyFormat(nonceValue));
} else {
- LOG.trace("Session is not active, ignoring CSP settings");
+ LOG.debug("Session is not active, ignoring CSP settings");
}
}
+ private void addCspHeadersWithRequest(HttpServletRequest request,
HttpServletResponse response) {
+ String nonceValue = generateNonceValue();
+ request.setAttribute(NONCE_KEY, nonceValue);
+ response.setHeader(cspHeader, createPolicyFormat(nonceValue));
+ }
+
private boolean isSessionActive(HttpServletRequest request) {
return request.getSession(false) != null;
}
- private void associateNonceWithSession(HttpServletRequest request) {
- String nonceValue =
Base64.getUrlEncoder().encodeToString(getRandomBytes());
- request.getSession().setAttribute("nonce", nonceValue);
+ private String generateNonceValue() {
+ return Base64.getUrlEncoder().encodeToString(getRandomBytes());
}
- protected String createPolicyFormat(HttpServletRequest request) {
- StringBuilder policyFormatBuilder = new StringBuilder()
- .append(OBJECT_SRC)
- .append(format(" '%s'; ", NONE))
- .append(SCRIPT_SRC)
- .append(" 'nonce-%s' ") // nonce placeholder
- .append(format("'%s' ", STRICT_DYNAMIC))
- .append(format("%s %s; ", HTTP, HTTPS))
- .append(BASE_URI)
- .append(format(" '%s'; ", NONE));
+ protected String createPolicyFormat(String nonceValue) {
+ StringBuilder builder = new StringBuilder()
+ .append(OBJECT_SRC)
+ .append(format(" '%s'; ", NONE))
+ .append(SCRIPT_SRC)
+ .append(format(" 'nonce-%s' ", nonceValue))
+ .append(format("'%s' ", STRICT_DYNAMIC))
+ .append(format("%s %s; ", HTTP, HTTPS))
+ .append(BASE_URI)
+ .append(format(" '%s'; ", NONE));
if (reportUri != null) {
- policyFormatBuilder
- .append(REPORT_URI)
- .append(format(" %s; ", reportUri));
- if(reportTo != null) {
- policyFormatBuilder
+ builder
+ .append(REPORT_URI)
+ .append(format(" %s; ", reportUri));
+ if (reportTo != null) {
+ builder
.append(REPORT_TO)
.append(format(" %s; ", reportTo));
}
}
- return format(policyFormatBuilder.toString(), getNonceString(request));
+ return builder.toString();
+ }
+
+ /**
+ * @deprecated since 6.8.0, for removal
+ */
+ @Deprecated
+ protected String createPolicyFormat(HttpServletRequest request) {
+ throw new UnsupportedOperationException("Unsupported implementation,
use createPolicyFormat(String) instead!");
Review Comment:
The deprecated method throws an UnsupportedOperationException immediately,
which creates a breaking change for any existing subclasses that override this
method. Consider providing a default implementation that extracts the nonce
from the request and calls the new method, or document this as a breaking
change.
```suggestion
// Default implementation: extract nonce from request attribute
"cspNonce"
Object nonceObj = request.getAttribute("cspNonce");
String nonceValue = (nonceObj instanceof String) ? (String) nonceObj
: null;
if (nonceValue == null) {
// Fallback: generate a new nonce if not present
nonceValue = generateNonceValue();
}
return createPolicyFormat(nonceValue);
```
##########
core/src/main/java/org/apache/struts2/interceptor/csp/DefaultCspSettings.java:
##########
@@ -43,63 +45,102 @@
*/
public class DefaultCspSettings implements CspSettings {
- private final static Logger LOG =
LogManager.getLogger(DefaultCspSettings.class);
+ private static final Logger LOG =
LogManager.getLogger(DefaultCspSettings.class);
+ private static final String NONCE_KEY = "nonce";
private final SecureRandom sRand = new SecureRandom();
+ private CspNonceSource nonceSource = CspNonceSource.SESSION;
+
protected String reportUri;
protected String reportTo;
// default to reporting mode
protected String cspHeader = CSP_REPORT_HEADER;
+ @Inject(value = StrutsConstants.STRUTS_CSP_NONCE_SOURCE, required = false)
+ public void setNonceSource(String nonceSource) {
+ if (StringUtils.isBlank(nonceSource)) {
+ this.nonceSource = CspNonceSource.SESSION;
+ } else {
+ this.nonceSource =
CspNonceSource.valueOf(nonceSource.toUpperCase());
+ }
+ }
+
@Override
public void addCspHeaders(HttpServletRequest request, HttpServletResponse
response) {
+ if (this.nonceSource == CspNonceSource.SESSION) {
+ addCspHeadersWithSession(request, response);
+ } else if (this.nonceSource == CspNonceSource.REQUEST) {
+ addCspHeadersWithRequest(request, response);
+ } else {
+ LOG.warn("Unknown nonce source: {}, ignoring CSP settings",
nonceSource);
+ }
+ }
+
+ private void addCspHeadersWithSession(HttpServletRequest request,
HttpServletResponse response) {
if (isSessionActive(request)) {
LOG.trace("Session is active, applying CSP settings");
- associateNonceWithSession(request);
- response.setHeader(cspHeader, createPolicyFormat(request));
+ String nonceValue = generateNonceValue();
+ request.getSession().setAttribute(NONCE_KEY, nonceValue);
+ response.setHeader(cspHeader, createPolicyFormat(nonceValue));
} else {
- LOG.trace("Session is not active, ignoring CSP settings");
+ LOG.debug("Session is not active, ignoring CSP settings");
}
}
+ private void addCspHeadersWithRequest(HttpServletRequest request,
HttpServletResponse response) {
+ String nonceValue = generateNonceValue();
+ request.setAttribute(NONCE_KEY, nonceValue);
+ response.setHeader(cspHeader, createPolicyFormat(nonceValue));
+ }
+
private boolean isSessionActive(HttpServletRequest request) {
return request.getSession(false) != null;
}
- private void associateNonceWithSession(HttpServletRequest request) {
- String nonceValue =
Base64.getUrlEncoder().encodeToString(getRandomBytes());
- request.getSession().setAttribute("nonce", nonceValue);
+ private String generateNonceValue() {
+ return Base64.getUrlEncoder().encodeToString(getRandomBytes());
}
- protected String createPolicyFormat(HttpServletRequest request) {
- StringBuilder policyFormatBuilder = new StringBuilder()
- .append(OBJECT_SRC)
- .append(format(" '%s'; ", NONE))
- .append(SCRIPT_SRC)
- .append(" 'nonce-%s' ") // nonce placeholder
- .append(format("'%s' ", STRICT_DYNAMIC))
- .append(format("%s %s; ", HTTP, HTTPS))
- .append(BASE_URI)
- .append(format(" '%s'; ", NONE));
+ protected String createPolicyFormat(String nonceValue) {
+ StringBuilder builder = new StringBuilder()
+ .append(OBJECT_SRC)
+ .append(format(" '%s'; ", NONE))
+ .append(SCRIPT_SRC)
+ .append(format(" 'nonce-%s' ", nonceValue))
+ .append(format("'%s' ", STRICT_DYNAMIC))
+ .append(format("%s %s; ", HTTP, HTTPS))
+ .append(BASE_URI)
+ .append(format(" '%s'; ", NONE));
if (reportUri != null) {
- policyFormatBuilder
- .append(REPORT_URI)
- .append(format(" %s; ", reportUri));
- if(reportTo != null) {
- policyFormatBuilder
+ builder
+ .append(REPORT_URI)
+ .append(format(" %s; ", reportUri));
+ if (reportTo != null) {
+ builder
.append(REPORT_TO)
.append(format(" %s; ", reportTo));
}
}
- return format(policyFormatBuilder.toString(), getNonceString(request));
+ return builder.toString();
+ }
+
+ /**
+ * @deprecated since 6.8.0, for removal
+ */
+ @Deprecated
+ protected String createPolicyFormat(HttpServletRequest request) {
+ throw new UnsupportedOperationException("Unsupported implementation,
use createPolicyFormat(String) instead!");
}
+ /**
+ * @deprecated since 6.8.0, for removal
+ */
+ @Deprecated
protected String getNonceString(HttpServletRequest request) {
- Object nonce = request.getSession().getAttribute("nonce");
- return Objects.toString(nonce);
+ throw new UnsupportedOperationException("Unsupported implementation,
don't use!");
}
Review Comment:
Similarly, this deprecated method throws an UnsupportedOperationException
immediately, creating a potential breaking change. Consider providing a bridge
implementation or clearly documenting the breaking change in release notes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
