andruhon commented on a change in pull request #376: WICKET-6682 add CSP nonce
support: DecoratingHeaderResponse approach
URL: https://github.com/apache/wicket/pull/376#discussion_r303823925
##########
File path:
wicket-core/src/main/java/org/apache/wicket/markup/head/JavaScriptHeaderItem.java
##########
@@ -365,15 +372,54 @@ protected final void
internalRenderJavaScriptReference(Response response, String
boolean isAjax =
RequestCycle.get().find(IPartialPageRequestHandler.class).isPresent();
// the url needs to be escaped when Ajax, because it will break
the Ajax Response XML (WICKET-4777)
CharSequence escapedUrl = isAjax ? Strings.escapeMarkup(url):
url;
-
- JavaScriptUtils.writeJavaScriptUrl(response, escapedUrl, id,
defer, charset, async);
+ AttributeMap attributes = AttributeMap.of(
+ HeaderItemAttribute.TYPE, "text/javascript",
+ HeaderItemAttribute.SCRIPT_SRC,
String.valueOf(escapedUrl)
+ );
+ if (id != null)
+ {
+ attributes.add(HeaderItemAttribute.ID, id);
+ }
+ if (defer)
+ {
+ attributes.add(HeaderItemAttribute.SCRIPT_DEFER,
"defer");
+ }
+ if (charset != null)
+ {
+ // XXX this attribute is not necessary for modern
browsers
+ attributes.add("charset", charset);
+ }
+ if (async)
+ {
+ attributes.add(HeaderItemAttribute.SCRIPT_ASYNC,
"async");
+ }
+ attributes.compute(HeaderItemAttribute.CSP_NONCE,
this::getNonce);
Review comment:
> I'm not ok with adding #noEscapeMarkupKeys to ValueMap.
>
> Maybe ValueMap isn't the right class for this, note that you're still
calling #add() when you should use #put() instead.
> Maybe it would be better to just add another "nonce" parameter to the
JavaScriptUtils methods and postpone the attempt to clean-up its signature and
delay the decision about 4777.
True. It's kind of ugly. There should be a way to render some parameters
without escaping them, even if it's never done by default.
Honestly I don't quite like that toString does the magic. What do you think
about adding a new method to IValueMap in wicket 9?
I would introduce two new methods, something like `#renderAttributes()` and
`#renderAttributes(Collection<String> nonEscapedAttributes)` in IValueMap, so
the map doesn't have to control them.
Somewhat like this:
```
/**
* Generates a <code>String</code> representation of this object. All
values are escaped by default.
*
* @param noEscapeMarkupKeys a collection of parameters which should
not be escaped
* @return <code>String</code> representation of this
<code>ValueMap</code> consistent with the
* tag-attribute style of markup elements. For example:
<code>a="x" b="y" c="z"</code>.
*/
public String renderAttributes(Collection<String> noEscapeMarkupKeys)
{
final StringBuilder buffer = new StringBuilder();
boolean first = true;
for (Map.Entry<String, Object> entry : entrySet())
{
if (first == false)
{
buffer.append(' ');
}
first = false;
String key = entry.getKey();
buffer.append(key);
buffer.append("=\"");
final Object value = entry.getValue();
if (value == null)
{
buffer.append("null");
}
else if (value.getClass().isArray())
{
buffer.append(Arrays.asList((Object[])value));
}
else
{
buffer.append(
!noEscapeMarkupKeys.isEmpty()
&& noEscapeMarkupKeys.contains(key)
? value
:
Strings.escapeMarkup(String.valueOf(value))
);
}
buffer.append('\"');
}
return buffer.toString();
}
/**
* Generates a <code>String</code> representation of this object. All
values are escaped.
*
* @return <code>String</code> representation of this
<code>ValueMap</code> consistent with the
* tag-attribute style of markup elements. For example:
<code>a="x" b="y" c="z"</code>.
*/
public String renderAttributes() {
return renderAttributes(Collections.emptyList());
}
/**
* Generates a <code>String</code> representation of this object. All
values are escaped.
*
* @return <code>String</code> representation of this
<code>ValueMap</code> consistent with the
* tag-attribute style of markup elements. For example:
<code>a="x" b="y" c="z"</code>.
*/
@Override
public String toString() {
return renderAttributes();
}
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
