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 uncaught exception of stof #61

Merged
merged 6 commits into from
Apr 7, 2024
Merged

Conversation

gglluukk
Copy link
Contributor

Fixes #60

@tcooper-xx
Copy link

Can confirm fix works on Ubuntu 22.04

@bmxkris
Copy link

bmxkris commented Mar 21, 2024

Works for me on Debian GNU/Linux 12 (bookworm)

@OboTheHobo
Copy link

Works for me as well on Fedora 39

@aidan-gibson
Copy link

Can confirm on arch

@FaazHafid
Copy link

Hi,

New here, how do I fix this with the code? Is there instructions to do that, apologies not too much familiar with GitHub or linux.

@OboTheHobo
Copy link

Hi,

New here, how do I fix this with the code? Is there instructions to do that, apologies not too much familiar with GitHub or linux.

inside the directory for SpeedTest, run git pull https://github.com/gglluukk/SpeedTest and then recompile as shown in the readme (the cmake and make command)

@Stew-McD
Copy link

Stew-McD commented Apr 5, 2024

Hi,

New here, how do I fix this with the code? Is there instructions to do that, apologies not too much familiar with GitHub or linux.

gh pr checkout 61

then the same install again

@ansemjo
Copy link

ansemjo commented Apr 6, 2024

There's some extraneous quotes around the IP and ISP when printing the values. I'm not sure if that's due to the explicit casting to strings or if the JSON library you used does something weird?

It's not a problem for textual outputs obviously but completely breaks JSON output because that is a simple string concatenation in main.cpp (actual values replaced, obviously):

{"client":{"ip":""x.x.x.x"","lat":"00.0000","lon":"00.0000","isp":""Deutsche Telekom AG""},"servers_online":"10","server":{"name":"Strasbourg","sponsor":"bvpn.eu","distance":"222.986","latency":"23","host":"ookla.de.eu.waselpro.com:8080"},"ping":"23","jitter":"0","_":"only latency requested"}

Can be fixed with:

    try {
        map["ip_address"] = obj["ip"].ToString();
        map["isp"] = obj["company"]["name"].ToString();
        map["lat"] = obj["location"]["latitude"].dump();
        map["lon"] = obj["location"]["longitude"].dump();
    } catch(...) {}

SpeedTest.cpp Outdated
map["isp"] = (std::ostringstream() << obj["company"]["name"]).str();
map["lat"] = (std::ostringstream() << obj["location"]["latitude"]).str();
map["lon"] = (std::ostringstream() << obj["location"]["longitude"]).str();
map["ip_address"] = static_cast<std::ostringstream &&>
Copy link

Choose a reason for hiding this comment

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

Try this instead to correctly dump the strings without extra quotes:

    try {
        map["ip_address"] = obj["ip"].ToString();
        map["isp"] = obj["company"]["name"].ToString();
        map["lat"] = obj["location"]["latitude"].dump();
        map["lon"] = obj["location"]["longitude"].dump();
    } catch(...) {}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have never tried to output to JSON, so your patch corrects it as well as brings to the code more readability.
Thanks for the fix!

@taganaka
Copy link
Owner

taganaka commented Apr 7, 2024

Thanks for helping on this!

@taganaka taganaka merged commit b743996 into taganaka:master Apr 7, 2024
axel-ppi pushed a commit to axel-ppi/SpeedTest that referenced this pull request Apr 16, 2024
* Fix uncaught exception of stof

* Another SPEED_TEST_IP_INFO_API_URL with JSON parsing

* Added Fixed: link before pull

* Fix static_cast<std::ostringstream &&> for type conversion

* Removed unused total_size and total_time

* JSON output fixed (thanks to ansemjo)

(cherry picked from commit b743996)
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.

Error: uncaught exception of stof
9 participants