These two functions use raw $_POST, rather than having the relevant operations handled by a _submit function:

http://api.drupal.org/api/HEAD/function/node_admin_content

http://api.drupal.org/api/HEAD/function/comment_admin

In addition to other problems, it causes tabs and help for the admin page to show on the deletion page.

CommentFileSizeAuthor
#10 kill_post_node_4.patch6.32 KBpwolanin
#6 kill_post_node_3.patch6.37 KBpwolanin
#5 kill_post_node_2.patch6.92 KBpwolanin
#3 kill_post_node_1.patch6.81 KBpwolanin

Comments

pwolanin’s picture

pwolanin commented
Title: failure to use forms API in comment and node modules » use of raw $_POST in comment, node, and other modules

a quick grep turns up these possible use of raw $_POST:

modules/poll/poll.module:525:    $edit = $_POST;
modules/poll/poll.module:527:    $vote = $_POST['vote'];
modules/poll/poll.module:620:    if ($_POST['op'] == t('Vote')) {
modules/search/search.module:926:  // Search form submits with POST but redirects to GET. This way we can keep
modules/search/search.module:929:  if (!isset($_POST['form_id'])) {
modules/system/system.module:1256:  if (!empty($_POST) && form_get_errors()) {
modules/system/system.module:1266:  $op = isset($_POST['op']) ? $_POST['op'] : '';
modules/system/system.module:2175:  if (function_exists('db_check_setup') && empty($_POST)) {
modules/system/system.module:2225:      $_POST['default_logo'] = 0;
modules/system/system.module:2226:      $_POST['logo_path'] = $file->filepath;
modules/system/system.module:2227:      $_POST['toggle_logo'] = 1;
modules/system/system.module:2240:      $_POST['default_favicon'] = 0;
modules/system/system.module:2241:      $_POST['favicon_path'] = $file->filepath;
modules/system/system.module:2242:      $_POST['toggle_favicon'] = 1;
modules/system/system.module:2412: * You can check for the existence of $_POST[$name] (where $name
modules/taxonomy/taxonomy.module:1441:  if ($_POST['op'] == t('Delete') || $_POST['confirm']) {
modules/taxonomy/taxonomy.module:1451:  if ($_POST['op'] == t('Delete') || $_POST['confirm']) {
modules/upload/upload.module:324:  if (!$_POST) {
modules/upload/upload.module:331:  // get a key in _POST form_builder has no way of knowing the difference between a check
modules/upload/upload.module:710:  $node = (object)$_POST;
modules/upload/upload.module:720:    '#post' => $_POST,
modules/user/user.module:1584:  $op = !empty($_POST['op']) ? $_POST['op'] : '';
modules/user/user.module:1585:  $edit = $op ? $_POST : (array)$account;
modules/user/user.module:1821:  if ($edit = $_POST) {
modules/user/user.module:1868:  if ($edit = $_POST) {
modules/user/user.module:2729:  $op = isset($_POST['op']) ? $_POST['op'] : $callback_arg;
modules/user/user.module:2734:      $keys = isset($_POST['keys']) ? $_POST['keys'] : NULL;
modules/user/user.module:2742:      if (!empty($_POST['accounts']) && isset($_POST['operation']) && ($_POST['operation'] == 'delete')) {
modules/node/node.module:1594:  if (isset($_POST['operation']) && ($_POST['operation'] == 'delete') && $_POST['nodes']) {
modules/node/node.module:1834:  $keys = isset($_POST['keys']) ? $_POST['keys'] : NULL;
modules/node/node.module:2509:  if (isset($_POST['op']) && ($_POST['op'] == t('Delete'))) {
modules/comment/comment.module:662:  $op = isset($_POST['op']) ? $_POST['op'] : '';
modules/comment/comment.module:1173:  $edit = $_POST;
modules/comment/comment.module:1452:  $op = isset($_POST['op']) ? $_POST['op'] : '';
modules/comment/comment.module.orig:662:  $op = isset($_POST['op']) ? $_POST['op'] : '';
modules/comment/comment.module.orig:1173:  $edit = $_POST;
modules/comment/comment.module.orig:1452:  $op = isset($_POST['op']) ? $_POST['op'] : '';
pwolanin’s picture

pwolanin commented

bump - D6 should not ship without fixing this 4.6 legacy code..

pwolanin’s picture

pwolanin commented
Status: Active » Needs work
StatusFileSize
new kill_post_node_1.patch6.81 KB

This is a partial patch - gets 2 of the 3 uses in node module. The 3rd is tricker since it goes into search module.

pwolanin’s picture

pwolanin commented
Title: use of raw $_POST in comment, node, and other modules » use of raw $_POST in node module

let's make this issue only for node module.

a separate issue for FAPI and menu cleanup in taxonomy module: http://drupal.org/node/163297

someone should start one for comment module (if it doesn't exist).

pwolanin’s picture

pwolanin commented
Title: use of raw $_POST in node module » fix use of raw $_POST in node module
Status: Needs work » Needs review
StatusFileSize
new kill_post_node_2.patch6.92 KB

Actually - this patch should get reviewed as is - no need to wait to fix the 3rd instance. That can come as a separate patch.

same patch attached, but re-rolled to remove offset. Enable/disable a module to force a menu rebuild.

pwolanin’s picture

pwolanin commented
StatusFileSize
new kill_post_node_3.patch6.37 KB

fix comment text and unneeded code movement

dmitrig01’s picture

dmitrig01 commented

haven't tested, but it passes code review

moshe weitzman’s picture

moshe weitzman commented

code looks like a nice cleanup. no time to test now - sorry.

dries’s picture

dries commented
Status: Needs review » Needs work

Tried testing it, but the code no longer applies. Requires a re-roll.

pwolanin’s picture

pwolanin commented
Status: Needs work » Needs review
StatusFileSize
new kill_post_node_4.patch6.32 KB

patch re-rolled and re-tested.

Note - be sure to enable/disable a module after applying this patch to get a menu rebuild.

dmitrig01’s picture

dmitrig01 commented
Status: Needs review » Reviewed & tested by the community

Tested - got HEAD from CVS, applied the patch, installed, enabled a module (aggregator, at random) and created five nodes (two stories and three pages).
Deleted a page, that worked. Node operations worked (selecting one, promoting it, refining, demoting another page). The node deletion operation worked as well. It's RTBC. (It actually fixes a bug with multiple clicks - you click delete multiple times on the confirm form for multiple deletes, and it detects it as an illegal choice

dmitrig01’s picture

dmitrig01 commented

chx, in IRC:

[3:40pm] <chx> first click deletes node then it's illegal click
dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Tested, reviewed and committed. Thanks! :)

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)