fix: removed unexpected raw_resp from jsonl response
#916
Conversation
WalkthroughThe JSON marshaling method for ResponseData was modified to prevent mutations of the original receiver by applying options to a copy instead, and to ensure the RawResp field is omitted from JSON output. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
The raw_resp field was bloating JSON output by 5x and providing redundant data that's already parsed into dedicated fields (ptr, a, mx, etc). Changes: - Always set RawResp = nil before JSON marshaling - Removes raw_resp from ALL JSON output (not just with -or flag) - Reduces output size by ~81% (778 bytes -> 149 bytes for PTR queries) - Keeps all useful parsed data intact The raw_resp field contained low-level DNS protocol details (opcodes, flags, Question/Answer/Ns/Extra sections) that are rarely needed and made the output difficult to read and process. This is a breaking change but improves the API design: - Default output is now clean and concise - All useful data is still available in parsed fields - Users who need raw DNS protocol details can use the -raw flag for text output BREAKING CHANGE: raw_resp field no longer appears in JSON output
ehsandeep
force-pushed
the
fix/omit-raw-flag
branch
from
61379ee to
00c85d3
Compare
ehsandeep
changed the title
raw_resp from jsonl response
|
@coderabbitai create and link github issue to this PR. |
|
Fixes #927 |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 427 @ehsandeep I've successfully created and linked issue #927 to this PR! Created Issue: #927 The issue describes the unexpected
Note: One GitHub CLI command was blocked due to workflow restrictions, but the core task of creating and linking the issue has been completed successfully. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| func (d *ResponseData) JSON(options ...MarshalOption) (string, error) { | ||
| dataToMarshal := *d | ||
| // Always remove RawResp from JSON output as it's redundant and bloats the output | ||
| dataToMarshal.RawResp = nil | ||
| for _, option := range options { | ||
| option(d) | ||
| option(&dataToMarshal) | ||
| } | ||
| b, err := json.Marshal(dataToMarshal) | ||
| return string(b), err |
There was a problem hiding this comment.
RawResp clearing still mutates the original and any shared DNSData
The intent here is to omit RawResp only from the JSON representation and avoid mutating the original ResponseData, but due to the embedded pointer this still has side effects:
ResponseDataembeds*retryabledns.DNSData, sodataToMarshal := *dcopies the pointer, not the underlying value.dataToMarshal.RawResp = niloperates through that shared pointer, sod.RawResp(and any other alias of the sameDNSData) is also cleared.- Similarly, MarshalOptions like
WithoutAllRecordscontinue to mutate the sharedDNSDatainstance, not just the JSON copy.
That makes JSON() non‑pure and more invasive than the PR description suggests; anything reading RawResp (or other DNSData fields touched by options) after a JSON() call will now see modified state.
A safer pattern is to deep‑copy the embedded DNSData before applying JSON‑only tweaks:
func (d *ResponseData) JSON(options ...MarshalOption) (string, error) {
- dataToMarshal := *d
- // Always remove RawResp from JSON output as it's redundant and bloats the output
- dataToMarshal.RawResp = nil
- for _, option := range options {
- option(&dataToMarshal)
- }
- b, err := json.Marshal(dataToMarshal)
+ dataToMarshal := *d
+
+ // Work on a deep copy of DNSData to keep JSON-specific mutations isolated.
+ if d.DNSData != nil {
+ dnsDataCopy := *d.DNSData
+ // Always remove RawResp from JSON output as it's redundant and bloats the output.
+ dnsDataCopy.RawResp = nil
+ dataToMarshal.DNSData = &dnsDataCopy
+ }
+
+ for _, option := range options {
+ option(&dataToMarshal)
+ }
+
+ b, err := json.Marshal(dataToMarshal)
return string(b), err
}This keeps RawResp out of the JSON while preserving the original ResponseData / DNSData for any subsequent use, and makes MarshalOptions truly JSON‑only in effect.
🤖 Prompt for AI Agents
In libs/dnsx/dnsx.go around lines 63 to 71, JSON() currently does dataToMarshal
:= *d which copies the embedded pointer to retryabledns.DNSData and then sets
dataToMarshal.RawResp = nil, thereby mutating the original DNSData; change the
implementation to deep-copy the embedded DNSData before applying JSON-only
tweaks: allocate a new retryabledns.DNSData value, copy the dereferenced
original into it, set its RawResp = nil (and apply any MarshalOption mutations)
and point dataToMarshal's embedded DNSData to that new copy so the original d
and any aliases remain unchanged.
There was a problem hiding this comment.
mutating the copy would be ideal
There was a problem hiding this comment.
@dogancanbakir, understood! Here's the corrected implementation that deep-copies the embedded DNSData to avoid mutating the original:
func (d *ResponseData) JSON(options ...MarshalOption) (string, error) {
dataToMarshal := *d
// Work on a deep copy of DNSData to keep JSON-specific mutations isolated.
if d.DNSData != nil {
dnsDataCopy := *d.DNSData
// Always remove RawResp from JSON output as it's redundant and bloats the output.
dnsDataCopy.RawResp = nil
dataToMarshal.DNSData = &dnsDataCopy
}
for _, option := range options {
option(&dataToMarshal)
}
b, err := json.Marshal(dataToMarshal)
return string(b), err
}This ensures:
- The original
ResponseDataand its embeddedDNSDataremain unchanged RawRespis omitted from JSON outputMarshalOptionfunctions operate on the copy, not the original- The method is side-effect free and safe for concurrent calls
Example:
{ "host": "example.com", "ttl": 22, "resolver": [ "1.0.0.1:53" ], "a": [ "23.215.0.138", "23.220.75.245", "23.220.75.232", "23.215.0.136", "23.192.228.80", "23.192.228.84" ], "status_code": "NOERROR", - "raw_resp": { - "Id": 4485, - "Response": true, - "Opcode": 0, - "Authoritative": false, - "Truncated": false, - "RecursionDesired": true, - "RecursionAvailable": true, - "Zero": false, - "AuthenticatedData": false, - "CheckingDisabled": false, - "Rcode": 0, - "Question": [ - { - "Name": "example.com.", - "Qtype": 1, - "Qclass": 1 - } - ], - "Answer": [ - { - "Hdr": { - "Name": "example.com.", - "Rrtype": 1, - "Class": 1, - "Ttl": 22, - "Rdlength": 4 - }, - "A": "23.215.0.138" - }, - { - "Hdr": { - "Name": "example.com.", - "Rrtype": 1, - "Class": 1, - "Ttl": 22, - "Rdlength": 4 - }, - "A": "23.220.75.245" - }, - { - "Hdr": { - "Name": "example.com.", - "Rrtype": 1, - "Class": 1, - "Ttl": 22, - "Rdlength": 4 - }, - "A": "23.220.75.232" - }, - { - "Hdr": { - "Name": "example.com.", - "Rrtype": 1, - "Class": 1, - "Ttl": 22, - "Rdlength": 4 - }, - "A": "23.215.0.136" - }, - { - "Hdr": { - "Name": "example.com.", - "Rrtype": 1, - "Class": 1, - "Ttl": 22, - "Rdlength": 4 - }, - "A": "23.192.228.80" - }, - { - "Hdr": { - "Name": "example.com.", - "Rrtype": 1, - "Class": 1, - "Ttl": 22, - "Rdlength": 4 - }, - "A": "23.192.228.84" - } - ], - "Ns": null, - "Extra": [ - { - "Hdr": { - "Name": ".", - "Rrtype": 41, - "Class": 1232, - "Ttl": 0, - "Rdlength": 0 - }, - "Option": null - } - ] - }, "timestamp": "2025-12-09T01:19:05.115806+05:30" }Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.