The forum_node_form_alter function used in the core Forum module attempts to set the default value of taxonomy_forums by using the forum id in the URL of the node/add// page. However, when the form is being rebuilt as part of an Ajax process, the URL is not the expected node/add// and the value retrieved by the hardcoded arg(3) callback is not the forum tid.

This results in forum posts being posted outside of the expected forum where a user without edit access to the taxonomy_forums field uploads a file to a forum topic, or triggers some other kind of ajax form rebuild.

CommentFileSizeAuthor
#5 edit_forum_posts_lose-2826373-5.patch1.69 KBcebasqueira
#2 forum-posts-lose-their-forum-2826373-2.patch1.71 KBMrDaleSmith
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

MrDaleSmith created an issue. See original summary.

MrDaleSmith’s picture

MrDaleSmith CreditAttribution: MrDaleSmith at CTI Digital commented
Status: Active » Needs review
FileSize
forum-posts-lose-their-forum-2826373-2.patch1.71 KB

Added patch that checks in the $form_state for an existing default value before resorting to arg(3).

Status: Needs review » Needs work

The last submitted patch, 2: forum-posts-lose-their-forum-2826373-2.patch, failed testing.

cebasqueira’s picture

cebasqueira CreditAttribution: cebasqueira commented
Assigned: Unassigned » cebasqueira
cebasqueira’s picture

cebasqueira CreditAttribution: cebasqueira commented
Assigned: cebasqueira » Unassigned
Status: Needs work » Needs review
FileSize
edit_forum_posts_lose-2826373-5.patch1.69 KB

Status: Needs review » Needs work

The last submitted patch, 5: edit_forum_posts_lose-2826373-5.patch, failed testing.

cebasqueira’s picture

cebasqueira CreditAttribution: cebasqueira commented
Status: Needs work » Needs review
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan as a volunteer commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Thanks!

+++ b/modules/forum/forum.module
@@ -630,10 +630,24 @@ function forum_form_node_form_alter(&$form, &$form_state, $form_id) {
+      // If $form_state['complete_form'] exists, the form is rebuilding. We need
...
+          $requested_forum_id = $form_state['complete form']['taxonomy_forums'][$langcode]['#default_value'];

I think at this stage it would be in $form_state['rebuild'] and in which case the value should be in $form_state['values']['taxonomy_forums'][$langcode] - can you check, as that is the normal approach to rebuilding a form from previous values. The problem with using the default value is the user may have changed the default before the ajax operation, and in which case the original value would be used.

Also, we need a test to go with this.

MrDaleSmith’s picture

MrDaleSmith CreditAttribution: MrDaleSmith at CTI Digital commented

The rebuild process replaces any value information from the $form_state already, but as the user doesn't have access to the field, the value isn't carried across into the $form_state.

I'll see if I can work out how to do testing and update the patch. :)

Antonio W. Mucciolo Jr.’s picture

Antonio W. Mucciolo Jr. CreditAttribution: Antonio W. Mucciolo Jr. as a volunteer and at CI&T commented
Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs tests +Reviewed & tested by the community

+RTBC

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan as a volunteer and at PreviousNext commented
Status: Reviewed & tested by the community » Needs work
Issue tags: -Reviewed & tested by the community +Needs tests

My review hasn't been addressed, nor have the tests been added

MrDaleSmith’s picture

MrDaleSmith CreditAttribution: MrDaleSmith at CTI Digital commented

Running the forum tests locally I get multiple fails before I've made any changes, which makes it hard to create a new test for this behaviour. Can't see an issue for this in the issue queue: is this a known problem, or is it something that only occurs to me locally?