Commit 0a392098 authored by Matthew Radcliffe's avatar Matthew Radcliffe
Browse files

fix: improve determining when to crawl external URL

By: mradcliffe
By: marcus_johansson
By: greggles
By: akalata
parent af54ac39
Loading
Loading
Loading
Loading
Loading
+4 −4
Changes for phpstan.neon.dist: 4 added lines, 4 removed lines.
Original line number Diff line number Diff line
@@ -8,10 +8,10 @@ parameters:
  paths:
    - .
  excludePaths:
    - config
    - coverage
    - templates
    - help
    - config/*
    - coverage/*
    - templates/*
    - help/*
  # Uncomment the below lines when copying this file as phpstan.neon, and adjust
  # the bootstrap file location based on where this module was installed.
  #bootstrapFiles:
+28 −5
Changes for src/Plugin/freelinking/External.php: 28 added lines, 5 removed lines.
Original line number Diff line number Diff line
@@ -20,7 +20,7 @@ use Symfony\Component\DependencyInjection\ContainerInterface;
#[Freelinking(
  id: 'external',
  title: new TranslatableMarkup('External links'),
  settings: ['scrape' => 1],
  settings: ['scrape' => 0],
)]
class External extends FreelinkingPluginBase implements ContainerFactoryPluginInterface {

@@ -78,12 +78,12 @@ class External extends FreelinkingPluginBase implements ContainerFactoryPluginIn
    $element['scrape'] = [
      '#type' => 'radios',
      '#title' => $this->t('Scrape external URLs'),
      '#description' => $this->t('Should freelinking try to scrape external URLs?'),
      '#description' => $this->t('Should freelinking try to scrape external URLs? Warning: this will expose the page title of remote web pages accessible to your web server.'),
      '#options' => [
        '0' => $this->t('No'),
        '1' => $this->t('Yes'),
      ],
      '#default_value' => $settings['scrape'] ?? '1',
      '#default_value' => $settings['scrape'] ?? '0',
    ];
    return $element;
  }
@@ -108,7 +108,7 @@ class External extends FreelinkingPluginBase implements ContainerFactoryPluginIn
    ];

    // Get the page title from the external URL or use the target text.
    if (!$target['text'] && $scrape) {
    if (!$target['text'] && $scrape && $this->shouldCrawlUrl($url)) {
      try {
        $page_title = $this->getPageTitle($url);
        if ($page_title) {
@@ -124,7 +124,7 @@ class External extends FreelinkingPluginBase implements ContainerFactoryPluginIn
          '#plugin' => 'external',
        ];

        if ($e->getResponse()->getStatusCode() >= 400) {
        if ($e->hasResponse() && $e->getResponse()->getStatusCode() >= 400) {
          $link['#message'] = $this->t('External target “@url” not found', ['@url' => $url]);
        }
      }
@@ -136,6 +136,29 @@ class External extends FreelinkingPluginBase implements ContainerFactoryPluginIn
    return $link;
  }

  /**
   * Determines if the URL is allowed to be crawled.
   *
   * @todo Allow this to be configurable moving the default deny hosts into
   *       default configuration.
   *
   * @param string $url
   *   The URL that will be linked.
   *
   * @return bool
   *   TRUE if the URL is allowed to be crawled for the page title.
   */
  protected function shouldCrawlUrl(string $url): bool {
    $host = parse_url($url, PHP_URL_HOST);
    if ($host) {
      if (preg_match("/[a-zA-Z]/", $host) && $host !== 'localhost') {
        return TRUE;
      }
      return (bool) filter_var($host, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE);
    }
    return FALSE;
  }

  /**
   * Get the page title by fetching from the external URL.
   *
+144 −0
Changes for tests/src/Unit/Plugin/freelinking/ExternalHostTest.php: 144 added lines, 0 removed lines.
Original line number Diff line number Diff line
<?php

namespace Drupal\Tests\freelinking\Unit\Plugin\freelinking;

use Drupal\Core\DependencyInjection\ContainerBuilder;
use Drupal\Core\StringTranslation\TranslatableMarkup;
use Drupal\Core\Url;
use Drupal\freelinking\Plugin\freelinking\External;
use Drupal\Tests\UnitTestCase;
use GuzzleHttp\Client;
use Prophecy\Argument;
use Prophecy\PhpUnit\ProphecyTrait;

/**
 * Tests denied URLs for crawling.
 *
 * @group freelinking
 */
class ExternalHostTest extends UnitTestCase {

  use ProphecyTrait;

  /**
   * The container.
   *
   * @var \Symfony\Component\DependencyInjection\ContainerInterface
   */
  protected $container;

  /**
   * String translation mock.
   *
   * @var \Drupal\Core\StringTranslation\TranslationInterface
   */
  protected $translationInterfaceMock;

  /**
   * {@inheritdoc}
   */
  protected function setUp(): void {
    parent::setUp();
    // Mock string translation service.
    $tProphet = $this->prophesize('\Drupal\Core\StringTranslation\TranslationInterface');
    $tProphet->translateString(Argument::any())->willReturn('Click to visit an external URL.');
    $this->translationInterfaceMock = $tProphet->reveal();

    // Mock module handler service.
    $moduleHandlerProphet = $this->prophesize('\Drupal\Core\Extension\ModuleHandlerInterface');
    $moduleHandlerProphet->moduleExists('search')->willReturn(FALSE);

    $httpClient = $this->getMockBuilder(Client::class)
      ->getMock();
    $httpClient->expects($this->never())->method('get');

    $this->container = new ContainerBuilder();
    $this->container->set('string_translation', $tProphet->reveal());
    $this->container->set('module_handler', $moduleHandlerProphet->reveal());
    $this->container->set('http_client', $httpClient);
    \Drupal::setContainer($this->container);
  }

  /**
   * Asserts that buildLink returns appropriate render array.
   *
   * @param string $expectedText
   *   The expected text of the link.
   * @param string $indicator
   *   The freelinking indacotr to use.
   * @param string $dest
   *   The target destination.
   * @param string $text
   *   The target text, if provided.
   *
   * @dataProvider provideTargetValues
   */
  public function testBuildLink(string $expectedText, string $indicator, string $dest, string $text): void {
    $configuration = ['settings' => ['scrape' => 1]];
    $plugin_definition = [
      'id' => 'external',
      'title' => 'External links',
      'hidden' => FALSE,
      'weight' => 0,
      'settings' => ['scrape' => 1],
    ];
    $plugin = External::create(
      $this->container,
      $configuration,
      'external',
      $plugin_definition,
    );

    $target = [
      'dest' => $dest,
      'indicator' => $indicator,
      'language' => NULL,
      'text' => $text,
    ];
    $scheme = preg_match('/^http(s)?$/', $target['indicator']) === 1 ? $target['indicator'] . ':' : '';
    $url = $scheme . $target['dest'];
    $expected = [
      '#type' => 'link',
      '#title' => $expectedText,
      '#url' => Url::fromUri($url, ['absolute' => TRUE, 'language' => NULL]),
      '#attributes' => [
        'title' => new TranslatableMarkup(
          'Click to visit an external URL.',
          [],
          [],
          $this->translationInterfaceMock,
        ),
      ],
    ];

    // Asserts the expected render array.
    $this->assertEquals($expected, $plugin->buildLink($target));
  }

  /**
   * Provides freelinking target values.
   *
   * @return array
   *   An array of test cases with 4 test arguments:
   *     - The expected title
   *     - The target indicator
   *     - The target destination
   *     - The target text
   */
  public static function provideTargetValues(): array {
    return [
      ['//127.0.0.1', 'http', '//127.0.0.1', ''],
      ['//[::1]', 'http', '//[::1]', ''],
      ['//172.16.0.13', 'http', '//172.16.0.13', ''],
      ['//localhost', 'http', '//localhost', ''],
      ['//127.0.0.1:8888', 'http', '//127.0.0.1:8888', ''],
      ['//[::1]:9000', 'http', '//[::1]:9000', ''],
      ['http://127.0.0.1', 'ext', 'http://127.0.0.1', ''],
      ['http://127.0.0.1', 'external', 'http://127.0.0.1', ''],
      ['http://127.0.0.1:8888', 'ext', 'http://127.0.0.1:8888', ''],
      ['//127.0.0.1/path/to', 'http', '//127.0.0.1/path/to', ''],
      ['Specified title', 'http', '//127.0.0.1', 'Specified title'],
    ];
  }

}