Contrib.social

Dont use curl, use the drupal client instead

Open on Drupal.org →
Created on 3 January 2017, over 7 years ago
Updated 14 May 2024, about 1 month ago

Firtst off: I love this module!
I have created a plugin where this module fetches remote content from all out local sites (indexed in elasticsearch). This module makes life so much easier!
Thanks.

We use a proxy server in our network and all our outgoing requests should go through that proxy.
Therefor the drupal Client should always be used when doing outgoing requests (or you should implement support for the proxy settings) .
Read more info here: https://www.drupal.org/node/2637058 →

So in Drupal\filefield_sources\Plugin\FilefieldSource\Remote.php line 69 

      $ch = curl_init();
      curl_setopt($ch, CURLOPT_URL, $url);
      curl_setopt($ch, CURLOPT_HEADER, TRUE);
      curl_setopt($ch, CURLOPT_NOBODY, TRUE);
      curl_setopt($ch, CURLOPT_RETURNTRANSFER, TRUE);
      curl_setopt($ch, CURLOPT_HEADERFUNCTION, array(get_called_class(), 'parseHeader'));
      // Causes a warning if PHP safe mode is on.
      @curl_setopt($ch, CURLOPT_FOLLOWLOCATION, TRUE);
      curl_exec($ch);
      $info = curl_getinfo($ch);
      if ($info['http_code'] != 200) {
        curl_setopt($ch, CURLOPT_HTTPGET, TRUE);
        $file_contents = curl_exec($ch);
        $info = curl_getinfo($ch);
      }
      curl_close($ch);

should be replaced by 

$client = \Drupal::httpClient();
try {
  $request = $client->get($url);
  $status = $request->getStatusCode();
  $file_contents = $request->getBody()->getContents();
}
catch (RequestException $e) {
  //An error happened.
}

Also Line 163

        $ch = curl_init();
        curl_setopt($ch, CURLOPT_URL, $url);
        curl_setopt($ch, CURLOPT_HEADER, FALSE);
        curl_setopt($ch, CURLOPT_WRITEFUNCTION, array(get_called_class(), 'curlWrite'));
        // Causes a warning if PHP safe mode is on.
        @curl_setopt($ch, CURLOPT_FOLLOWLOCATION, TRUE);
        $transfer_success = curl_exec($ch);
        curl_close($ch);

Should be replaced by something like

$client = \Drupal::httpClient();
try {
  $request = $client->get($url);
  $status = $request->getStatusCode();
  $transfer_success = $request->getBody()->getContents();
}
catch (RequestException $e) {
  //An error happened.
}

But I don't understand what the CURLOPT_HEADERFUNCTION does.

🐛 Bug report
Status

Needs review

Version

2.0

Component

Source: Remote URL

Created by

🇧🇪Belgium wouters_f Leuven

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

Merge Requests

Comments & Activities

Not all content is available!

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

  • Comment 11 months ago →
    🇮🇹Italy Kris77

    Patch #9 works for me too.
    Thanks @w.drupal

  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.1 & MySQL 8
    last update 6 months ago
    15 pass
  • Comment 6 months ago →
    🇺🇸United States rschwab

    Since php 8 is required for Drupal 10, I'm marking this as a child of 📌 Drupal 10 compatibility Needs work to try and get that ticket completed. This potentially solves the last errors in the D10 test suite.

  • Merge request !8Resolve #2840594 "Dont use curl" → (Open) created by rschwab
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 8
    last update 6 months ago
    15 pass
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.1 & MySQL 8
    last update 6 months ago
    15 pass
  • Status changed to Needs review 6 months ago
  • Comment 6 months ago →
    🇺🇸United States rschwab

    Applied #9 to an issue fork, then removed the legacy curl functions that are no longer in use. Currently passing all tests, but needs community review.

  • Comment 5 months ago →
    🇺🇸United States rschwab
  • Status changed to Needs work 5 months ago
  • Comment 5 months ago →
    🇭🇺Hungary szato

    There is a fatal php error because of duplicate use statements.

    I'm going to rebase the MR8 on 2.0.x and fix these.

  • Status changed to Needs review 5 months ago
  • Comment 5 months ago →
    🇭🇺Hungary szato

    Please review it, now it works ;)

  • Comment 2 months ago →
    🇺🇸United States rschwab

    In case anyone wants a patch to test this, the current MR is attached here in patch format.

  • Comment 2 months ago →
    🇺🇸United States rschwab

    I think we still need WidgetInterface when the function is restored in Remote.php:

    /**
   * Implements hook_filefield_source_settings().
   */
  public static function settings(WidgetInterface $plugin) {

    I'm not sure how it happened, but when I look at this branch Drupal/Field/WidgetInterface is no longer included at all (there were not doubles somehow).

  • Comment about 1 month ago →
    🇫🇷France erwangel

    I tried MR 8 on a Drupal 10.2.6 which applied with no problem on ffs 2.0.x-dev. Using it on a media module image field (admin/content/media) as well as directly on a node with media field, the remote image is downloaded and listed in admin/content/files but when saving the media or the node I get:

    Error: Object of class Drupal\media\Entity\MediaType could not be converted to string in Drupal\Component\Render\HtmlEscapedText->__construct() (line 31 of /path_to_site/web/core/lib/Drupal/Component/Render/HtmlEscapedText.php).

contrib.social Blog FAQ Discussions
Production build 0.69.0 2024