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.

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

Stop compressing names in RT records #847

Merged
merged 2 commits into from
Nov 30, 2018
Merged

Conversation

tmthrgd
Copy link
Collaborator

@tmthrgd tmthrgd commented Nov 30, 2018

Although RFC 1183 allows names in the RT record to be compressed with:

   The concrete encoding is identical to the MX RR.

But RFC 3597 specifically prohibits compressing names in any record not defined in RFC 1035.

Note: I had to modify the two generated files by hand as they weren't picking up my changes to types.go for some reason. There shouldn't be any changes running go generate later though.

Updates #522

Although RFC 1183 allows names in the RT record to be compressed with:
 "The concrete encoding is identical to the MX RR."

RFC 3597 specifically prohibits compressing names in any record not
defined in RFC 1035.
@tmthrgd tmthrgd requested a review from miekg November 30, 2018 00:02
@codecov-io
Copy link

codecov-io commented Nov 30, 2018

Codecov Report

Merging #847 into master will increase coverage by 0.06%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #847      +/-   ##
==========================================
+ Coverage    57.9%   57.96%   +0.06%     
==========================================
  Files          42       42              
  Lines       10661    10675      +14     
==========================================
+ Hits         6173     6188      +15     
+ Misses       3399     3398       -1     
  Partials     1089     1089
Impacted Files Coverage Δ
types.go 73.63% <ø> (ø) ⬆️
ztypes.go 45.48% <0%> (ø) ⬆️
zmsg.go 50.75% <100%> (ø) ⬆️
server.go 66.19% <0%> (+0.23%) ⬆️
msg.go 77.98% <0%> (+0.53%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 778aa4f...b0c52bd. Read the comment docs.

@miekg
Copy link
Owner

miekg commented Nov 30, 2018

you need to 'go install' at the right moment, because the generating code looks there for the new definition instead of the current dir with the new types.go

@tmthrgd
Copy link
Collaborator Author

tmthrgd commented Nov 30, 2018

@miekg Well that worked, I had no idea, how odd.

@miekg
Copy link
Owner

miekg commented Nov 30, 2018 via email

@tmthrgd tmthrgd merged commit 6b6e08b into miekg:master Nov 30, 2018
@tmthrgd tmthrgd deleted the rt-compression branch November 30, 2018 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants