liuch / dmarc-srg

A php parser, viewer and summary report generator for incoming DMARC reports.
GNU General Public License v3.0
228 stars 34 forks source link

show hostname of IP address in report #5

Closed Junker closed 2 years ago

liuch commented 3 years ago

It's a good idea, but I can see two problem with your code:

  1. What if gethostbyaddr is unable to resolve the passed IP address to the host name, what will we see on the frontend? That should be handled.
  2. What if the host name is to long? This will definitely break the markup in the web interface. I think that too long part of the hostname should be cut off in the same way that too long report IDs are cut off in the report list.
mbsouth commented 1 year ago

Some thoughts:

  • What if gethostbyaddr is unable to resolve the passed IP address to the host name, what will we see on the frontend? That should be handled.

gethostbyaddr() -> Returns the host name on success, the unmodified ip on failure (or false on malformed input [should not occur]).

Be cautious when looking up many hostnames!

If the DNS server is slow to respond, you may have to pump up your max_execution time for your php scripts and it will slow down your server.

It would be a bad idea to use gethostbyaddr() within the while ($res = $st->fetch(PDO::FETCH_NUM)) loop.
Maybe it's better to do it while picking up the reports from IMAP account and using an extra ip-to-hostname-db-table (hostname = unique) and adding just unknown hostnames to it.

  • What if the host name is to long? This will definitely break the markup in the web interface. I think that too long part of the hostname should be cut off in the same way that too long report IDs are cut off in the report list.

Simple solution:

.report-record > .header {
    word-break: break-word;
}
liuch commented 1 year ago

@mbsouth Thanks for your thoughts. To be clear, I didn't mean that it can't be done, I meant that it's not implemented in the current version of the PR.