This is a kernel-patch follow-up.

drupal_access_denied() is now a trivial wrapper around throwing Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException. Let's eliminate the middle man.

All instances of drupal_access_denied() should be replaced with throwing the appropriate exception, or else refactored so that the "denied" logic happens further up in the routing layer where it belongs.

Files: 
CommentFileSizeAuthor
#19 core-replace_drupal_access_denied-1591604-17.patch10.97 KBunderq
PASSED: [[SimpleTest]]: [MySQL] 36,699 pass(es).
[ View ]
#17 core_replace_drupal_access_denied-1591604-17.patch11.17 KBchrisdolby
PASSED: [[SimpleTest]]: [MySQL] 36,712 pass(es).
[ View ]
#15 core_replace_drupal_access_denied-1591604-15.patch8.63 KBchrisdolby
PASSED: [[SimpleTest]]: [MySQL] 36,699 pass(es).
[ View ]
#12 1591604-12-core-replace_drupal_access_denied.patch12.31 KBpfrenssen
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1591604-12-core-replace_drupal_access_denied.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#8 core-replace_drupal_access_denied-1591604-8.patch6.73 KBAlbert Volkman
PASSED: [[SimpleTest]]: [MySQL] 36,684 pass(es).
[ View ]
#6 core-replace_drupal_access_denied-1591604-6.patch7.66 KBunderq
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch core-replace_drupal_access_denied-1591604-6.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#4 core-replace_drupal_access_denied-1591604-4.patch5.92 KBunderq
FAILED: [[SimpleTest]]: [MySQL] 36,664 pass(es), 9 fail(s), and 0 exception(s).
[ View ]

Comments

Title:Replace drupal_access_denied() with throw Replace drupal_access_denied() with throw AccessDeniedHttpException

Priority:Normal» Major

Issue tags:+Novice

This should be fairly routine.

Status:Active» Needs review
StatusFileSize
new5.92 KB
FAILED: [[SimpleTest]]: [MySQL] 36,664 pass(es), 9 fail(s), and 0 exception(s).
[ View ]

Patch done, need review ;-)

Status:Needs review» Needs work

The last submitted patch, core-replace_drupal_access_denied-1591604-4.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new7.66 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch core-replace_drupal_access_denied-1591604-6.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Add

<?php
use SymfonyComponentHttpKernelExceptionAccessDeniedHttpException;
?>
/code>

Status:Needs review» Needs work

The last submitted patch, core-replace_drupal_access_denied-1591604-6.patch, failed testing.

StatusFileSize
new6.73 KB
PASSED: [[SimpleTest]]: [MySQL] 36,684 pass(es).
[ View ]

Re-roll.

Status:Needs work» Needs review

Status:Needs review» Needs work

The are mentions of drupal_access_denied() scattered across the documentation, please update these too.

For example, in the documentation for drupal_deliver_page():

<?php
/**
* Delivers a page callback result to the browser in the appropriate format.
*
* This function is most commonly called by menu_execute_active_handler(), but
* can also be called by error conditions such as drupal_not_found(),
* drupal_access_denied(), and drupal_site_offline().
*/
?>

Assigned:Unassigned» pfrenssen

I'll do this, have done the same in #1587850: Replace drupal_not_found() with throw NotFoundHttpException. The patches in these two issues will give conflicts anyway since they are affecting the same lines of documentation.

Assigned:pfrenssen» Unassigned
Status:Needs work» Needs review
StatusFileSize
new12.31 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1591604-12-core-replace_drupal_access_denied.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Did the following updates to the patch in #8:

  • Removed the drupal_access_denied() function itself.
  • Updated documentation of drupal_deliver_page(), file_download() and search_box_form_submit().
  • Replaced calls to drupal_acccess_denied() in aggregator_admin_refresh_feed(), comment_approve() and overlay_user_dismiss_message().
  • Removed code that was never going to be executed after throwing the AccessDeniedHttpException in contact_site_form(), contact_personal_form(), menu_delete_menu_page(), menu_item_delete_page()

Status:Needs review» Needs work
Issue tags:+Novice, +WSCCI, +symfony, +kernel-followup

The last submitted patch, 1591604-12-core-replace_drupal_access_denied.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new8.63 KB
PASSED: [[SimpleTest]]: [MySQL] 36,699 pass(es).
[ View ]

I've rebased pfrenssen's patch on the latest head, should hopefully test OK now.

Status:Needs review» Needs work

You forgot to remove the drupal_access_denied() function

Status:Needs work» Needs review
StatusFileSize
new11.17 KB
PASSED: [[SimpleTest]]: [MySQL] 36,712 pass(es).
[ View ]

Sorry, that was the point after all!

There were also some other changes missing. This should cover includes/common.inc, includes/file.inc and modules/aggregator/aggregator.admin.inc, and in the process removing the function.

Status:Needs review» Reviewed & tested by the community

Did a grep, is rtbc if this come back green!

Status:Reviewed & tested by the community» Needs review
StatusFileSize
new10.97 KB
PASSED: [[SimpleTest]]: [MySQL] 36,699 pass(es).
[ View ]

Rebased chrisdolby patch with remove drupal_access_denied(), edit file_download() comment and add AccessDeniedHttpException() in aggregator_admin_refresh_feed() function.

I hope that's good.

Oups you are so fast :)

Status:Needs review» Reviewed & tested by the community

They both are correct, but the patch command they use was different, thats why there is a size change. Btw you can't access the first patch anymore because they used the same name while crossposting, so the second one overwrites the first one.

No, one was core_replace and the other core-replace.

The first patch changes the file permissions which is wrong. The second one (#19) is RTBC.

Woow timplunkett, I didn't notice the subtle permission change in the git headers.
Thank you!

Status:Reviewed & tested by the community» Fixed

Great. Love the clean-ups. Committed to 8.x. Thanks!

Status:Fixed» Reviewed & tested by the community

This and several other issues look like they weren't actually committed/pushed. So, #19 is still RTBC.

Status:Reviewed & tested by the community» Fixed

For anyone who's looking for it, a change notification for this issue was started at http://drupal.org/node/1616360

Automatically closed -- issue fixed for 2 weeks with no activity.