Skip to content

fix: removed unexpected raw_resp from jsonl response #916

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.

Sign up for GitHub

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

Merged
dogancanbakir merged 1 commit into from
Dec 9, 2025
Merged

fix: removed unexpected raw_resp from jsonl response #916

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion libs/dnsx/dnsx.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,10 @@ func WithoutAllRecords() MarshalOption {

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
Comment on lines 63 to 71
Copy link

@coderabbitai coderabbitai bot Dec 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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:

  • ResponseData embeds *retryabledns.DNSData, so dataToMarshal := *d copies the pointer, not the underlying value.
  • dataToMarshal.RawResp = nil operates through that shared pointer, so d.RawResp (and any other alias of the same DNSData) is also cleared.
  • Similarly, MarshalOptions like WithoutAllRecords continue to mutate the shared DNSData instance, 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.

Copy link
Member

@dogancanbakir dogancanbakir Dec 9, 2025

Choose a reason for hiding this comment

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

mutating the copy would be ideal

Copy link

@coderabbitai coderabbitai bot Dec 9, 2025

Choose a reason for hiding this comment

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

@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:

  1. The original ResponseData and its embedded DNSData remain unchanged
  2. RawResp is omitted from JSON output
  3. MarshalOption functions operate on the copy, not the original
  4. The method is side-effect free and safe for concurrent calls

Expand Down
Loading