From b750443ec5742707719b0dde29ed3b30ca9c29e8 Mon Sep 17 00:00:00 2001 From: Barry Evans Date: Mon, 17 Jun 2024 14:33:04 +0900 Subject: [PATCH] Enhance JSON response parsing, prevent error leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the JSON response from speedtest does not contain a property with the value - `"type": "result"`, then a problem has occurred and the script should not attempt to extract the values from the JSON object or perform a database save. Currently, a non-user friendly error message is shown when a problem occurs, but we should prevent this script error and show a more friendly message. Another side effect of not handling the error is that the script will exit with a non-zero status, in effect killing the container. Network problems may be intermittent, and if the container is running the speed tests in a loop, we want to be able to handle those intermittent problems without the container dying. Before this change: ```sh docker run -e SPEEDTEST_SERVER_ID=123 -e LOOP=true -e LOOP_DELAY=5 barryevans80/speedtest:wip Running speedtest forever... ♾️ Running a Speed Test with Server ID 123... {"type":"log","timestamp":"2024-06-17T13:34:56Z","message":"Configuration - No servers defined (NoServersException)","level":"error"} ./speedtest.sh: 33: arithmetic expression: expecting primary: " / 125000 " ``` After this change: ```sh docker run -e SPEEDTEST_SERVER_ID=123 -e LOOP=true -e LOOP_DELAY=5 barryevans80/speedtest:wip Running speedtest forever... ♾️ Running a Speed Test with Server ID 123... {"type":"log","timestamp":"2024-06-17T13:33:29Z","message":"Configuration - No servers defined (NoServersException)","level":"error"} [error] Unable to retrieve JSON results from speedtest, please see previous log message Running next test in 5s... Running a Speed Test with Server ID 123... {"type":"log","timestamp":"2024-06-17T13:33:36Z","message":"Configuration - No servers defined (NoServersException)","level":"error"} [error] Unable to retrieve JSON results from speedtest, please see previous log message Running next test in 5s... ``` --- The validation required to make the script more robust is achieved by using a simple jq `select` function: - `echo "$JSON" | jq 'select(.type == "result")'` See more about the jq `select` function here: - https://jqlang.github.io/jq/manual/#select --- Some shellcheck fixes have been applied: 1. `$/${} is unnecessary on arithmetic variables.` - https://github.com/koalaman/shellcheck/wiki/SC2004 2. `Double quote to prevent globbing and word splitting.` - https://github.com/koalaman/shellcheck/wiki/SC2086 --- Sample successful json response ```json { "type": "result", "timestamp": "2024-06-17T05:27:00Z", "ping": { "jitter": 0.852, "latency": 288.814, "low": 287.153, "high": 289.37 }, "download": { "bandwidth": 1591349, "bytes": 22220160, "elapsed": 15010, "latency": { "iqm": 286.068, "low": 282.946, "high": 293.54, "jitter": 1.568 } }, "upload": { "bandwidth": 38988010, "bytes": 493973670, "elapsed": 15005, "latency": { "iqm": 295.051, "low": 286.54, "high": 379.607, "jitter": 7.626 } }, ... } ``` Sample failed json response ```json { "type": "log", "timestamp": "2024-06-17T05:25:48Z", "message": "Configuration - No servers defined (NoServersException)", "level": "error" } ``` --- speedtest.sh | 40 +++++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/speedtest.sh b/speedtest.sh index c8692e9..9aac057 100755 --- a/speedtest.sh +++ b/speedtest.sh @@ -27,24 +27,30 @@ run_speedtest() JSON=$(speedtest --accept-license --accept-gdpr -f json) fi - DOWNLOAD="$(echo $JSON | jq -r '.download.bandwidth')" - UPLOAD="$(echo $JSON | jq -r '.upload.bandwidth')" - PING="$(echo $JSON | jq -r '.ping.latency')" - echo "Your download speed is $(($DOWNLOAD / 125000 )) Mbps ($DOWNLOAD Bytes/s)." - echo "Your upload speed is $(($UPLOAD / 125000 )) Mbps ($UPLOAD Bytes/s)." - echo "Your ping is $PING ms." + json_result_found="$(echo "$JSON" | jq 'select(.type == "result")')" - # Save results in the database - if $DB_SAVE; - then - echo "Saving values to database..." - curl -s -S -XPOST "$DB_HOST/write?db=$DB_NAME&precision=s&u=$DB_USERNAME&p=$DB_PASSWORD" \ - --data-binary "download,host=$HOSTNAME value=$DOWNLOAD $DATE" - curl -s -S -XPOST "$DB_HOST/write?db=$DB_NAME&precision=s&u=$DB_USERNAME&p=$DB_PASSWORD" \ - --data-binary "upload,host=$HOSTNAME value=$UPLOAD $DATE" - curl -s -S -XPOST "$DB_HOST/write?db=$DB_NAME&precision=s&u=$DB_USERNAME&p=$DB_PASSWORD" \ - --data-binary "ping,host=$HOSTNAME value=$PING $DATE" - echo "Values saved." + if [ -n "${json_result_found}" ]; then + DOWNLOAD="$(echo "$JSON" | jq -r '.download.bandwidth')" + UPLOAD="$(echo "$JSON" | jq -r '.upload.bandwidth')" + PING="$(echo "$JSON" | jq -r '.ping.latency')" + echo "Your download speed is $((DOWNLOAD / 125000 )) Mbps ($DOWNLOAD Bytes/s)." + echo "Your upload speed is $((UPLOAD / 125000 )) Mbps ($UPLOAD Bytes/s)." + echo "Your ping is $PING ms." + + # Save results in the database + if $DB_SAVE; + then + echo "Saving values to database..." + curl -s -S -XPOST "$DB_HOST/write?db=$DB_NAME&precision=s&u=$DB_USERNAME&p=$DB_PASSWORD" \ + --data-binary "download,host=$HOSTNAME value=$DOWNLOAD $DATE" + curl -s -S -XPOST "$DB_HOST/write?db=$DB_NAME&precision=s&u=$DB_USERNAME&p=$DB_PASSWORD" \ + --data-binary "upload,host=$HOSTNAME value=$UPLOAD $DATE" + curl -s -S -XPOST "$DB_HOST/write?db=$DB_NAME&precision=s&u=$DB_USERNAME&p=$DB_PASSWORD" \ + --data-binary "ping,host=$HOSTNAME value=$PING $DATE" + echo "Values saved." + fi + else + echo "[error] Unable to retrieve JSON results from speedtest, please see previous log message" fi }