Skip to content

Commit

Permalink
Enhance JSON response parsing, prevent error leak
Browse files Browse the repository at this point in the history
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"
}
```
  • Loading branch information
bazzani committed Jun 17, 2024
1 parent b03a1e8 commit c0f177a
Showing 1 changed file with 23 additions and 17 deletions.
40 changes: 23 additions & 17 deletions speedtest.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down

0 comments on commit c0f177a

Please sign in to comment.