Contrib.social

Dont use curl, use the drupal client instead

Open on Drupal.org →
Created on 3 January 2017, almost 8 years ago
Updated 12 August 2023, over 1 year 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

RTBC

Version

1.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 over 1 year 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 about 1 year ago
    15 pass
  • Comment about 1 year 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 about 1 year ago
    15 pass
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.1 & MySQL 8
    last update about 1 year ago
    15 pass
  • Status changed to Needs review about 1 year ago
  • Comment about 1 year 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 11 months ago →
    🇺🇸United States rschwab
  • Status changed to Needs work 11 months ago
  • Comment 11 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 11 months ago
  • Comment 11 months ago →
    🇭🇺Hungary szato

    Please review it, now it works ;)

  • Comment 8 months ago →
    🇺🇸United States rschwab

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

  • Comment 8 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 7 months 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).

  • Comment 3 months ago →
    🇮🇹Italy kopeboy Milan

    Before applying MR!8, getting a PNG or Webp image from this API url worked:
    (Simple API docs here: https://monkey.banano.cc/documentation -- you can try Webp by changing the format=png to format=webp).

    After applying MR!8 it didn't anymore and clicking on Transfer would return Error: The remote URL must be a file and have an extension.

    The same error is happening, even without this MR, when trying to upload an SVG from the same API, for example this one (removing the parameters you get an <svg> by default).

    I don't know if the previous non-error was just a lucky coincidence, but I imagine that many people will have my same need, ie. to get files from APIs that don't have specific URLs pointing to single files but that generate them.

    I'm keeping this as "Needs review" because this problem might be unrelated: a traditional file URL was working fine indeed!

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024