Straight from my log:

screenshot

The log message starting with 'Honeypot lookup results for' shows the reversed IP address.

Thank you for your work on httpbl!

CommentFileSizeAuthor
#1 httpbl_IP_reversed.2466371.1.patch669 bytessalvis
reversed_IP.png4.98 KBsalvis
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

salvis’s picture

Status: Active » Needs review
FileSize
669 bytes

The fix: avoid clobbering $ip.

salvis’s picture

Any chance to get this nit fixed?

bryrock’s picture

Status: Needs review » Closed (works as designed)

This is not a bug. The lookup is done in reverse per project honeypot specs. Sorry if that looks funky in the debug log, but it is correct.

salvis’s picture

Status: Closed (works as designed) » Active

Yes, that's true, but that's only between http:BL and dnsbl.httpbl.org, and only if it's done through the API.

My patch still passes the reversed IP to dnsbl.httpbl.org but it shows the non-reversed IP in the watchdog log. It makes no sense (it's confusing!) to display the reversed IP to the webmaster.

Please reconsider.

bryrock’s picture

Status: Active » Closed (won't fix)

Sorry. I took another look at this and almost committed it. I agree that it looks better and makes more sense from an end user p.o.v., but after another look at the level of the message, I noted that it is a debug message. So the purpose of that particular log message is to verify—from a developer p.o.v.—exactly what happened in that dns lookup, and what happened was a (properly) reversed ip was looked up. It would fail if not reversed, so you want to see (debug) that it is doing what is expected.

There are other, more "friendly" messages (notice level) that report on results of those lookups, from a more practical p.o.v, telling us whether that lookup will result in a blacklist, grey-list, yada yada.

By the way, just put up a D8 dev release today. Quite a bit of changes to the logging (but not that particular one), except that it now only takes one line to get any logging over and done with:

    // Lookup at Project Honey Pot was successful.
    $this->logTrapper->trapDebug('DNS lookup results for @ip, response was @response', array('@ip' => $ip, '@response' => $values['raw']));

All the logic pertaining to the logging itself (are we or aren't we logging these today?) has been moved out of the main logic flow. Logging can be added in the code whenever, wherever, and the LogTrapper service (an httpbl exclusive!) makes the decision of whether or not to actually send it.

salvis’s picture

I would agree in a dpm() message. What you say is correct for the http:BL developer. But there are way more developers (like myself) who use http:BL and want to develop something around it. To us it's very confusing!

How often do you need to test the quirky API that requires the reversed IP? And how often do you (or anyone) use the IP address for some other purpose (as an "end user" developer)?

The reversed IP makes sense only in the scope of the httpbl_dnslookup() function. How many hours do you expect to spend debugging that function? As soon as you pass the reversed IP out of the scope of the httpbl_dnslookup() function, like into watchdog, it gets confusing, for everyone who lives outside that function.

If you really must have it, then how about adding something like "(reversed!)" in front of the reversed IP address? (In front because in the back it could get clipped.)

bryrock’s picture

If it's too confusing, you can always turn off Verbose logging, which shouldn't be used in production, anyway. You'll never see it.

This discussion is ironic because before I was maintainer of this module I advocated for that log message, which was not present at all in D5 or the initial D6 port (checkout historical branches of httpbl and look at httpbl_dnslookup() function). The initial D6 port, by the original module creator, did not work. I had a client that needed httpbl, so I was trying to debug why it didn't work. Back then, without any cool debugging tools (like PhpStorm), I couldn't tell if that reverse was happening or not. So I requested that log message to be added for verification that the IP was correctly being reversed, per project honeypot api/spec (DNS Query Format). I know from the original developer of this module that quite a bit of time was spent debugging the httpbl_dnslookup() function in D5, but he resisted adding any additional log messages at all, because he was concerned that any logging would steal bandwidth from the request handling.

Long story short, he got tired of me bugging him about that and other additional log messages, so made me the maintainer.

Send your suggested change (but don't show the IP not reversed), and I will consider adding it to D7 and D8.