Hello

Yesterday I've got fatal error in module cloudflare.

The reason of the error was that cloudflare returns list of IP's with symbol \n in the end.

Please look at the screenshot:

screen

There is piece of code, from function cloudflare_ip_ranges().
You can see that we have empty value in the end of the list of IP's after execution of explode function.

Due this reason we have recursion in function cloudflare_ip_address_in_range():
1. First execution cloudflare_ip_address_in_range() - $range is empty, we will take $range from cloudflare_ip_ranges()
2. We've got $range, it's array. Foreach is executing.
3. Last item in the foreach is empty string.
4. Function cloudflare_ip_address_in_range is called for this empty string and the $range is empty again and function cloudflare_ip_ranges() will be called again. We're receiving the same list of the IP's again to check each of them and it's infinity cycle.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

eugene.ilyin created an issue. See original summary.

eugene.ilyin’s picture

Status: Active » Needs review
FileSize
479 bytes

I've prepared patch to correct this problem. We should cut this empty string to avoid recursion.

Arvi89’s picture

Thx man, I couldn't figure out where this recursion came from, I thought I had broken something somehow ^^
(varnish would return me an error 500 which was a bit scary at first ^^)

Jason Ruyle’s picture

I ran into this same problem and had to turn off cloudflare on a few of my sites. It didn't happen to all of them, but did cause some major headaches.
I'm testing this patch and so far so good.
This should be added to dev or stable branch.

phayes’s picture

Status: Needs review » Fixed

Merged.

eugene.ilyin’s picture

Thank you. Perhaps it was committed but seems not pushed yet.

pwaterz’s picture

Status: Fixed » Active

@phayes doesn't look like you committed the patch. Also we are still seeing this issue even with this patch.

pwaterz’s picture

What happens if https://www.cloudflare.com/ips-v4 goes down? There isn't any error checking for that.

nullkernel’s picture

Status: Active » Fixed

I recently downloaded and have done some testing and debugging of the currently available 7.x-2.x-dev. The patch in #2 was part of what I downloaded. I was unable to reproduce the problem with the latest code so I'm changing this issue back to "Fixed".

There should be no fatal error or infinite recursion. I tested by changing the CLOUDFLARE_URL_IPV4_RANGE constant to point to a file under my control (and commenting out the cache_set statement so that I wouldn't have to clear cache repeatedly to test). Having extra newlines in the file doesn't cause problems - they are effectively ignored. Even when the file has only newlines in it, there was no infinite recursion.

As for error checking, having the file unavailable (404) or on a nonexistent server doesn't cause problems either. When either of these cases occur, an error message gets logged: "Unable to fetch IP address information from cloudflare. Until resolved, IP address detection is non-functional". Once the server is responsive (i.e. not a 404 or otherwise causing file_get_contents to return FALSE), Drupal will cache Cloudflare's response.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.