Part of #1998638: Replace almost all remaining superglobals ($_GET, $_POST, etc.) with Symfony Request object

Files needing conversion:

  • core/modules/block/block.module
  • core/modules/block/custom_block/lib/Drupal/custom_block/CustomBlockFormController.php
  • core/modules/book/book.module
Files: 
CommentFileSizeAuthor
#8 1998708-block-request-8.patch2.42 KBkim.pepper
PASSED: [[SimpleTest]]: [MySQL] 57,450 pass(es).
[ View ]
#4 1998708-block-request-4.patch19.25 KBkim.pepper
PASSED: [[SimpleTest]]: [MySQL] 55,358 pass(es).
[ View ]
#1 1998708_1.patch2.46 KBkmcculloch
PASSED: [[SimpleTest]]: [MySQL] 55,822 pass(es).
[ View ]

Comments

kmcculloch’s picture

Status:Active» Needs review
StatusFileSize
new2.46 KB
PASSED: [[SimpleTest]]: [MySQL] 55,822 pass(es).
[ View ]

Initial attempt to convert, using (what I believe are) proper Symfony methods

kim.pepper’s picture

+++ b/core/modules/block/block.moduleundefined
@@ -315,7 +315,7 @@ function _block_get_renderable_region($list = array()) {
+    !in_array(\Drupal::request()->server->get('REQUEST_METHOD'), array('GET', 'HEAD'));

This could just be $request->isMethod('GET') || $request->isMethod('HEAD')

kim.pepper’s picture

Status:Needs review» Needs work
kim.pepper’s picture

Status:Needs work» Needs review
StatusFileSize
new19.25 KB
PASSED: [[SimpleTest]]: [MySQL] 55,358 pass(es).
[ View ]

There is now only one occurrence of PHP superglobals, so I started this again from scratch. I've used a $request-isMethodSafe() to encapsulate the same check.

Status:Needs review» Needs work

The last submitted patch, 1998708-block-request-4.patch, failed testing.

Steven Merrill’s picture

Status:Needs work» Needs review

#4: 1998708-block-request-4.patch queued for re-testing.

Crell’s picture

Status:Needs review» Needs work
+++ b/core/includes/menu.inc
@@ -917,7 +926,8 @@ function _menu_link_translate(&$item, $translate = FALSE) {
+        $item['access'] = drupal_container()->get('access_manager')->check($route, $request);

This should be Drupal::service('access_manager')

There's also a ton of unrelated config system stuff going on in there. Looks like a patch got mixed up by accident.

kim.pepper’s picture

Status:Needs work» Needs review
StatusFileSize
new2.42 KB
PASSED: [[SimpleTest]]: [MySQL] 57,450 pass(es).
[ View ]

Looks like a bad rebase. Trying again...

Crell’s picture

Status:Needs review» Reviewed & tested by the community
Issue tags:+Quick fix

That's more like it. :-)

alexpott’s picture

Status:Reviewed & tested by the community» Fixed

Committed 76518e6 and pushed to 8.x. Thanks!

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