|
| Description: |
The "Delete" action for comments and annotations (AnnotationCode.Macros#displayAnnotationToolboxFromReference,
and the equivalent in flamingo's commentsinline.vm) is a plain `<a href="...">` link to the `objectremove`
action, gated only by a confirmation modal (Bootstrap `#deleteModal`) whose confirm button navigates the
browser to that link's href. The link itself performs the actual mutation (deleting the comment/annotation
object) as a simple GET-style navigation, protected only by a `form_token` carried in the query string.
This was flagged during review of PR #5928 (https://github.com/xwiki/xwiki-platform/pull/5928#discussion_r3735753256):
using a link to modify server-side state is bad practice; a proper HTML `<form method="POST">` should be
used instead. The reviewer noted this pattern is already used pervasively across the codebase (comments,
annotations, and elsewhere), so it's a pre-existing issue, not something introduced by that PR - it should be
tracked and fixed on its own rather than folded into an unrelated UI-focused PR.
Several comment/annotation actions mutate server-side state through a plain `<a href="...">` link instead of
a real HTTP POST/PUT/DELETE request - using a link to modify state is bad practice: the href can be followed
by a crawler, prefetched, opened in a new tab, or navigated to directly if JavaScript never attaches, in
which case the mutation happens as a simple GET-style navigation.
Remaining occurrences:
1. The "Delete" action for standard comments in flamingo's commentsinline.vm: a plain `<a href="...">` link
to the `objectremove` action, gated only by a confirmation modal (Bootstrap `#deleteModal`) whose confirm
button navigates the browser to that link's href, protected only by a `form_token` carried in the query
string.
2. In AnnotationCode.Macros#displayAnnotationToolboxFromReference's `#else` branch (used for custom,
non-`XWiki.XWikiComments` annotation classes), the `validate` and `delete` actions are both
`<a href="...">` links relying on the annotation REST API's HTTP-method-override query parameter
(`?method=PUT` / `?method=DELETE`) rather than an actual PUT/DELETE request. |
|
