Skip to content

Conversation

syedazeez337
Copy link
Contributor

@syedazeez337 syedazeez337 commented Jul 9, 2025

What this PR does

This PR ensures SRV record names and targets preserve their original case in responses, aligning with RFC 6763 §4.1.1, which requires that service instance names be treated as user-friendly Unicode text and must retain casing and formatting.


What Changed

  • ✅ SRV target is no longer lowercased in Insert() (e.g. samba.home.arpa.)
  • ✅ SRV record name now bypasses forced lowercasing during insert
  • less() in plugin/file/tree/less.go now compares labels case-insensitively
  • ✅ Verified with manual test zone using dig
_smb._tcp                     PTR Home\032Media._smb._tcp.home.arpa.
Home\032Media._smb._tcp       SRV 0 0 445 samba.home.arpa.

Query:

dig @127.0.0.1 -p 53 SRV "Home\\032Media._smb._tcp.home.arpa."

Expected response:

Home\032Media._smb._tcp.home.arpa. 5 IN SRV 0 0 445 samba.home.arpa.

Why this is needed

Although DNS is case-insensitive, [RFC 6763](https://datatracker.ietf.org/doc/html/rfc6763#section-4.1.1) mandates that SRV record names used in service discovery retain original case and formatting, especially for mDNS, Bonjour, and DNS-SD clients.

Prior to this fix, CoreDNS returned lowercased instance names, breaking spec-compliant discovery clients.


Limitations

This PR does not preserve the casing of the query name, as it is already normalized (lowercased) by CoreDNS or miekg/dns before plugin processing. This fix ensures the record data (response) complies with the spec.


Related

Fixes #7237

@yongtang
Copy link
Member

@syedazeez337 will it make sense to add a test?

Copy link

codecov bot commented Jul 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.93%. Comparing base (93c57b6) to head (de6b23b).
Report is 1525 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7402      +/-   ##
==========================================
+ Coverage   55.70%   59.93%   +4.23%     
==========================================
  Files         224      273      +49     
  Lines       10016    18015    +7999     
==========================================
+ Hits         5579    10798    +5219     
- Misses       3978     6586    +2608     
- Partials      459      631     +172     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@syedazeez337 syedazeez337 force-pushed the fix-preserve-case-srv branch from 84dc450 to 4eeb77c Compare July 11, 2025 04:56
@syedazeez337
Copy link
Contributor Author

syedazeez337 commented Jul 11, 2025

Hi @yongtang, I have added the test case for my code changes. Let me know if it rightly addresses the code guidelines of the organisation. Happy to contribute the changes needed. Thank you.
image
Also I have addressed the format issue that was failing it should pass now.

@syedazeez337 syedazeez337 force-pushed the fix-preserve-case-srv branch from 4eeb77c to 586d04d Compare July 12, 2025 08:52
@syedazeez337
Copy link
Contributor Author

Screenshot 2025-07-12 142413 I have fixed the linting issues, it should pass now. Let me know if the logic checks out. Thank you.

Signed-off-by: Syed Azeez <syedazeez337@gmail.com>
@syedazeez337 syedazeez337 force-pushed the fix-preserve-case-srv branch from 586d04d to de6b23b Compare July 13, 2025 05:46
@syedazeez337
Copy link
Contributor Author

syedazeez337 commented Jul 13, 2025

Hi @yongtang, I have resolved all the issues that were failing on the github Checks. I will wait for your review on the logic. Thank you.

@yongtang yongtang merged commit d8906ce into coredns:master Jul 15, 2025
13 checks passed
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.

auto: Does not preserve case of SRV replies
2 participants