snoopdave commented on code in PR #227:
URL: https://github.com/apache/roller/pull/227#discussion_r4406288156
##########
app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/ValidateSaltInterceptor.java:
##########
@@ -33,13 +35,23 @@
import com.opensymphony.xwork2.interceptor.AbstractInterceptor;
/**
- * Validates salts after Struts has wrapped a multipart form request.
+ * Validates salts after Struts has wrapped a multipart form request, and on
+ * requests other than POST that call an action method which changes data.
*/
public class ValidateSaltInterceptor extends AbstractInterceptor implements
StrutsStatics {
private static final long serialVersionUID = 2446434402795510394L;
private static final Log log =
LogFactory.getLog(ValidateSaltInterceptor.class);
+ /**
+ * Action methods that only display data. Links and redirects may reach
+ * these without a salt; any other method needs a POST or a salt.
+ */
+ static final Set<String> DISPLAY_METHODS = Set.of(
+ "execute", "input", "view", "search", "query", "edit", "cancel",
Review Comment:
🤖Claude: fixed by having `PingTargetEdit` take the request method (like
`GlobalConfig`) and save only on a POST. Any other method returns the input
result without saving. The modal already saves by AJAX POST, so the page works
as before. New `PingTargetEditTest` checks that a GET does not save or flush
and that a POST does; the GET case fails without the change. Also checked on
Jetty: a GET to `commonPingTargetAdd.rol` with bean parameters adds nothing,
and the modal POST still adds the target. A sweep of the other exempt methods
found no others that save. (Local change, in the next push.)
##########
app/src/main/webapp/WEB-INF/jsps/editor/Pings.jsp:
##########
@@ -69,13 +69,15 @@
<td class="rollertable" align="center" >
<s:if test="pingStatus[#pingTarget.id]">
<s:url var="disableUrl" action="pings!disable" >
+ <s:param name="salt" value="%{salt}" />
Review Comment:
🤖Claude: fixed by returning a new `done` result from
`pings!enable/disable/pingNow`, `commonPingTargets!enable/disable` and
`menu!accept/decline`. The result redirects to the plain page
(`pings.rol?weblog=…`, `commonPingTargets.rol`, `menu.rol`), and those three
actions add the Struts `store` interceptor in AUTOMATIC mode, so the ping
result and the invite messages survive the redirect and show once. Checked on
Jetty: each link returns a 302 without the salt in `Location`, the message
appears after the redirect, and a refresh shows the page with no error. While
checking this I found that the invite links double-escaped `&` (`s:url` and
then `s:property`), so `inviteId` never arrived; they now use `s:a href="%{…}"`
like the pings pages. Opening a second link from the same, already-used page
still needs a reload, because every link on a page shares one salt. (Local
change, in the next push.)
##########
CHANGES.md:
##########
@@ -58,6 +58,11 @@
- **Startup fails if the XML-RPC parser cannot be configured.** Roller used to
log an error and continue. It now stops at startup, so check the log if a
custom XML parser is on the classpath.
+- **Links and URLs that change data need the form token.** A request other
Review Comment:
🤖Claude: fixed by closing the ping target path (see the `PingTargetEdit`
reply), so the entry holds as written. I also added that saving a common ping
target needs a POST, and that the links return to the page without the salt in
the address. (Local change, in the next push.)
--
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]
