Repository navigation
Conversation
Since 0xERR0R#2102 responses that fit the client's buffer and 512 bytes uncompressed are sent without name compression. Some embedded stub resolvers (e.g. the Garmin Index scale) discard such responses. responseCompression.always compresses every response (pre-v0.32.0 behaviour), responseCompression.clients does so only for the listed IPs, CIDRs or client names. The default behaviour is unchanged. Refs 0xERR0R#2272
|
Thanks for the investigation and implementation proposal. Is there any reason not to always enable compression? Adding additional configuration is a kinda overkill imho. What do you mean? |
|
For me just a global enable/disable also would be OK. I just wanted to provide both options. |
|
If you want me to remove the per client option just tell me and I will do that 🙂 |
|
In order to keep the config simple, I would just enable compression without possibility to disable it |
|
But wasn't there a reason to just use compression for responses larger than 512 bytes in #2102 ? |
|
I think it was a review finding but if it causes other problems I dont see any objections to revert this change. I think it is pure technical stuff and we should not overload user with such configuration posibility. |
|
OK, I'm fine with that 🙂👍 |
Fixes #2272.
Since #2102, blocky lets
Truncatedecide whether a response is compressed, and #2213 re-enables compression for responses larger than 512 bytes. Small responses, e.g. a single A record, therefore go out without name compression. RFC 1035 §4.1.4 requires receivers to understand both forms, but some embedded stub resolvers don't: the Garmin Index scale from #2272 silently discards the uncompressed answer and retransmits the query until it gives up. Before v0.32.0 it worked, since every response was compressed.This PR adds an opt-in
responseCompressionsection. The default behaviour, including the #2102 optimization, is unchanged.clientsuses the same identifier forms asclientGroupsBlock(IP, CIDR, client name withfilepath.Matchwildcards, case-insensitive), except FQDN identifiers. That keeps the cost of compression limited to the devices that need it.alwaysis there for users who can't identify the affected client, or just want the old behaviour back.Implementation
config.ResponseCompression: IPs/CIDRs are parsed once invalidate(reusingparseCIDRorIPfrom the rate limiter); everything else is treated as a client name pattern. Empty entries are rejected.clientQuerygets aforceCompressfield, applied innormalizeResponseafterTruncateand the existing> dns.MinMsgSizerule.forceCompressis set inServer.resolveafter the resolver chain, becauserequest.ClientNamesis only populated by the client names resolver inside the chain.newClientQueryruns before that, so it can't be set there.The two options are independent (
Alwaysis a single||inForceFor,Clientsis its own field and matcher), so if you'd rather ship only one of them, the other can be dropped without touching the rest.Tests
config/response_compression_test.go: IP, CIDR (v4/v6), exact and wildcard names, case-insensitivity, IP identifiers not matching as names, empty entries rejected.server/server_test.go, underresponse compression: a single-A response is compressed withalways, for a matching IP and for a matching client name (set by the mocked chain, like the client names resolver would), and stays uncompressed for a non-matching client. Each case also checks the packed bytes for thec0 0cpointer. The three positive cases fail without the change inserver.go.docs/config.schema.jsonregenerated viago generate ./config/...; docs anddocs/config.ymlupdated.I ran the tests locally on Windows.
configandserverare green except for tests that also fail on an unmodifiedmainthere (Linux-specific error messages and unix sockets); CI will cover Linux.