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

Geoip map #606

Closed
wants to merge 18 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@dmiller-nmap

dmiller-nmap commented Dec 5, 2016

Let's get this into Nmap!

@dmiller-nmap

This comment has been minimized.

Show comment
Hide comment
@dmiller-nmap

dmiller-nmap Dec 5, 2016

@mogigoma This is looking good! There were a couple issues with missing libraries (string, table), and one place where you used query instead of path in an error message return. Use the check script or git hooks to check this stuff.

Of a bit more concern, when I use the KML script with ip-geolocation-geoplugin and then import into mymaps.google.com, it won't show anything with a negative longitude. Is there a standard for this that Google is expecting but Geoplugin is providing differently? Should we be doing some sort of conversion?

For the KML script, if your output is just a message as a string, it's ok to just return that. No need to mess around with stdnse.output_table() or stdnse.format_output. In fact, it's usually incorrect to mix those two.

Try to reduce the number of calls to table.insert in ip-geolocation-map-kml. Create the table with a single string element containing the file header. Insert one element into the table per coordinate: the KML for that point. It can contain newlines, you can format it with string.format, or anything. That is cheaper and cleaner than building a table with 6 times as many elements as necessary.

Overall, this looks great, and I'd like to get it done this week.

dmiller-nmap commented Dec 5, 2016

@mogigoma This is looking good! There were a couple issues with missing libraries (string, table), and one place where you used query instead of path in an error message return. Use the check script or git hooks to check this stuff.

Of a bit more concern, when I use the KML script with ip-geolocation-geoplugin and then import into mymaps.google.com, it won't show anything with a negative longitude. Is there a standard for this that Google is expecting but Geoplugin is providing differently? Should we be doing some sort of conversion?

For the KML script, if your output is just a message as a string, it's ok to just return that. No need to mess around with stdnse.output_table() or stdnse.format_output. In fact, it's usually incorrect to mix those two.

Try to reduce the number of calls to table.insert in ip-geolocation-map-kml. Create the table with a single string element containing the file header. Insert one element into the table per coordinate: the KML for that point. It can contain newlines, you can format it with string.format, or anything. That is cheaper and cleaner than building a table with 6 times as many elements as necessary.

Overall, this looks great, and I'd like to get it done this week.

@mogigoma

This comment has been minimized.

Show comment
Hide comment
@mogigoma

mogigoma Dec 8, 2016

I had the Git hook in place but it was failing without me noticing due to reinstalling and not having lua5.2 and pep8 installed. Ran all files through nmap-check.sh, should be good now.

For the KML, swapped lat and lon and added protections in the library against nonsensical GPS coords. KML script now has fewer concats and inserts. Used [[ ... ]] notation and out-dented further than normal to get the KML output to look right, seemed better than lots of \n[sp][sp][sp][sp] to get the indentation correct.

mogigoma commented Dec 8, 2016

I had the Git hook in place but it was failing without me noticing due to reinstalling and not having lua5.2 and pep8 installed. Ran all files through nmap-check.sh, should be good now.

For the KML, swapped lat and lon and added protections in the library against nonsensical GPS coords. KML script now has fewer concats and inserts. Used [[ ... ]] notation and out-dented further than normal to get the KML output to look right, seemed better than lots of \n[sp][sp][sp][sp] to get the indentation correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment