Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.As seen in issues like:
- #2510642: Request headers missing
- #2233159: compress.zlib stream wrapper for a remote file
- #2040053: get_headers() does not work with module enabled
- #1416564: $http_response_header not set
overriding the default http stream handler is an issue. Here's an attempt at solving *some* of those issues.
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | remote_stream_wrapper-use-native-2522056-14.patch | 10.35 KB | das-peter |
| |||
| #14 | interdiff-2522056-13-14-do-not-test.diff | 1.34 KB | das-peter |
Comments
Comment #1
twistor CreditAttribution: twistor as a volunteer commentedComment #2
twistor CreditAttribution: twistor as a volunteer commentedThis actually still needs work. Specifically around handling context and setting the correct user agent, but I wanted to get feedback before I went too far.
Comment #4
twistor CreditAttribution: twistor as a volunteer commentedHaha, I have a lot of changes in mind. #2522060: Use a temp stream for managing the downloaded content. would make this patch easier, but it's definitely not a blocker.
Comment #5
twistor CreditAttribution: twistor as a volunteer commentedUgh, this took way to long to debug because of @ usage.
Comment #6
twistor CreditAttribution: twistor as a volunteer commentedThis still needs context handling to fix more issues listed in the issue summary as well as setting the user agent.
Comment #7
twistor CreditAttribution: twistor as a volunteer commentedSo this makes the test run with both stream wrappers. Ensuring that we don't break one while testing the other.
Comment #8
twistor CreditAttribution: twistor as a volunteer commentedNo really running both sets of tests.
Comment #9
twistor CreditAttribution: twistor as a volunteer commentedThis adds proper context handling.
It would make sense for us to add context support for both wrappers for options like timeout and redirects.
Edit: This also adds support for drupal_http_request() proxy handling in the native handler.
Comment #10
twistor CreditAttribution: twistor as a volunteer commentedD.O ate my previous post. Walking it off.
Comment #12
twistor CreditAttribution: twistor as a volunteer commentedComment #13
twistor CreditAttribution: twistor as a volunteer commentedSo, this fixes #2233159: compress.zlib stream wrapper for a remote file at least for the native stream, and possibly for the drupal_http_request version, but it depends on the PHP version. stream_cast() is required in my testing to get the zlib stream to work. #2522060: Use a temp stream for managing the downloaded content. would make it trivial to implement stream_cast(), so I'm holding off on fixing everything at once.
This fixes #2510642: Request headers missing for both streams and makes the headers accessible in the same way. That's what the ArrayAccess stuff is for. Note: It's not a direct match, since we're using an iterator, and not a regular array, so there still could be problems with other people's code, but it should be easy enough to work around.
#1416564: $http_response_header not set That's some voodoo I don't know.
#2040053: get_headers() does not work with module enabled. I looked into this one actually, and it's broken specifically because we're re-registering the stream not using STREAM_IS_URL or whatever. I think it would still be broken if we changed that (we can't), because get_headers() is checking if wrapper_data is an array. There's no way for us to set wrapper_data, it just gets set to the stream object itself, so there's not much to be done.
The stream wrapper API is terrible.
Comment #14
das-peter CreditAttribution: das-peter at Cando commented@twistor Thanks for this patch! I've just come across an issue with Remote stream wrapper and the Mailchimp module.
The latest mailchimp module uses Guzzle and the http stream wrapper of Remote stream wrapper breaks all kinds of things.
This patch seems to help a lot even thought there remain some problems.
I concur! ;)
Specifically the handling of
$http_response_header- but here we can't do much as this is a php bug: https://bugs.php.net/bug.php?id=63897So if there's anything that uses Guzzle best bet is to ensure the CURL implementation is used as described in #1416564: $http_response_header not set / #2891977: Acquia Search uses a Solarium adapter that conflicts with the remote_stream_wrapper module
However, I've found another situation in which we can do something. The
// Don't allow user-agent overriding.part made some trouble together with guzzle as it sets the header as string and not as array - that makesunset()sad.The attached patch adds a check to see how that has to be handled.
Further it adds some documentation regarding
$http_response_header.