Issue #1847768 by rootatwc, tim.plunkett, effulgentsia, Crell, Damien...

Issue #1847768 by rootatwc, tim.plunkett, effulgentsia, Crell, Damien Tournoud: Remove ip_address().

core/core.services.yml

@@ -249,6 +249,11 @@ services:
     tags:
       - { name: paramconverter }
     arguments: ['@plugin.manager.entity']
+  reverse_proxy_subscriber:
+    class: Drupal\Core\EventSubscriber\ReverseProxySubscriber
+    tags:
+      - { name: event_subscriber }
+    arguments: ['@settings']
   router_processor_subscriber:
     class: Drupal\Core\EventSubscriber\RouteProcessorSubscriber
     tags:
@@ -350,7 +355,7 @@ services:
     class: Drupal\Core\Transliteration\PHPTransliteration
   flood:
     class: Drupal\Core\Flood\DatabaseBackend
-    arguments: ['@database']
+    arguments: ['@database', '@request']
   plugin.manager.condition:
     class: Drupal\Core\Condition\ConditionManager
     arguments: ['%container.namespaces%']

core/includes/bootstrap.inc

@@ -560,9 +560,10 @@ function config_get_config_directory($type = CONFIG_ACTIVE_DIRECTORY) {
  * return the expected values.
  *
  * Most other parameters do not need to be passed in, but may be necessary in
- * some cases; for example, if Drupal's ip_address() function needs to return
- * anything but the standard localhost value ('127.0.0.1'), the command line
- * script should pass in the desired value via the 'REMOTE_ADDR' key.
+ * some cases; for example, if Drupal::request()->getClientIP()
+ * needs to return anything but the standard localhost value ('127.0.0.1'),
+ * the command line script should pass in the desired value via the
+ * 'REMOTE_ADDR' key.
  *
  * @param $variables
  *   (optional) An associative array of variables within $_SERVER that should
@@ -573,7 +574,7 @@ function config_get_config_directory($type = CONFIG_ACTIVE_DIRECTORY) {
  *
  * @see conf_path()
  * @see request_uri()
- * @see ip_address()
+ * @see \Symfony\Component\HttpFoundation\Request::getClientIP()
  */
 function drupal_override_server_variables($variables = array()) {
   // Allow the provided URL to override any existing values in $_SERVER.
@@ -1739,7 +1740,7 @@ function watchdog($type, $message, array $variables = NULL, $severity = WATCHDOG
       'uid'         => $user_uid,
       'request_uri' => $base_root . request_uri(),
       'referer'     => isset($_SERVER['HTTP_REFERER']) ? $_SERVER['HTTP_REFERER'] : '',
-      'ip'          => ip_address(),
+      'ip'          => Drupal::request()->getClientIP(),
       // Request time isn't accurate for long processes, use time() instead.
       'timestamp'   => time(),
     );
@@ -2003,7 +2004,7 @@ function drupal_hash_base64($data) {
 function drupal_anonymous_user() {
   $values = array(
     'uid' => 0,
-    'hostname' => ip_address(),
+    'hostname' => Drupal::request()->getClientIP(),
     'roles' => array(
       DRUPAL_ANONYMOUS_RID => DRUPAL_ANONYMOUS_RID,
     ),
@@ -3063,52 +3064,6 @@ function arg($index = NULL, $path = NULL) {
   }
 }
 
-/**
- * Returns the IP address of the client machine.
- *
- * If Drupal is behind a reverse proxy, we use the X-Forwarded-For header
- * instead of $_SERVER['REMOTE_ADDR'], which would be the IP address of
- * the proxy server, and not the client's. The actual header name can be
- * configured by the reverse_proxy_header variable.
- *
- * @return
- *   IP address of client machine, adjusted for reverse proxy and/or cluster
- *   environments.
- */
-function ip_address() {
-  $ip_address = &drupal_static(__FUNCTION__);
-
-  if (!isset($ip_address)) {
-    $ip_address = $_SERVER['REMOTE_ADDR'];
-
-    if (settings()->get('reverse_proxy', 0)) {
-      $reverse_proxy_header = settings()->get('reverse_proxy_header', 'HTTP_X_FORWARDED_FOR');
-      if (!empty($_SERVER[$reverse_proxy_header])) {
-        // If an array of known reverse proxy IPs is provided, then trust
-        // the XFF header if request really comes from one of them.
-        $reverse_proxy_addresses = settings()->get('reverse_proxy_addresses', array());
-
-        // Turn XFF header into an array.
-        $forwarded = explode(',', $_SERVER[$reverse_proxy_header]);
-
-        // Trim the forwarded IPs; they may have been delimited by commas and spaces.
-        $forwarded = array_map('trim', $forwarded);
-
-        // Tack direct client IP onto end of forwarded array.
-        $forwarded[] = $ip_address;
-
-        // Eliminate all trusted IPs.
-        $untrusted = array_diff($forwarded, $reverse_proxy_addresses);
-
-        // The right-most IP is the most specific we can trust.
-        $ip_address = array_pop($untrusted);
-      }
-    }
-  }
-
-  return $ip_address;
-}
-
 /**
  * Initializes and returns the class loader.
  *

core/includes/install.core.inc

@@ -331,6 +331,10 @@ function install_begin_request(&$install_state) {
   if ($install_state['settings_verified']) {
     $kernel = new DrupalKernel('install', FALSE, drupal_classloader(), FALSE);
     $kernel->boot();
+    // Set the request in the kernel to the new created Request above
+    // so it is available to the rest of the installation process.
+    $kernel->getContainer()
+      ->set('request', $request);
   }
   else {
     // @todo Move into a proper Drupal\Core\DependencyInjection\InstallContainerBuilder.

core/includes/session.inc

@@ -173,7 +173,7 @@ function _drupal_session_write($sid, $value) {
       // Either ssid or sid or both will be added from $key below.
       $fields = array(
         'uid' => $user->uid,
-        'hostname' => ip_address(),
+        'hostname' => Drupal::request()->getClientIP(),
         'session' => $value,
         'timestamp' => REQUEST_TIME,
       );

core/lib/Drupal/Core/DrupalKernel.php

@@ -284,6 +284,11 @@ protected function getClassName() {
    */
   protected function initializeContainer() {
     $persist = $this->getServicesToPersist();
+    // If we are rebuilding the kernel and we are in a request scope, store
+    // request info so we can add them back after the rebuild.
+    if (isset($this->container) && $this->container->hasScope('request')) {
+      $request = $this->container->get('request');
+    }
     $this->container = NULL;
     $class = $this->getClassName();
     $cache_file = $class . '.php';
@@ -345,7 +350,11 @@ protected function initializeContainer() {
     $this->container->set('kernel', $this);
     // Set the class loader which was registered as a synthetic service.
     $this->container->set('class_loader', $this->classLoader);
-
+    // If we have a request set it back to the new container.
+    if (isset($request)) {
+      $this->container->enterScope('request');
+      $this->container->set('request', $request);
+    }
     \Drupal::setContainer($this->container);
   }
 

core/lib/Drupal/Core/EventSubscriber/ReverseProxySubscriber.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\EventSubscriber\ReverseProxySubscriber.
+ */
+
+namespace Drupal\Core\EventSubscriber;
+
+use Symfony\Component\HttpKernel\KernelEvents;
+use Symfony\Component\HttpKernel\Event\GetResponseEvent;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+use Drupal\Component\Utility\Settings;
+
+/**
+ * Reverse proxy subscriber for controller requests.
+ */
+class ReverseProxySubscriber implements EventSubscriberInterface {
+
+  /**
+   * A settings object.
+   *
+   * @var \Drupal\Component\Utility\Settings
+   */
+  protected $settings;
+
+  /**
+   * Construct the ReverseProxySubscriber.
+   *
+   * @param \Drupal\Component\Utility\Settings $settings
+   *   The read-only settings object of this request.
+   */
+  public function __construct(Settings $settings) {
+    $this->settings = $settings;
+  }
+
+  /**
+   * Passes reverse proxy settings to current request.
+   *
+   * @param \Symfony\Component\HttpKernel\Event\GetResponseEvent $event
+   *   The Event to process.
+   */
+  public function onKernelRequestReverseProxyCheck(GetResponseEvent $event) {
+    $request = $event->getRequest();
+    if ($this->settings->get('reverse_proxy', 0)) {
+      $reverse_proxy_header = $this->settings->get('reverse_proxy_header', 'HTTP_X_FORWARDED_FOR');
+      $request::setTrustedHeaderName($request::HEADER_CLIENT_IP, $reverse_proxy_header);
+      $reverse_proxy_addresses = $this->settings->get('reverse_proxy_addresses', array());
+      $request::setTrustedProxies($reverse_proxy_addresses);
+    }
+  }
+
+  /**
+   * Registers the methods in this class that should be listeners.
+   *
+   * @return array
+   *   An array of event listener definitions.
+   */
+  static function getSubscribedEvents() {
+    $events[KernelEvents::REQUEST][] = array('onKernelRequestReverseProxyCheck', 10);
+    return $events;
+  }
+}

core/lib/Drupal/Core/Flood/DatabaseBackend.php

@@ -7,6 +7,7 @@
 
 namespace Drupal\Core\Flood;
 
+use Symfony\Component\HttpFoundation\Request;
 use Drupal\Core\Database\Connection;
 
 /**
@@ -21,15 +22,25 @@ class DatabaseBackend implements FloodInterface {
    */
   protected $connection;
 
+  /**
+   * A request object.
+   *
+   * @var \Symfony\Component\HttpFoundation\Request
+   */
+  protected $request;
+
   /**
    * Construct the DatabaseBackend.
    *
    * @param \Drupal\Core\Database\Connection $connection
    *   The database connection which will be used to store the flood event
    *   information.
+   * @param \Symfony\Component\HttpFoundation\Request $request
+   *   The HttpRequest object representing the current request.
    */
-  public function __construct(Connection $connection) {
+  public function __construct(Connection $connection, Request $request) {
     $this->connection = $connection;
+    $this->request = $request;
   }
 
   /**
@@ -37,7 +48,7 @@ public function __construct(Connection $connection) {
    */
   public function register($name, $window = 3600, $identifier = NULL) {
     if (!isset($identifier)) {
-      $identifier = ip_address();
+      $identifier = $this->request->getClientIp();
     }
     $this->connection->insert('flood')
       ->fields(array(
@@ -54,7 +65,7 @@ public function register($name, $window = 3600, $identifier = NULL) {
    */
   public function clear($name, $identifier = NULL) {
     if (!isset($identifier)) {
-      $identifier = ip_address();
+      $identifier = $this->request->getClientIp();
     }
     $this->connection->delete('flood')
       ->condition('event', $name)
@@ -67,7 +78,7 @@ public function clear($name, $identifier = NULL) {
    */
   public function isAllowed($name, $threshold, $window = 3600, $identifier = NULL) {
     if (!isset($identifier)) {
-      $identifier = ip_address();
+      $identifier = $this->request->getClientIp();
     }
     $number = $this->connection->select('flood', 'f')
       ->condition('event', $name)

core/lib/Drupal/Core/Flood/MemoryBackend.php

@@ -7,22 +7,41 @@
 
 namespace Drupal\Core\Flood;
 
+use Symfony\Component\HttpFoundation\Request;
+
 /**
  * Defines the memory flood backend. This is used for testing.
  */
 class MemoryBackend implements FloodInterface {
 
+  /**
+   * A request object.
+   *
+   * @var \Symfony\Component\HttpFoundation\Request
+   */
+  protected $request;
+
   /**
    * An array holding flood events, keyed by event name and identifier.
    */
   protected $events = array();
 
+  /**
+   * Construct the MemoryBackend.
+   *
+   * @param \Symfony\Component\HttpFoundation\Request $request
+   *   The HttpRequest object representing the current request.
+   */
+  public function __construct(Request $request) {
+    $this->request = $request;
+  }
+
   /**
    * Implements Drupal\Core\Flood\FloodInterface::register().
    */
   public function register($name, $window = 3600, $identifier = NULL) {
     if (!isset($identifier)) {
-      $identifier = ip_address();
+      $identifier = $this->request->getClientIP();
     }
     // We can't use REQUEST_TIME here, because that would not guarantee
     // uniqueness.
@@ -35,7 +54,7 @@ public function register($name, $window = 3600, $identifier = NULL) {
    */
   public function clear($name, $identifier = NULL) {
     if (!isset($identifier)) {
-      $identifier = ip_address();
+      $identifier = $this->request->getClientIP();
     }
     unset($this->events[$name][$identifier]);
   }
@@ -45,7 +64,7 @@ public function clear($name, $identifier = NULL) {
    */
   public function isAllowed($name, $threshold, $window = 3600, $identifier = NULL) {
     if (!isset($identifier)) {
-      $identifier = ip_address();
+      $identifier = $this->request->getClientIP();
     }
     $limit = microtime(true) - $window;
     $number = count(array_filter($this->events[$name][$identifier], function ($timestamp) use ($limit) {

core/modules/action/tests/action_loop_test/action_loop_test.module

@@ -69,7 +69,7 @@ function watchdog_skip_semaphore($type, $message, $variables = array(), $severit
     'uid'         => isset($user->uid) ? $user->uid : 0,
     'request_uri' => $base_root . request_uri(),
     'referer'     => $_SERVER['HTTP_REFERER'],
-    'ip'          => ip_address(),
+    'ip'          => Drupal::request()->getClientIP(),
     'timestamp'   => REQUEST_TIME,
   );
 

core/modules/ban/ban.admin.inc

@@ -85,7 +85,7 @@ function ban_ip_form_validate($form, &$form_state) {
   if (db_query("SELECT * FROM {ban_ip} WHERE ip = :ip", array(':ip' => $ip))->fetchField()) {
     form_set_error('ip', t('This IP address is already banned.'));
   }
-  elseif ($ip == ip_address()) {
+  elseif ($ip == Drupal::request()->getClientIP()) {
     form_set_error('ip', t('You may not ban your own IP address.'));
   }
   elseif (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_NO_RES_RANGE) == FALSE) {

core/modules/ban/lib/Drupal/ban/EventSubscriber/BanSubscriber.php

@@ -43,8 +43,7 @@ public function __construct(BanIpManager $manager) {
    *   The Event to process.
    */
   public function onKernelRequestBannedIpCheck(GetResponseEvent $event) {
-    // @todo convert this to Request::getClientIP().
-    $ip = ip_address();
+    $ip = $event->getRequest()->getClientIp();
     if ($this->manager->isDenied($ip)) {
       $response = new Response('Sorry, ' . check_plain($ip) . ' has been banned.', 403);
       $event->setResponse($response);

core/modules/ban/lib/Drupal/ban/Tests/IpAddressBlockingTest.php

@@ -78,7 +78,7 @@ function testIPAddressValidation() {
     // manually.
     // TODO: On some systems this test fails due to a bug/inconsistency in cURL.
     // $edit = array();
-    // $edit['ip'] = ip_address();
+    // $edit['ip'] = \Drupal::request()->getClientIP();
     // $this->drupalPost('admin/config/people/ban', $edit, t('Save'));
     // $this->assertText(t('You may not ban your own IP address.'));
   }

core/modules/comment/lib/Drupal/comment/CommentStorageController.php

@@ -153,7 +153,7 @@ protected function preSave(EntityInterface $comment) {
       }
       // Add the values which aren't passed into the function.
       $comment->thread->value = $thread;
-      $comment->hostname->value = ip_address();
+      $comment->hostname->value = \Drupal::request()->getClientIP();
     }
   }
 

core/modules/comment/lib/Drupal/comment/Tests/CommentLinksTest.php

@@ -145,7 +145,7 @@ function setEnvironment(array $info) {
           'uid' => 0,
           'status' => COMMENT_PUBLISHED,
           'subject' => $this->randomName(),
-          'hostname' => ip_address(),
+          'hostname' => '127.0.0.1',
           'langcode' => LANGUAGE_NOT_SPECIFIED,
           'comment_body' => array(LANGUAGE_NOT_SPECIFIED => array($this->randomName())),
         ));

core/modules/comment/lib/Drupal/comment/Tests/CommentNewIndicatorTest.php

@@ -52,7 +52,7 @@ public function testCommentNewCommentsIndicator() {
       'uid' => $this->loggedInUser->uid,
       'status' => COMMENT_PUBLISHED,
       'subject' => $this->randomName(),
-      'hostname' => ip_address(),
+      'hostname' => '127.0.0.1',
       'langcode' => LANGUAGE_NOT_SPECIFIED,
       'comment_body' => array(LANGUAGE_NOT_SPECIFIED => array($this->randomName())),
     ));

core/modules/config/config.admin.inc

@@ -182,8 +182,8 @@ function config_admin_diff_page($config_file) {
 
   // Return AJAX requests as a dialog.
   // @todo: Set up separate content callbacks for the non-JS and dialog versions
-  // of this page using the router system. See http://drupal.org/node/1944472. 
-  if (Drupal::service('request')->isXmlHttpRequest()) {
+  // of this page using the router system. See http://drupal.org/node/1944472.
+  if (Drupal::request()->isXmlHttpRequest()) {
     // Add class to the close link.
     $output['back']['#attributes']['class'][] = 'dialog-cancel';
 

core/modules/dblog/lib/Drupal/dblog/Tests/DBLogTest.php

@@ -137,7 +137,7 @@ private function generateLogEntries($count, $type = 'custom', $severity = WATCHD
       'uid'         => isset($this->big_user->uid) ? $this->big_user->uid : 0,
       'request_uri' => $base_root . request_uri(),
       'referer'     => $_SERVER['HTTP_REFERER'],
-      'ip'          => ip_address(),
+      'ip'          => '127.0.0.1',
       'timestamp'   => REQUEST_TIME,
       );
     $message = 'Log entry added to test the dblog row limit. Entry #';
@@ -424,7 +424,7 @@ protected function testDBLogAddAndClear() {
       'uid'         => isset($this->big_user->uid) ? $this->big_user->uid : 0,
       'request_uri' => $base_root . request_uri(),
       'referer'     => $_SERVER['HTTP_REFERER'],
-      'ip'          => ip_address(),
+      'ip'          => '127.0.0.1',
       'timestamp'   => REQUEST_TIME,
     );
     // Add a watchdog entry.

core/modules/openid/openid.module

@@ -1066,7 +1066,7 @@ function openid_verify_assertion_nonce($service, $response) {
     return TRUE;
   }
   else {
-    watchdog('openid', 'Nonce replay attempt blocked from @ip, nonce: @nonce.', array('@ip' => ip_address(), '@nonce' => $response['openid.response_nonce']), WATCHDOG_CRITICAL);
+    watchdog('openid', 'Nonce replay attempt blocked from @ip, nonce: @nonce.', array('@ip' => Drupal::request()->getClientIP(), '@nonce' => $response['openid.response_nonce']), WATCHDOG_CRITICAL);
     return FALSE;
   }
 }

core/modules/system/lib/Drupal/system/Tests/Bootstrap/IpAddressTest.php

-<?php
-
-/**
- * @file
- * Definition of Drupal\system\Tests\Bootstrap\IpAddressTest.
- */
-
-namespace Drupal\system\Tests\Bootstrap;
-
-use Drupal\simpletest\WebTestBase;
-
-/**
- * Tests getting IP addresses and hostname validation.
- */
-class IpAddressTest extends WebTestBase {
-
-  public static function getInfo() {
-    return array(
-      'name' => 'IP address and HTTP_HOST test',
-      'description' => 'Get the IP address from the current visitor from the server variables, check hostname validation.',
-      'group' => 'Bootstrap'
-    );
-  }
-
-  function setUp() {
-    $this->oldserver = $_SERVER;
-
-    $this->remote_ip = '127.0.0.1';
-    $this->proxy_ip = '127.0.0.2';
-    $this->proxy2_ip = '127.0.0.3';
-    $this->forwarded_ip = '127.0.0.4';
-    $this->cluster_ip = '127.0.0.5';
-    $this->untrusted_ip = '0.0.0.0';
-
-    drupal_static_reset('ip_address');
-
-    $_SERVER['REMOTE_ADDR'] = $this->remote_ip;
-    unset($_SERVER['HTTP_X_FORWARDED_FOR']);
-    unset($_SERVER['HTTP_X_CLUSTER_CLIENT_IP']);
-
-    parent::setUp();
-  }
-
-  function tearDown() {
-    $_SERVER = $this->oldserver;
-    drupal_static_reset('ip_address');
-    parent::tearDown();
-  }
-
-  /**
-   * Tests IP address and hostname.
-   */
-  function testIPAddressHost() {
-    // Test the normal IP address.
-    $this->assertTrue(
-      ip_address() == $this->remote_ip,
-      'Got remote IP address.'
-    );
-
-    // Proxy forwarding on but no proxy addresses defined.
-    $this->settingsSet('reverse_proxy',  1);
-    $this->assertTrue(
-      ip_address() == $this->remote_ip,
-      'Proxy forwarding without trusted proxies got remote IP address.'
-    );
-
-    // Proxy forwarding on and proxy address not trusted.
-    $this->settingsSet('reverse_proxy_addresses', array($this->proxy_ip, $this->proxy2_ip));
-    drupal_static_reset('ip_address');
-    $_SERVER['REMOTE_ADDR'] = $this->untrusted_ip;
-    $this->assertTrue(
-      ip_address() == $this->untrusted_ip,
-      'Proxy forwarding with untrusted proxy got remote IP address.'
-    );
-
-    // Proxy forwarding on and proxy address trusted.
-    $_SERVER['REMOTE_ADDR'] = $this->proxy_ip;
-    $_SERVER['HTTP_X_FORWARDED_FOR'] = $this->forwarded_ip;
-    drupal_static_reset('ip_address');
-    $this->assertTrue(
-      ip_address() == $this->forwarded_ip,
-      'Proxy forwarding with trusted proxy got forwarded IP address.'
-    );
-
-    // Multi-tier architecture with comma separated values in header.
-    $_SERVER['REMOTE_ADDR'] = $this->proxy_ip;
-    $_SERVER['HTTP_X_FORWARDED_FOR'] = implode(', ', array($this->untrusted_ip, $this->forwarded_ip, $this->proxy2_ip));
-    drupal_static_reset('ip_address');
-    $this->assertTrue(
-      ip_address() == $this->forwarded_ip,
-      'Proxy forwarding with trusted 2-tier proxy got forwarded IP address.'
-    );
-
-    // Custom client-IP header.
-    $this->settingsSet('reverse_proxy_header', 'HTTP_X_CLUSTER_CLIENT_IP');
-    $_SERVER['HTTP_X_CLUSTER_CLIENT_IP'] = $this->cluster_ip;
-    drupal_static_reset('ip_address');
-    $this->assertTrue(
-      ip_address() == $this->cluster_ip,
-      'Cluster environment got cluster client IP.'
-    );
-
-    // Verifies that drupal_valid_http_host() prevents invalid characters.
-    $this->assertFalse(drupal_valid_http_host('security/.drupal.org:80'), 'HTTP_HOST with / is invalid');
-    $this->assertFalse(drupal_valid_http_host('security\\.drupal.org:80'), 'HTTP_HOST with \\ is invalid');
-    $this->assertFalse(drupal_valid_http_host('security<.drupal.org:80'), 'HTTP_HOST with &lt; is invalid');
-    $this->assertFalse(drupal_valid_http_host('security..drupal.org:80'), 'HTTP_HOST with .. is invalid');
-    // IPv6 loopback address
-    $this->assertTrue(drupal_valid_http_host('[::1]:80'), 'HTTP_HOST containing IPv6 loopback is valid');
-  }
-}

core/modules/system/lib/Drupal/system/Tests/System/FloodTest.php

@@ -8,11 +8,20 @@
 namespace Drupal\system\Tests\System;
 
 use Drupal\simpletest\WebTestBase;
+use Symfony\Component\HttpFoundation\Request;
 
 /**
  * Functional tests for the flood control mechanism.
  */
 class FloodTest extends WebTestBase {
+
+  /**
+   * The Request object that flood classes should use.
+   *
+   * @var \Symfony\Component\HttpFoundation\Request
+   */
+  protected $request;
+
   public static function getInfo() {
     return array(
       'name' => 'Flood control mechanism',
@@ -21,6 +30,16 @@ public static function getInfo() {
     );
   }
 
+  public function setUp() {
+    parent::setUp();
+
+    // Flood backends need a request object. Create a dummy one and insert it
+