Skip to content
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

Jump to bottom

Added WithTCPRetry option that fixes truncated answers #118

Closed
wants to merge 14 commits into from
Closed

Added WithTCPRetry option that fixes truncated answers #118

wants to merge 14 commits into from

Conversation

Miniwoffer
Copy link

@Miniwoffer Miniwoffer commented Jan 29, 2024 •
edited
Loading

I was unable to push more changes and re-open #113 , so i created a new MR instead.

@Integralist Integralist requested a review from ydnar January 29, 2024 08:54
Integralist
Integralist approved these changes Jan 29, 2024
View reviewed changes
Copy link
Contributor

@Integralist Integralist left a comment

Choose a reason for hiding this comment

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

This looks good to me but I've asked @ydnar to give the final 👍🏻 before merging.

cee-dub
cee-dub requested changes Jan 29, 2024
View reviewed changes
Copy link
Member

@cee-dub cee-dub left a comment

Choose a reason for hiding this comment

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

Thanks for adding the option and the test.

Miniwoffer and others added 2 commits January 29, 2024 21:49
@Miniwoffer @cee-dub
Update resolver.go
1582a7f 
Co-authored-by: Cameron Walters (cee-dub) <cameron.walters@gmail.com>
Fixed so timeout is used correctly on tcp retry
d18731d
@Miniwoffer Miniwoffer requested a review from cee-dub January 31, 2024 07:51
@Integralist Integralist changed the title Added WithTCPRetry option that fixses truncated answers Added WithTCPRetry option that fixes truncated answers Jan 31, 2024
@cee-dub
Copy link
Member

cee-dub commented Feb 12, 2024

I've added a flag to the CLI that lets us easily access the new functionality.

Sample output: 

$ go run ./cmd/dnsr/main.go -v google.com TXT
╭─── resolve("google.com.", "TXT", 1)
│   ╭─── resolve("google.com.", "NS", 2)
│   │   ╭─── resolve("com.", "NS", 3)
│   │   │    23ms (T- 1999ms): dig +norecurse @d.root-servers.net. com. NS  # rmsg: NOERROR Answer: 0 NS: 13 Extra: 15
│   │   ╰─── 23ms: resolve("com.", "NS", 3) # [26]RR = NS(com.)=a.gtld-servers.net. NS(com.)=b.gtld-servers.net. ...
        X    34ms (T- 1999ms): dig +norecurse @c.root-servers.net. com. NS  # rmsg: NOERROR Answer: 0 NS: 13 Extra: 15 == CANCELED ==
│   │    87ms (T- 1976ms): dig +norecurse @a.gtld-servers.net. google.com. NS  # rmsg: NOERROR Answer: 0 NS: 4 Extra: 8
│   ╰─── 111ms: resolve("google.com.", "NS", 2) # [8]RR = NS(google.com.)=ns2.google.com. NS(google.com.)=ns1.google.com. ...
    X    94ms (T- 1976ms): dig +norecurse @b.gtld-servers.net. google.com. NS  # rmsg: NOERROR Answer: 0 NS: 4 Extra: 8 == CANCELED ==
│    25ms (T- 1888ms): dig +norecurse @ns1.google.com. google.com. TXT  # rmsg: NOERROR Answer: 6 NS: 0 Extra: 0
╰─── 137ms: resolve("google.com.", "TXT", 1) # [10]RR = TXT(google.com.)=atlassian-domain-verification=5YjTmWmjI92ewqkx2oXmBaD60Td9zWon9r6eakvHX6B77zzkFQto8PQ9QsKnbf4I TXT(google.com.)=google-site-verification=TV9-DBe4R80X4v0M4U_bd_J9cpOJM0nikft0jAgjmsQ ...

