The attached patch addresses 3 issues around the node/%/addanother callback.
We can make use of menu wildcard loaders, in this case %node (which called node_load for us). This also handles if a "bad" node is passed in for us for free (returns a 404 for us).
The goto function can be simplified now it received a node object only. It never seems to be used without node object before.
The access function has been tightened up. There was no check on node_access('update'... so you could potentially, as a non-authorised user, end up getting redirected to a node/add/{type} page by randomly hitting node ID's. Not a critical issue but could be considered information disclosure.
I've tested the patch locally and it seems to work ok - would appreciate feedback/testing from others though :)
Looked over your comments and ways to streamline this and came out with this change set. I'm going to call this issue fixed; feel free to file another with any more changes that could be helpful!
Comments
Comment #1
nicholasthompsonThe attached patch addresses 3 issues around the
node/%/addanothercallback.%node(which called node_load for us). This also handles if a "bad" node is passed in for us for free (returns a 404 for us).node_access('update'...so you could potentially, as a non-authorised user, end up getting redirected to a node/add/{type} page by randomly hitting node ID's. Not a critical issue but could be considered information disclosure.I've tested the patch locally and it seems to work ok - would appreciate feedback/testing from others though :)
Comment #2
robin monks commentedNo patch attached; the Drupal gremlins ate it ;)
Comment #3
robin monks commentedLooked over your comments and ways to streamline this and came out with this change set. I'm going to call this issue fixed; feel free to file another with any more changes that could be helpful!
Thanks!