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

Fix nmap table result parsing #11

Merged
merged 7 commits into from
Jun 4, 2019
Merged

Fix nmap table result parsing #11

merged 7 commits into from
Jun 4, 2019

Conversation

botastic
Copy link
Contributor

@botastic botastic commented May 17, 2019

As mentioned in #9 the data from tables isn't always parsed correctly. As I was already checking the nmap DTD XML specifications, I saw that the relation between scripts, tables and elements is a bit more complicated.
While your approach - using a map for one table - was quiet comfortable it lacked completeness.

Here's an excerpt of the relevant section in the DTD:

<!ELEMENT script	(#PCDATA|table|elem)* >
<!ATTLIST script	
	id	CDATA	#REQUIRED
	output	CDATA	#REQUIRED
>

<!ELEMENT table	(table|elem)* >
<!ATTLIST table
    key CDATA #IMPLIED
>

<!ELEMENT elem	(#PCDATA)>
<!ATTLIST elem
    key CDATA #IMPLIED
>

The major problems are:
- nested tables
- elements without key
- (tables without key)

Because of the optional keys I decided to create new structs for Tables and Elements in my approach. I also considered adding an additional map to each one of the new structs, that would allow for a more comfortable access. But in the end I decide against that, as the missing keys would still be a problem.

Currently the script does not have a field for the PCDATA. This was not of relevance to me but can be easily added.

As in my other PR I adapted the tests and extended them a bit to better suit the new implementation. Additionally I also changed the TestToFile function, as in the other PR.

If you'd like to accept both PRs, I also have a branch that currently holds all the changes,

Best regards

PS: The first commits have been reverted, as I created the branch after submitting my first PR and wanted to create two PRs for an easier review ;)

Copy link
Owner

@Ullaakut Ullaakut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Ullaakut Ullaakut changed the title Relation between script, table and element Fix nmap table result parsing Jun 4, 2019
@Ullaakut Ullaakut merged commit 794694a into Ullaakut:master Jun 4, 2019
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.

2 participants