;; RESULTS:
google.com.	      3600	IN	TXT	atlassian-domain-verification=5YjTmWmjI92ewqkx2oXmBaD60Td9zWon9r6eakvHX6B77zzkFQto8PQ9QsKnbf4I
google.com.	      3600	IN	TXT	google-site-verification=TV9-DBe4R80X4v0M4U_bd_J9cpOJM0nikft0jAgjmsQ
google.com.	      3600	IN	TXT	webexdomainverification.8YX6G=6e6922db-e3e6-4a36-904e-a805c28087fa
google.com.	      3600	IN	TXT	globalsign-smime-dv=CDYX+XFHUw2wml6/Gb8+59BsH31KzUr6c1l2BPvqKX8=
google.com.	      3600	IN	TXT	MS=E4A68B9AB2BB9670BCE15412F62916164C0B20BB
google.com.	      3600	IN	TXT	onetrust-domain-verification=de01ed21f2fa4d8781cbc3ffb89cf4ef
google.com.	      3600	IN	NS	ns2.google.com.
google.com.	      3600	IN	NS	ns1.google.com.
google.com.	      3600	IN	NS	ns3.google.com.
google.com.	      3600	IN	NS	ns4.google.com.
;; TRUE	google.com	TXT
;; Elapsed: 138.206625ms
$ go run ./cmd/dnsr/main.go -v -t google.com TXT
╭─── resolve("google.com.", "TXT", 1)
│   ╭─── resolve("google.com.", "NS", 2)
│   │   ╭─── resolve("com.", "NS", 3)
│   │   │    29ms (T- 1999ms): dig +norecurse @c.root-servers.net. com. NS  # rmsg: NOERROR Answer: 0 NS: 13 Extra: 15
│   │   ╰─── 30ms: resolve("com.", "NS", 3) # [26]RR = NS(com.)=f.gtld-servers.net. NS(com.)=h.gtld-servers.net. ...
│   │    32ms (T- 1969ms): dig +norecurse @h.gtld-servers.net. google.com. NS  # rmsg: NOERROR Answer: 0 NS: 4 Extra: 8
│   ╰─── 63ms: resolve("google.com.", "NS", 2) # [8]RR = NS(google.com.)=ns2.google.com. NS(google.com.)=ns1.google.com. ...
    X    38ms (T- 1969ms): dig +norecurse @f.gtld-servers.net. google.com. NS  # rmsg: NOERROR Answer: 0 NS: 4 Extra: 8 == CANCELED ==
        X    93ms (T- 1999ms): dig +norecurse @b.root-servers.net. com. NS  # rmsg: NOERROR Answer: 0 NS: 13 Extra: 12 == CANCELED ==
│    21ms (T- 1936ms): dig +norecurse @ns1.google.com. google.com. TXT  # rmsg: NOERROR Answer: 12 NS: 0 Extra: 0
╰─── 134ms: resolve("google.com.", "TXT", 1) # [16]RR = TXT(google.com.)=v=spf1 include:_spf.google.com ~all TXT(google.com.)=apple-domain-verification=30afIBcvSuDV2PLX ...

;; RESULTS:
google.com.	      3600	IN	TXT	v=spf1 include:_spf.google.com ~all
google.com.	      3600	IN	TXT	apple-domain-verification=30afIBcvSuDV2PLX
google.com.	      3600	IN	TXT	atlassian-domain-verification=5YjTmWmjI92ewqkx2oXmBaD60Td9zWon9r6eakvHX6B77zzkFQto8PQ9QsKnbf4I
google.com.	      3600	IN	TXT	globalsign-smime-dv=CDYX+XFHUw2wml6/Gb8+59BsH31KzUr6c1l2BPvqKX8=
google.com.	      3600	IN	TXT	onetrust-domain-verification=de01ed21f2fa4d8781cbc3ffb89cf4ef
google.com.	      3600	IN	TXT	docusign=05958488-4752-4ef2-95eb-aa7ba8a3bd0e
google.com.	      3600	IN	TXT	docusign=1b0a6754-49b1-4db5-8540-d2c12664b289
google.com.	      3600	IN	TXT	facebook-domain-verification=22rm551cu4k0ab0bxsw536tlds4h95
google.com.	      3600	IN	TXT	google-site-verification=TV9-DBe4R80X4v0M4U_bd_J9cpOJM0nikft0jAgjmsQ
google.com.	      3600	IN	TXT	MS=E4A68B9AB2BB9670BCE15412F62916164C0B20BB
google.com.	      3600	IN	TXT	google-site-verification=wD8N7i1JTNTkezJ49swvWW48f8_9xveREV4oB-0Hf5o
google.com.	      3600	IN	TXT	webexdomainverification.8YX6G=6e6922db-e3e6-4a36-904e-a805c28087fa
google.com.	      3600	IN	NS	ns2.google.com.
google.com.	      3600	IN	NS	ns1.google.com.
google.com.	      3600	IN	NS	ns3.google.com.
google.com.	      3600	IN	NS	ns4.google.com.
;; TRUE	google.com	TXT
;; Elapsed: 135.228541ms

cee-dub
cee-dub approved these changes Feb 12, 2024
View reviewed changes
Copy link
Member

@cee-dub cee-dub left a comment

Choose a reason for hiding this comment

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

This looks OK to me. @ydnar?

@cee-dub
Copy link
Member

cee-dub commented Feb 12, 2024

Sorry, merge/rebase got a little messy here and I didn't realize I was pushing to your fork. I intended to include your work in #118 and add the test from #112.

@cee-dub cee-dub mentioned this pull request Feb 12, 2024
Merged
@cee-dub
Copy link
Member

cee-dub commented Feb 12, 2024

Closing in favor of #120

@cee-dub cee-dub closed this Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cee-dub cee-dub cee-dub approved these changes

@Integralist Integralist Integralist approved these changes

@ydnar ydnar Awaiting requested review from ydnar

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Miniwoffer @cee-dub @Integralist