| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | base system |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Issue Summary
Hello,
There is a slight bug in drupal_http_request().
The function uses fsockopen(); in http://au.php.net/manual/en/function.fsockopen.php you read:
errno
If provided, holds the system level error number that occurred in the system-level connect() call.
If the value returned in errno is 0 and the function returned FALSE, it is an indication that the error occurred before the
connect() call. This is most likely due to a problem initializing the socket.
The second sentence is where the problem lies. If you use drupal_http_request() on a NON-EXISTING URL, it will fail - but errno will be 0 and errstr will be empty (the socked wasn't initialized). As a result, for any calling function, $result->error will be '0' (FALSE). No error!
Basically, the code needs to check the condition where fp is NULL *and* errno is 0 - in which case, it needs to create an error "artificially".
This is exactly what my patch does.
It's a 3 liner - it would be great if it were included in 6.x
Bye,
Merc.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| patch_merc.diff | 422 bytes | Idle | FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch patch_merc.diff. | View details | Re-test |
Comments
#1
Hi,
...anybody?
Merc.
#2
Hi,
I realise this is not a very important bug, as it doesn't affect many users. However, I do think it has _some_ importance...
Bye,
Merc.
#3
Hi,
I don't like raising a bug's importance.
However, this bug has been sitting here for more than 3 months now. Not a comment, nothing.
Can somebody please look at this? If an URL doesn't work, anybody calling drupal_http_request() on a non-existing URL won't be able to tell that there was a problem at all.
I don't think it's urgent. However, after 3 months, I think it has somehow *become* urgent.
Bye,
Merc.
#4
Patches won't get much review if they are not marked as such.
This looks okay on initial inspection, but needs more spaces to match http://drupal.org/coding-standards
Patches are usually best made against the current development version and then back-ported as necessary.
#5
Hi,
I don't have a Drupal 7 right now.
However, the patch should be exactly the same, since the code has the same bug.
It's easy to just add the 3 lines by hand, in fact.
Please let me know if you will go forwards with this. Since it's such a tiny patch, backporting it to D6 and D5 would be grand!
Merc.
#6
#7
The last submitted patch, 193073-6-drupal_http_request.patch, failed testing.
#8
#9
#10
Any luck with this patch?
#11
Bumping to D8. I updated the patch a bit to match current coding standards. As $errno can be 0 if the socket is not initialized, and -0 is still 0, we assign a specific error code of -1004.
#12
If your looking to use this right now, I've added this patch to #1320222-1: Bring in other drupal patches. I'm expecting to commit it to httprl sometime tomorrow.
If you know of any other issues that deal with drupal_http_request() let me know!
#13
Tagging as novice for the task of rerolling the Drupal 8.x patch on account of #22336: Move all core Drupal files under a /core folder to improve usability and upgrades.
If you need help rerolling this patch, you can come to core office hours or ask in #drupal-gitsupport on IRC.
#14
Rerolled the patch.
#15
#16
Er, we need to strip .txt from the filename so testbot will test it. :)
#17
Ouch, my bad sorry.
#18
And setting to NR so testbot tests it. :)
#19
Hi,
Wow, I submitted this patch some 4 years ago... (!).
@mikeytown2: did this end up getting committed?
Merc.
#20
@mercmobily
This patch did get committed to HTTPRL
#21
Hi Mike,
Ah, I see. But it hasn't been included in Core yet, right?
Merc.