starred/hickory-dns

Fork 0

mirror of https://github.com/hickory-dns/hickory-dns.git synced 2026-04-25 11:15:54 +03:00

[GH-ISSUE #1179] Name compression failures on corner cases #623

New issue

Closed

opened 2026-03-15 23:30:58 +03:00 by kerem · 10 comments

kerem commented

2026-03-15 23:30:58 +03:00

Owner

Originally created by @jacoblin1994 on GitHub (Aug 3, 2020).
Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/1179

Describe the bug
Name compression seems to fail on some corner cases, in this case, 'frontdoor.nest.com.' followed by several of its answer records 'frontdoor-srt01-production-1909587911.us-east-1.elb.amazonaws.com.' Strangely enough, if you swap the order presented to BinEncoder such that 'frontdoor.nest.com.' is placed at the end, the compression seems to be done correctly as expected.

To Reproduce
trust-dns/crates/proto/src/rr/domain/name.rs

#[test]
fn test_pointer() {
    let mut bytes: Vec<u8> = Vec::with_capacity(512);

    let first = Name::from_str("frontdoor.nest.com.").unwrap();
    let second = Name::from_str("frontdoor-srt01-production-1909587911.us-east-1.elb.amazonaws.com.").unwrap();
    let third = second.clone();

    {
        let mut e = BinEncoder::new(&mut bytes);

        first.emit(&mut e).unwrap();
        assert_eq!(e.len(), 20);

        second.emit(&mut e).unwrap();
        assert_eq!(e.len(), 84);

        third.emit(&mut e).unwrap();
        assert_eq!(e.len(), 86); // Should be 86 but get 148 instead that adds another 64 bytes
    }
}

Expected behavior
Expect Name/labels to be properly compressed but the result does not seem to suggest so.

Version:
Version: [0.7.3]

Originally created by @jacoblin1994 on GitHub (Aug 3, 2020). Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/1179 **Describe the bug** Name compression seems to fail on some corner cases, in this case, 'frontdoor.nest.com.' followed by several of its answer records 'frontdoor-srt01-production-1909587911.us-east-1.elb.amazonaws.com.' Strangely enough, if you swap the order presented to BinEncoder such that 'frontdoor.nest.com.' is placed at the end, the compression seems to be done correctly as expected. **To Reproduce** [trust-dns/crates/proto/src/rr/domain/name.rs](https://github.com/bluejekyll/trust-dns/blob/16f4e862ef158972967ab738c86d567d2938d899/crates/proto/src/rr/domain/name.rs) #[test] fn test_pointer() { let mut bytes: Vec<u8> = Vec::with_capacity(512); let first = Name::from_str("frontdoor.nest.com.").unwrap(); let second = Name::from_str("frontdoor-srt01-production-1909587911.us-east-1.elb.amazonaws.com.").unwrap(); let third = second.clone(); { let mut e = BinEncoder::new(&mut bytes); first.emit(&mut e).unwrap(); assert_eq!(e.len(), 20); second.emit(&mut e).unwrap(); assert_eq!(e.len(), 84); third.emit(&mut e).unwrap(); assert_eq!(e.len(), 86); // Should be 86 but get 148 instead that adds another 64 bytes } } **Expected behavior** Expect Name/labels to be properly compressed but the result does not seem to suggest so. **Version:** Version: [0.7.3]

kerem

2026-03-15 23:30:58 +03:00

closed this issue
added the
enhance

perf

crate:proto
labels

kerem commented

2026-03-15 23:31:14 +03:00

Author

Owner

@bluejekyll commented on GitHub (Aug 4, 2020):

woh. 0.7? is there a reason you're using such an old version?

There were some label compression issues in earlier releases, but current versions should be working correctly.

@bluejekyll commented on GitHub (Aug 4, 2020): woh. 0.7? is there a reason you're using such an old version? There were some label compression issues in earlier releases, but current versions should be working correctly.

kerem commented

2026-03-15 23:31:19 +03:00

Author

Owner

@jacoblin1994 commented on GitHub (Aug 4, 2020):

Thank you, Benjamin, for the reply.

I also did the testing on the latest version 0.19.5 as well, which is failing as well. This compression method, emit_as_canonical, has been untouched for the past 3 years or so if I understand it correctly. Could you please confirm if you are able to compress appropriately on the above specific queries?

@jacoblin1994 commented on GitHub (Aug 4, 2020): Thank you, Benjamin, for the reply. I also did the testing on the latest version 0.19.5 as well, which is failing as well. This compression method, [emit_as_canonical](https://github.com/bluejekyll/trust-dns/blob/16f4e862ef158972967ab738c86d567d2938d899/crates/proto/src/rr/domain/name.rs#L587), has been untouched for the past 3 years or so if I understand it correctly. Could you please confirm if you are able to compress appropriately on the above specific queries?

kerem commented

2026-03-15 23:31:24 +03:00

Author

Owner

@bluejekyll commented on GitHub (Aug 4, 2020):

It’s been a while since I’ve looked at this. It’s quite possible the logic hasn’t changed in 3 years.

I haven’t spent a ton of time optimizing the label compression, so it wouldn’t surprise me if there are some improvements available in this area.

@bluejekyll commented on GitHub (Aug 4, 2020): It’s been a while since I’ve looked at this. It’s quite possible the logic hasn’t changed in 3 years. I haven’t spent a ton of time optimizing the label compression, so it wouldn’t surprise me if there are some improvements available in this area.

kerem commented

2026-03-15 23:31:29 +03:00

Author

Owner

@bluejekyll commented on GitHub (Aug 4, 2020):

Looking at this a little more, I think the issue here is that we're not picking the best label compression option, only the first found. It feels like we should have two passes here rather than the single one that is happening today. Basically, search through all the labels, and look for the longest matching one.

@bluejekyll commented on GitHub (Aug 4, 2020): Looking at this a little more, I think the issue here is that we're not picking the best label compression option, only the first found. It feels like we should have two passes here rather than the single one that is happening today. Basically, search through all the labels, and look for the longest matching one.

kerem commented

2026-03-15 23:31:35 +03:00

Author

Owner

@jacoblin1994 commented on GitHub (Aug 4, 2020):

It looks like trust-dns-proto does not support labels ending with a pointer compression as in encoder.trim(), the associated name_pointers get removed completely due to the new offset being set.
https://tools.ietf.org/html/rfc1035#section-4.1.4

@jacoblin1994 commented on GitHub (Aug 4, 2020): It looks like trust-dns-proto does not support labels ending with a pointer compression as in encoder.trim(), the associated name_pointers get removed completely due to the new offset being set. https://tools.ietf.org/html/rfc1035#section-4.1.4

kerem commented

2026-03-15 23:31:40 +03:00

Author

Owner

@bluejekyll commented on GitHub (Aug 4, 2020):

That's definitely possible. Again, this needs to be optimized, and that hasn't happened. The code clarity in this area could be improved as well.

If you were interested in working on this, I'd definitely love the help.

@bluejekyll commented on GitHub (Aug 4, 2020): That's definitely possible. Again, this needs to be optimized, and that hasn't happened. The code clarity in this area could be improved as well. If you were interested in working on this, I'd definitely love the help.

kerem commented

2026-03-15 23:31:45 +03:00

Author

Owner

@jacoblin1994 commented on GitHub (Aug 5, 2020):

Thanks Benjamin. Happy to help with this.

I do have a working patch that I still want to take some time polishing.

@jacoblin1994 commented on GitHub (Aug 5, 2020): Thanks Benjamin. Happy to help with this. I do have a working patch that I still want to take some time polishing.

kerem commented

2026-03-15 23:31:50 +03:00

Author

Owner

@jacoblin1994 commented on GitHub (Aug 6, 2020):

Hi Benjamin, could you please add me as a collaborator, so that I can make a pull request? Thanks.

@jacoblin1994 commented on GitHub (Aug 6, 2020): Hi Benjamin, could you please add me as a collaborator, so that I can make a pull request? Thanks.

kerem commented

2026-03-15 23:31:55 +03:00

Author

Owner

@bluejekyll commented on GitHub (Aug 6, 2020):

Usually folks fork the repo in Github, and then submit PRs from their fork in Github, that doesn't require collaborator status: https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/creating-a-pull-request-from-a-fork

@bluejekyll commented on GitHub (Aug 6, 2020): Usually folks fork the repo in Github, and then submit PRs from their fork in Github, that doesn't require collaborator status: https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/creating-a-pull-request-from-a-fork

kerem commented

2026-03-15 23:32:00 +03:00

Author

Owner

@jacoblin1994 commented on GitHub (Aug 6, 2020):

Sounds good. I will follow this. Thanks.

@jacoblin1994 commented on GitHub (Aug 6, 2020): Sounds good. I will follow this. Thanks.

kerem referenced this issue

2026-03-16 02:13:16 +03:00

[PR #623] [MERGED] Support Windows hosts file when system configuration is used #1562