esnet / esnet/iperf

JSON output (-J) emits two top-level "error" keys on server-relayed errors

Open Beginner friendly
#2,051 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
C
Stars
8.8k
Forks
1.5k
PR merge metrics
No merged PRs in 30d

Description

# Context

* Version of iperf3:
3.21 (cJSON 1.7.15), git `d39cf41526626b4e5a130f115d931cd6cbdffc19`. The relevant code paths are unchanged on current `master`, so earlier 3.x releases are very likely affected as well.

* Hardware:
x86_64 (immaterial — this is in the JSON-assembly logic, not platform-specific).

* Operating system (and distribution, if any):
Linux. Platform-independent.

# Bug Report

* Expected Behavior

With `-J`, an error that is relayed from the server should produce a single top-level `"error"` member in the JSON document.

* Actual Behavior

When the client receives a `SERVER_ERROR` control message, the `-J` document contains **two** `"error"` members in the top-level object — one with a `SERVER ERROR - ` prefix, one without:

```json
"intervals": [],
"end": {
},
"error": "SERVER ERROR - client's requested duration exceeds the server's maximum permitted limit",
"error": "client's requested duration exceeds the server's maximum permitted limit"
```

RFC 8259 §4 permits duplicate names but states that in that case "the behavior of software that receives such an object is unpredictable." In practice the result is parser-dependent: last-wins parsers (Python `json`, JS `JSON.parse`, Go, Rust `serde_json`) surface only the bare message and silently drop the `SERVER ERROR - ` prefix; a first-wins reader (including cJSON's own `cJSON_GetObjectItem`) keeps the prefixed one. A consumer can't reliably tell which `"error"` string it will get.

* Steps to Reproduce

```sh
# terminal 1 — server that refuses tests longer than 2s
iperf3 -s -1 --server-max-duration 2 -p 5301

# terminal 2 — client requests a 10s test, which exceeds the limit
iperf3 -c 127.0.0.1 -p 5301 -t 10 -J
```

The server relays `SERVER_ERROR` (`IEMAXSERVERTESTDURATIONEXCEEDED`) and the client's `-J` document ends with the two `"error"` members shown above.

* Root Cause

Two independent code paths each add `"error"` to the same `test->json_top`, and `cJSON_AddStringToObject()` *appends* a node rather than replacing an existing key:

1. `src/iperf_client_api.c:406` — the `SERVER_ERROR` case in `iperf_handle_message_client()` calls `iperf_err(test, "SERVER ERROR - %s", iperf_strerror(i_errno))`. `iperf_err()` writes the first `"error"` at `src/iperf_error.c:61`, then the handler returns -1.
2. The -1 unwinds into `cleanup_and_fail()`, which at `src/iperf_client_api.c:895` adds `"error"` a second time (the bare `iperf_strerror(i_errno)`) before calling `iperf_json_finish()`.

Neither path is aware the other has already annotated the object.

* Suggested Fix

Make the second write idempotent — e.g. in `cleanup_and_fail()` use `cJSON_ReplaceItemInObjectCaseSensitive()` (or check `cJSON_GetObjectItem(test->json_top, "error")` before adding), or have the `SERVER_ERROR` handler report via a non-JSON-emitting path so only `cleanup_and_fail()` writes the key. Either yields a single, unambiguous `"error"` member.

Contributor guide

Open the contributing guide

Research direction

Reproduce the duplicate-key output with the documented server and client commands, then inspect the SERVER_ERROR case in src/iperf_client_api.c:406, iperf_err() in src/iperf_error.c:61, and cleanup_and_fail() in src/iperf_client_api.c:895. The work is done when the JSON document contains one unambiguous top-level error member for the relayed server error.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
networking
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
85/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.