[
https://issues.apache.org/jira/browse/WICKET-7183?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18093897#comment-18093897
]
ASF GitHub Bot commented on WICKET-7183:
----------------------------------------
solomax commented on code in PR #1504:
URL: https://github.com/apache/wicket/pull/1504#discussion_r3526580373
##########
wicket-core/src/main/java/org/apache/wicket/core/util/string/JavaScriptUtils.java:
##########
@@ -95,6 +95,29 @@ public static CharSequence escapeQuotesAndBackslash(final
CharSequence input)
return s;
}
+ /**
+ * Removes the surrounding single quotes from a string after trimming
+ * leading and trailing whitespace.
+ *
+ * <p>Examples:</p>
+ * <pre>
+ * unquoteSingleQuoted(" 'url' ") -> "url"
+ * unquoteSingleQuoted("'url'") -> "url"
+ * unquoteSingleQuoted("''") -> ""
+ * </pre>
+ *
+ * @param s the single-quoted string
+ * @return the contents between the outer single quotes
+ */
+ public static String unquoteSingleQuoted(String s)
+ {
+ if (s != null && s.trim().startsWith("'") &&
s.trim().endsWith("'"))
Review Comment:
`trim()` is used 3 times, maybe it's better to introduce variable for
trimmed string?
> Changed behaviour / JavaDoc of PopupSettings#setTarget incorrect since latest
> security fixes
> --------------------------------------------------------------------------------------------
>
> Key: WICKET-7183
> URL: https://issues.apache.org/jira/browse/WICKET-7183
> Project: Wicket
> Issue Type: Bug
> Affects Versions: 9.23.0
> Reporter: Daniel Radünz
> Priority: Minor
>
> Due to the changed behaviour of {{PopupSettings#setTarget}} with [this
> commit|https://github.com/apache/wicket/pull/1450/changes/ceaac22b5df520954cf3c114d52852332cf38814#diff-6051c993387bf3d6e5c1194b954d1bc7603cbfd5deab3df5b1f9b7b50023733aR159-R162]
> the JavaDoc is now incorrect. It still states, that links have to be
> manually enclosed by single quotes.
> {panel:title=JavaDoc}
> Note: if the target is an url (relative or absolute) then it should be
> wrapped in quotes, for example: setTarget("'some/url'").
> {panel}
> In Wicket 9.22.0 this still worked as described, in Wicket 9.23.0 this now
> leads to incorrectly opened Popup on our site with links looking like
> "http://example.com/mypage/Page1'../mypage/Page2'?1" (noctice the quotes in
> the URL). Removing the manually added single quotes in our Java code when
> calling setTarget worked just fine for us though.
> Since this was a change to increase the security of Wicket, I'd assume that
> changing the JavaDoc is the prefered course of action rather than reverting
> the change, even if the change might break things for a small group of people
> using this method.
> I'd assume this affects Wicket 10 as well, but we are still on 9, so that's
> where we noticed it.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
