wiglenet / wigle-wifi-wardriving

Nethugging client for Android, from wigle.net
https://wigle.net
BSD 3-Clause "New" or "Revised" License
657 stars 204 forks source link

[ENH] Original SSID with commas valid CSV format fix #529

Closed Krzysztofz01 closed 9 months ago

Krzysztofz01 commented 2 years ago

I do not know if this pull request will be approved because I assume that the compatibility with the WiGLE backend is key and it probably handles SSIDs in its own way (in which the commas have been replaced with underscores). Recently, while researching networks, I came across a large number of networks with SSIDs containing commas. The names looked something like this:

Hotspot 2,4Ghz
Hotspot 5,0Ghz

In one of my projects, I dealt with the appropriate SSID formatting on the backend, but here this problem can be solved during the object-CSV serialization stage. The solution is to put the SSID in quotation marks according to the RFC4180:

6. Fields containing line breaks (CRLF), double quotes, and commas should be enclosed in double-quotes.

Thanks to this, the CSV format will be valid and we will maintain the highest level of compliance with the actual parameters of a given access point.

XenoKovah commented 1 year ago

Just as an FYI, I'm guessing it's already covered, but Bluetooth names can also have commas in them too

Krzysztofz01 commented 1 year ago

[...] but Bluetooth names can also have commas in them too

As far as I know, if the Network in question is Bluetooth/BT then the device name is also stored in the ssid field. So this kind of CSV format escaping will also work for Bluetooth devices. Please correct me if I am wrong.

XenoKovah commented 1 year ago

As far as I know too that's correct. But I figured a heads up was in order for the WiGLE folks to double check just in case it happened to get stored somewhere else too

Krzysztofz01 commented 1 year ago

Good point! Actually, it is not known how different types of networks are handled by the backend.

rksh commented 10 months ago

I think this is getting incorporated/co-fixed in https://github.com/wiglenet/wigle-wifi-wardriving/pull/642 More importantly server-side parsing fixes are coming online so it'll work correctly.

Krzysztofz01 commented 9 months ago

I think this is getting incorporated/co-fixed in #642 More importantly server-side parsing fixes are coming online so it'll work correctly.

Under such circumstances, do we still want to secure this on the mobile app side?

bobzilladev commented 9 months ago

Aye, the mobile side is fixed in #642