Contrib.social

Does ReverseProxyHeaderClientIpRestore need the GuzzleHttp Client?

Open on Drupal.org →
Created on 14 July 2021, over 3 years ago
Updated 7 July 2023, over 1 year ago

Problem/Motivation

Looks like there may be some leftover copy-paste in the event subscriber?

Steps to reproduce


namespace Drupal\reverse_proxy_header\EventSubscriber;

use Drupal\Core\Site\Settings;
use GuzzleHttp\ClientInterface;
use Psr\Log\LoggerInterface;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpKernel\Event\RequestEvent;
use Symfony\Component\HttpKernel\KernelEvents;

/**
 * Restores the true client IP address.
 */
class ReverseProxyHeaderClientIpRestore implements EventSubscriberInterface {

  /**
   * The HTTP client to fetch the feed data with.
   *
   * @var \GuzzleHttp\ClientInterface
   */
  protected $httpClient;

Proposed resolution

Remove the references to the Guzzle http client if they're not necessary.

Remaining tasks

commit a fix etc..

User interface changes

n/a

API changes

n/a

Data model changes

n/a

🐛 Bug report
Status

Needs review

Version

1.0

Component

Code

Created by

🇬🇧United Kingdom mcdruid 🇬🇧🇪🇺

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment over 1 year ago →
    🇺🇦Ukraine bohart Lutsk, Ukraine

    Hi there,
    It looks like there is no real need to have a dependency on GuzzleHttp\ClientInterface and we can safely remove everything related to it.

    The merge request for this fix is always welcome here.
    Thanks!

  • Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.3 & MySQL 5.7
    last update over 1 year ago
    Not currently mergeable.
  • @mcdruid opened merge request.
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.3 & MySQL 5.7
    last update over 1 year ago
    2 fail
  • Comment over 1 year ago →
    🇺🇦Ukraine bohart Lutsk, Ukraine

    It looks like:
    1) ReverseProxyHeaderClientIpRestore::__construct should be updated as well.
    2) ReverseProxyHeaderTestBase should be accordingly updated.
    Thanks && looking forward!

  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.3 & MySQL 5.7
    last update over 1 year ago
    8 pass
  • Status changed to Needs review over 1 year ago
  • Comment over 1 year ago →
    🇬🇧United Kingdom mcdruid 🇬🇧🇪🇺

    IIRC changing a constructor like this in core would mean adding deprecation warnings etc.. but as far as I know that's not necessary here.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024