search

narkisr / nmap4j

A Java Nmap wrapper
Other
80 stars 59 forks source link

Multiple Hostname Entries #11

Closed TheDropZone closed 4 years ago

TheDropZone commented 4 years ago

While using this awesome library, I found out that we were losing data under the tags. The current version only stores the last Hostname entry under the Hostnames tag. If Nmap returns multiple Hostname entries for a given host, we lose all but the last Hostname. I'm proposing a similar construct to the Host.addresses where we store all Hostname objects in an ArrayList so we don't lose that data.

Thanks!

narkisr commented 4 years ago

Hi @TheDropZone thank you for submitting the PR, it seems that the code does not compile with this change:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.1:compile (default-compile) on project org.nmap4j: Compilation failure: Compilation failure: 
[ERROR] nmap4j/src/main/java/org/nmap4j/data/nmaprun/Host.java:[73,27] cannot find symbol
[ERROR]   symbol:   class Hostname
[ERROR]   location: class org.nmap4j.data.nmaprun.Host
[ERROR] nmap4j/src/main/java/org/nmap4j/data/nmaprun/Host.java:[87,26] cannot find symbol
[ERROR]   symbol:   class Hostname
[ERROR]   location: class org.nmap4j.data.nmaprun.Host
[ERROR] nmap4j/src/main/java/org/nmap4j/data/nmaprun/Host.java:[90,44] cannot find symbol
[ERROR]   symbol:   class Hostname
[ERROR]   location: class org.nmap4j.data.nmaprun.Host
[ERROR] nmap4j/src/main/java/org/nmap4j/data/nmaprun/Host.java:[93,33] cannot find symbol
[ERROR]   symbol:   class Hostname
[ERROR]   location: class org.nmap4j.data.nmaprun.Host
[ERROR] -> [Help 1]
[ERROR]
TheDropZone commented 4 years ago

@narkisr haha, well that's embarrassing. My bad. It seems I forgot to include the import for Hostname.... I'll get that fixed up and run all the tests before submitting again. Thanks

narkisr commented 4 years ago

No worries :), thank you

TheDropZone commented 4 years ago

@narkisr Got the import added in and it is compiling! Woohoo. I did try running the tests but as I am on a Windows with a different Nmap location, not all of them passed.

TheDropZone commented 4 years ago

@narkisr Anything else on this branch I need to clean up? Thanks!

narkisr commented 4 years ago

Hi @TheDropZone the main thing left to do is to make sure the build stay green and that all the tests are passing (https://travis-ci.org/narkisr/nmap4j).

Its recommended to run them under an Ubuntu machine (could be a VM), the current setup requires symlinking the nmap binary to /usr/local/nmap

Ill try to take a look on which tests break and if a quick fix is possible.

Thanks

narkisr commented 4 years ago

Ok doing a quick run of the tests I can see that only one is failing:

https://github.com/narkisr/nmap4j/blob/master/src/test/java/org/nmap4j/parsers/NMapXmlHandlerTest.java#L25

As far as I can tell its because the hostnames value is null and get passed in as the event payload in:

https://github.com/narkisr/nmap4j/blob/master/src/test/java/org/nmap4j/parsers/NMapXmlHandlerTest.java#L67

I still need to think what the proper fix is in this case.

narkisr commented 4 years ago

Hi @TheDropZone iv done another analysis of the code (the model is a bit confusing) and it seems using a similar approach to Ports tag makes this change easier to implement while keeping all the tests green.

Please have a look at https://github.com/narkisr/nmap4j/pull/12 and see if it works for your use case.

Thanks