Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

show hostname of IP address in report #5

Closed
wants to merge 1 commit into from

Conversation

Junker
Copy link

@Junker Junker commented Aug 23, 2021

No description provided.

@liuch
Copy link
Owner

liuch commented Aug 23, 2021

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
Copy link

mbsouth commented Jul 5, 2023

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
Copy link
Owner

liuch commented Jul 6, 2023

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants