#13915 closed enhancement (fixed)
WP_Http_Fopen is PHP4 only
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | 3.2 | Priority: | normal |
Severity: | trivial | Version: | 3.0 |
Component: | HTTP API | Keywords: | 3.2-early has-patch |
Focuses: | Cc: |
Description
Patch checks for PHP5 and then prevents Fopen transport from being used on PHP5.
Attachments (3)
Change History (26)
#2
@
15 years ago
Not really a bug, since everything works as it should, but this was really only for PHP4 for fopen support.
#4
@
15 years ago
- Cc gazouteast added
- Keywords (are you sure?) added
Jacob - not so sure about that - earlier this year, I had major tickets with WPMU regarding non-displaying avatars and images that were traced to fopen not being available under PHP 5.x (can't find the original discussions since WPMU and BuddyPress changed their forum engines and structures).
A number of people including Andrea_r got roped in on that one, I think johnjamesjacoby did too, and all conceded the catch when I finally tracked it down.
Can you absolutely assure that all requirements for fopen have been removed from WP 3.0 otherwise you might be being a bit premature here, and have to leave backwards compatibility in place - this also rubs against the many users reporting images not displaying after upgrade to 3.0 from earlier versions.
Gaz
#5
@
15 years ago
- Keywords (are you sure?) removed
The WP_Http_Fopen class was written to enable fopen support for PHP4. PHP5 added support for Streams and the WP_Http_Streams class uses the streams method when using fopen on PHP5+. The problem is that this class is in rare cases being used on PHP5 setups where WP_Http_Streams is failing (because of SSL or another reason) and the WP_Http_Fopen is passing. The goal was to allow fsockopen to be used if at all possible on PHP5 when fopen is not available and hope that fsockopen is still available.
I have to confess I did not follow the discussion with BuddyPress or WPMU, as neither of those really have anything to do with this ticket. I don't know the specifics of the issues that were discussed but with my limited information, I don't believe they are relevant to this ticket.
If you have more information, on that, I would like to see.
#6
follow-up:
↓ 7
@
15 years ago
@jacobsantos, if you can explain how to duplicate the environment I'll gladly test it against BuddyPress and help if a fix is necessary.
If I recall, this issue may have been almost (if not over) a year ago, so I'm a little fuzzy on the topic myself.
At first glance, it doesn't seem like the patch would hurt. Although the WP_Http_Streams::test method puts that check after the function_exists('fopen') check. That doesn't mean it wouldn't wreck our days if we tried it. :)
#7
in reply to:
↑ 6
@
15 years ago
Replying to johnjamesjacoby:
@jacobsantos, if you can explain how to duplicate the environment I'll gladly test it against BuddyPress and help if a fix is necessary.
I still don't understand. This is a PHP4 environment, so you would simply install PHP4. The Streams will not be available. If you Install PHP5, then this class should never be used, the next fail over should be fsockopen.
If I recall, this issue may have been almost (if not over) a year ago, so I'm a little fuzzy on the topic myself.
That is unfortunate as I'm trying to figure out what the other guy means. If fopen doesn't exist, then I'm unsure how the HTTP API has anything to do with avatars and what not with BuddyPress and WPMU (which is part of WP now). There were problems with specific versions of fopen when cURL was installed and used, but that generally has no good fix.
I'm to assume that either it has to do with the filesystem which is outside the scope of this ticket and also HTTP API. Or the problem was outside the scope of this class and API and is a PHP issue. Neither of which has anything to do with this ticket.
Until more information is given, I can't really give a solution and therefore have to ignore it. As of right now, until more information is given, I believe it has nothing to do with this ticket or the patch, I would frankly rather it not be discussed further.
#8
@
14 years ago
Since we are dropping PHP4 support in 3.2, no reason to keep this around. It was only kept because PHP4 didn't support streams with fopen. Now that we don't have this restriction no need to keep this code around.
#9
@
14 years ago
- Keywords 3.2-early added
- Owner jacobsantos deleted
- Status changed from accepted to assigned
#10
@
14 years ago
if this is removed based on PHP5+ availability, Note that WP_Http_Fopen::test()
is used in image-edit.php to test to see if external fopen() calls are allowed.
#11
@
14 years ago
- Keywords needs-refresh added; has-patch tested removed
refresh needed based on my previous comment
#12
@
14 years ago
I reduced the check for allow_url_fopen for a single ini_get call.
The previous one was checking for function existance of fopen(), ini_get() and then to check ini_get( 'allow_url_fopen' ). Any Idea dd32? I
#15
follow-up:
↓ 16
@
14 years ago
The previous one was checking for function existance of fopen(), ini_get() and then to check ini_get( 'allow_url_fopen' ). Any Idea dd32? I
Perhaps you're not the best person to patch it if you're unaware of why it's done as it is?
Off the top of my head, disabled_functions setting will cause fopen to not exist, similarly, some hosts disable ini_get, and finally, need to check that allow_url_fopen is allowed.
#16
in reply to:
↑ 15
@
14 years ago
Replying to dd32:
The previous one was checking for function existance of fopen(), ini_get() and then to check ini_get( 'allow_url_fopen' ). Any Idea dd32? I
Perhaps you're not the best person to patch it if you're unaware of why it's done as it is?
Just was asking for a second opinion, I know why that code was originally in, I just don't think it's necessary to check for those with such a detail at that place.
#18
@
14 years ago
- Milestone changed from Future Release to 3.2
the image-edit.php change was originally made in [11985].
#21
@
14 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
Looks like in [17535] you removed the SSL proxy, compare: http://core.trac.wordpress.org/attachment/ticket/16920/16920.2.patch , there is a second patch available as well reg. class-http so you can compare.
Prevent WP_Http_Fopen from being used on PHP5+