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.
Comment | File | Size | Author |
---|---|---|---|
#17 | mr8-fatal-error.png | 96.38 KB | szato |
#9 | filefield_sources-use_drual_http_client-2840594-9.patch | 4.52 KB | w.drupal |
#6 | use_drual_http_client-2840594-6.patch | 5.65 KB | KevinVanRansbeeck |
#3 | use_drual_http_client-2840594-#1.patch | 5.03 KB | wouters_f |
Issue fork filefield_sources-2840594
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
wouters_f CreditAttribution: wouters_f commented* patch on the way...
Comment #3
wouters_f CreditAttribution: wouters_f commentedThe uploading using the "remote" filefield source works with these changes.
I'm not really sure what to do in the error cases. Any advice would be welcome.
Comment #4
wouters_f CreditAttribution: wouters_f commentedComment #5
oriol_e9gNew code break tests, still needs work, $info var is removed and used later.
Comment #6
KevinVanRansbeeck CreditAttribution: KevinVanRansbeeck at AmeXio commentedComment #7
KevinVanRansbeeck CreditAttribution: KevinVanRansbeeck at AmeXio for VRT commentedComment #8
jonathanshawWe seem to have lost that functionality, but in pirnciple it still seems relevant. Maybe it could be accessed from that request instead.
Comment #9
w.drupal CreditAttribution: w.drupal at Engine Eight commented@jonathanshaw
Seems it's not relevant as well as a second call via HTTP client. Even if the source path will return 301 redirect the client will process it and return response from the redirected page. Let me know if i'm wrong but it what my tests showed. Also i added:
Comment #10
rivimeyLooks to me like a reasonable patch. The old code did the equivalent of a HEAD call on the URL and checked the returned headers before (possibly) retrying with a GET; the new code just does the GET straight away. I'm not sure why the old code wanted a distinct HEAD, but it does seem unnecessary to me.
From my POV this passes a visual review, though I haven't applied the patch to actually test it (yet).
Comment #11
marassa CreditAttribution: marassa commentedI encountered warnings about wrong calls of curl-related functions after I switched to PHP 8: Warnings while adding a remote file after switching to PHP 8.0. Applying patch #9 getting rid of direct curl calls turned out easier than trying to figure out the existing code and solved the problem. Recommended.
Comment #12
Kris77 CreditAttribution: Kris77 commentedPatch #9 works for me too.
Thanks @w.drupal
Comment #13
rschwab CreditAttribution: rschwab at University of California commentedSince php 8 is required for Drupal 10, I'm marking this as a child of #3336268: Drupal 10 compatibility to try and get that ticket completed. This potentially solves the last errors in the D10 test suite.
Comment #15
rschwab CreditAttribution: rschwab at University of California commentedApplied #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 #16
rschwab CreditAttribution: rschwab at University of California commentedComment #17
szato CreditAttribution: szato at Brainsum for The International Federation of Red Cross and Red Crescent Societies (IFRC) commentedThere is a fatal php error because of duplicate use statements.
I'm going to rebase the MR8 on 2.0.x and fix these.
Comment #18
szato CreditAttribution: szato at Brainsum for The International Federation of Red Cross and Red Crescent Societies (IFRC) commentedPlease review it, now it works ;)