mraible commented on code in PR #154:
URL: https://github.com/apache/roller/pull/154#discussion_r4107459274
##########
it-selenium/logs/roller-2026-08-11-165858.log.gz:
##########
Review Comment:
Removed, along with the rest of `it-selenium/logs`. The existing `logs/`
ignore rule keeps new ones out.
##########
app/src/main/resources/struts.xml:
##########
@@ -17,10 +17,26 @@
directory of this distribution.
-->
<!DOCTYPE struts PUBLIC
- "-//Apache Software Foundation//DTD Struts Configuration 2.5//EN"
- "http://struts.apache.org/dtds/struts-2.5.dtd";>
+ "-//Apache Software Foundation//DTD Struts Configuration 6.5//EN"
+ "https://struts.apache.org/dtds/struts-6.5.dtd";>
<struts>
+ <!-- Struts 7 denies OGNL access to any class outside this allowlist, which
+ otherwise blocks reads of Roller's own action and bean properties. -->
+ <constant name="struts.allowlist.packageNames"
+ value="org.apache.roller,java.util,java.lang"/>
+
+ <!-- Struts serves its bundled JS/CSS (tooltips, etc.) from this path,
which
+ must match the /struts/* filter-mapping in web.xml. Struts defaults it
+ to /static, which Roller does not map. -->
+ <constant name="struts.ui.staticContentPath" value="/struts"/>
+
+ <!-- Struts 7 only binds request parameters to properties annotated with
+ @StrutsParameter. Roller's actions predate that annotation, so without
+ this no form in the application can submit data. Annotating the action
+ beans would be the stricter alternative. -->
+ <constant name="struts.parameters.requireAnnotations" value="false"/>
Review Comment:
Switched to annotations. `struts.parameters.requireAnnotations=false` is
gone, so the Struts 7 default applies, and `@StrutsParameter` now marks exactly
the properties forms and URLs bind to: about 70 setters and `getBean()` getters
across the actions, with `depth = 1` on the form-bean getters, the deepest
nesting any form uses.
It was a real security risk, not just a warning. With the gate off, any OGNL
path reachable from an action could be set from a request parameter. I
confirmed `authenticatedUser.fullName=HACKED` on `profile!save` and
`actionWeblog.creator.fullName=HACKED` on `weblogConfig!save` both changed
persistent data before. Both are rejected now, because getters that return
domain objects (users, weblogs, entries, folders, media directories, templates)
are not annotated.
A few details:
- PlanetGroupSubs used to bind `group.title` and `group.handle` straight
onto the persistent `PlanetGroup`. It now binds to a small `PlanetGroupBean`
DTO that is copied onto the group only after validation.
- Bookmarks' `folder.name` input no longer writes to the persistent folder;
renames go through `folderEdit`.
- `StrutsParameterAnnotationTest` checks every Struts form field,
`<s:param>`, and redirect parameter in the JSPs and struts.xml against Struts'
own authorizer, so a form that loses its binding fails the build.
- Every admin and editor form was exercised against a running instance with
the authorizer logging at DEBUG, plus the Playwright suite in all three auth
modes.
##########
app/src/main/webapp/WEB-INF/web.xml:
##########
@@ -1,8 +1,8 @@
<?xml version="1.0" encoding="UTF-8"?>
-<web-app xmlns="http://xmlns.jcp.org/xml/ns/javaee";
+<web-app xmlns="https://jakarta.ee/xml/ns/jakartaee";
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
- xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/javaee
http://xmlns.jcp.org/xml/ns/javaee/web-app_4_0.xsd";
- version="4.0">
+ xsi:schemaLocation="https://jakarta.ee/xml/ns/jakartaee
https://jakarta.ee/xml/ns/jakartaee/web-app_6_0.xsd";
+ version="6.0">
Review Comment:
Yes, good catch. `web.xml` now declares the `web-app_6_1.xsd` schema and
`version="6.1"`, matching the Servlet 6.1 API the build targets.
##########
.gitignore:
##########
@@ -28,3 +28,4 @@ docker/postgresql-data
docker/roller-data
assembly-release/release.sh
it-selenium/overlays/
+logs/
Review Comment:
Yes. `.gitignore` now covers `docker/postgresql-16-data`, the directory
docker-compose binds.
##########
app/src/main/java/org/apache/roller/weblogger/ui/struts2/util/UIAction.java:
##########
@@ -88,13 +87,14 @@ public abstract class UIAction extends ActionSupport
public void myPrepare() {
// no-op
}
-
- @Override
- public void setRequest(Map<String, Object> map) {
- this.salt = (String) map.get("salt");
- }
public String getSalt() {
+ if (salt == null) {
+ HttpServletRequest request =
ActionContext.getContext().getServletRequest();
Review Comment:
Done. All ten call sites (plus the two `ServletActionContext` lookups
master's installer added) now receive what they need through Struts 7's
`org.apache.struts2.action` interfaces. `UIAction` implements
`ServletRequestAware` and shares the request with subclasses through
`getServletRequest()`, `FolderEdit` uses `ServletResponseAware`, and
`GlobalConfig`, `PlanetConfig`, and `Members` use `ParametersAware`. That
restores the injection pattern the Struts 2 code had.
The browser tests caught one side effect: master's new `BootstrapToken`
action implemented `ServletRequestAware` itself, which replaced `UIAction`'s
method, so the setup page lost its salt. It now uses the inherited request.
##########
app/src/test/resources/roller-jettyrun.properties:
##########
@@ -49,3 +50,13 @@ cache.sitewide.enabled=false
cache.weblogpage.enabled=false
cache.weblogfeed.enabled=false
cache.planet.enabled=false
+
+# Authentication defaults to Roller's own user database, which needs no extra
+# services. To develop against OIDC instead, start Keycloak with
Review Comment:
Agreed. The commented Keycloak settings moved to #155, where the OIDC
support lives.
--
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]
