* Use map[string]struct{} for compression map in Len
map[string]int requires 8 bytes per entry to store the unused position
information.
* Add MsgLength benchmark with more RRs
* Pass dns.Compress explicitly to packBufferWithCompressionMap
* Avoid creating compression map for question only Msg
This idea was inspired by:
"Skip dname compression for replies with no answers."
https://www.nlnetlabs.nl/bugs-script/show_bug.cgi?id=235
* Continue compressing multiple questions
* Remove ErrTruncated from the library
ErrTruncated is removed. This (correctly) assume that a truncated
message will be fully formed. Any message that isn't fully formed will
return (most likely) an unpack error.
Any program using ErrTruncated will fail to compile when they update to
this version: this is by design: you're doing it wrong. For checking if
a message was truncated you should checked the msg.Truncated boolean;
assuming the unpack didn't fail.
Fixes#814
Signed-off-by: Miek Gieben <miek@miek.nl>
* Restore tests
Signed-off-by: Miek Gieben <miek@miek.nl>
When packDomainName is called with an escaped domain name and compress
being true, bs wasn't be truncated to the correct length and would
include garbage that would be included in the compression map.
When the dname is found in the compression map and compress is true,
this copy is as it will simply be overwritten later. This could provide
a very slight speedup.
* properly set extended rcode when packing
When calling `SetExtendedRcode`, we expect to get the full extended
rcode, not the rcode after we shift 4 bytes right.
* fix extended rcode
* fix TestOPTTtl test
* set error messages in TestPackExtendedBadCookie
* Set Rcode with extended rcode
* |=
* Set extended RCODE field to 0 when RCODE is not an extended one.
+ unittests
* Force setting extended rcode if we have an OPT available.
* go fmt + @tmthrgd comments
* comments and nits
* reformat comment
* Add test that srv.conns is empty in checkInProgressQueriesAtShutdownServer
* Track ResponseWriter Close without nil-ing tcp
* Remove LocalAddr and RemoteAddr panic after Close
This is no longer needed as the tcp field is no longer set to nil in
Close.
* Add more explicit WriteMsg panic after Close
Previously this would panic with `dns: Write called after Close` which
is obviously less clear.
* Panic if Hijack is called after Close
Previously this worked, but later calls to Write would panic. This is
more explicit.
* Return an error if Close called multiple times
Neither io.Closer, nor ResponseWriter, provide any guarantees about the
behaviour of multiple calls to Close. This was made explicit in
https://golang.org/cl/8575043 and in practice implementations differ
wildly.
This matches ShutdownContext which returns an error if called multiple
times.
* Check map len under lock in checkInProgressQueriesAtShutdownServer
* Correct error message in checkInProgressQueriesAtShutdownServer
* Remove panic-after-Close from Hijack
* Return errors, not panic, on Write after Close
* Revert "Require URLs for DOH addresses (#684)"
This reverts commit 8ccae88257.
* Revert "WIP: DNS-over-HTTPS support for Client.Exchange API (#671)"
This reverts commit 64746df23b.
Signed-off-by: Miek Gieben <miek@miek.nl>
* Finish revert of DoH
Signed-off-by: Miek Gieben <miek@miek.nl>
* Add back in the race condition comment
Signed-off-by: Miek Gieben <miek@miek.nl>
* Improve ParseZone tests
* Add new ZoneParser API
* Use the ZoneParser API directly in ReadRR
* Merge parseZoneHelper into ParseZone
* Make generate string building slightly more efficient
* Add SetDefaultTTL method to ZoneParser
This makes it possible for external consumers to implement ReadRR.
* Make $INCLUDE directive opt-in
The $INCLUDE directive opens a user controlled file and parses it as
a DNS zone file. The error messages may reveal portions of sensitive
files, such as:
/etc/passwd: dns: not a TTL: "root❌0:0:root:/root:/bin/bash" at line: 1:31
/etc/shadow: dns: not a TTL: "root:$6$<redacted>::0:99999:7:::" at line: 1:125
Both ParseZone and ReadRR are currently opt-in for backward
compatibility.
* Disable $INCLUDE support in ReadRR
ReadRR and NewRR are often passed untrusted input. At the same time,
$INCLUDE isn't really useful for ReadRR as it only ever returns the
first record.
This is a breaking change, but it currently represents a slight
security risk.
* Document the need to drain the ParseZone chan
* Cleanup the documentation of NewRR, ReadRR and ParseZone
* Document the ZoneParser API
* Deprecated the ParseZone function
* Add whitespace to ZoneParser.Next
* Remove prevName field from ZoneParser
This doesn't track anything meaningful as both zp.prevName and h.Name
are only ever set at the same point and to the same value.
* Use uint8 for ZoneParser.include field
It has a maximum value of 7 which easily fits within uint8.
This reduces the size of ZoneParser from 160 bytes to 152 bytes.
* Add setParseError helper to ZoneParser
* Surface $INCLUDE os.Open error in error message
* Rename ZoneParser.include field to includeDepth
* Make maximum $INCLUDE depth a const
* Add ParseZone and ZoneParser benchmarks
* Parse $GENERATE directive with a single ZoneParser
This should be more efficient than calling NewRR for each generated
record.
* Run go fmt on generate_test.go
* Add a benchmark for $GENERATE directives
* Use a custom reader for generate
This avoids the overhead and memory usage of building the zone string.
name old time/op new time/op delta
Generate-12 165µs ± 4% 157µs ± 2% -5.06% (p=0.000 n=25+25)
name old alloc/op new alloc/op delta
Generate-12 42.1kB ± 0% 31.8kB ± 0% -24.42% (p=0.000 n=20+23)
name old allocs/op new allocs/op delta
Generate-12 1.56k ± 0% 1.55k ± 0% -0.38% (p=0.000 n=25+25)
* Return correct ParseError from generateReader
The last commit made these regular errors while they had been
ParseErrors before.
* Return error message as string from modToPrintf
This is slightly simpler and they don't need to be errors.
* Skip setting includeDepth in generate
This sub parser isn't allowed to use $INCLUDE directives anyway.
Note: If generate is ever changed to allow $INCLUDE directives, then
this line must be added back. Without doing that, it would be
be possible to exceed maxIncludeDepth.
* Make generateReader errors sticky
ReadByte should not be called after an error has been returned, but
this is cheap insurance.
* Move file and lex fields to end of generateReader
These are only used for creating a ParseError and so are unlikely to be
accessed.
* Don't return offset with error in modToPrintf
Along for the ride, are some whitespace and style changes.
* Add whitespace to generate and simplify step
* Use a for loop instead of goto in generate
* Support $INCLUDE directives inside $GENERATE directives
This was previously supported and may be useful. This is now more
rigorous as the maximum include depth is respected and relative
$INCLUDE directives are now supported from within $GENERATE.
* Don't return any lexer tokens after read error
Without this, read errors are likely to be lost and become parse errors
of the remaining str. The $GENERATE code relies on surfacing errors from
the reader.
* Support $INCLUDE in NewRR and ReadRR
Removing $INCLUDE support from these is a breaking change and should
not be included in this pull request.
* Add test to ensure $GENERATE respects $INCLUDE support
* Unify TestZoneParserIncludeDisallowed with other tests
* Remove stray whitespace from TestGenerateSurfacesErrors
* Move ZoneParser SetX methods above Err method
* $GENERATE should not accept step of 0
If step is allowed to be 0, then generateReader (and the code it
replaced) will get stuck in an infinite loop.
This is a potential DOS vulnerability.
* Fix ReadRR comment for file argument
I missed this previosuly. The file argument is also used to
resolve relative $INCLUDE directives.
* Prevent test panics on nil error
* Rework ZoneParser.subNext
This is slightly cleaner and will close the underlying *os.File even
if an error occurs.
* Make ZoneParser.generate call subNext
This also moves the calls to setParseError into generate.
* Report errors when parsing rest of $GENERATE directive
* Report proper error location in $GENERATE directive
This makes error messages much clearer.
* Simplify modToPrintf func
Note: When width is 0, the leading 0 of the fmt string is now excluded.
This should not alter the formatting of numbers in anyway.
* Add comment explaining sub field
* Remove outdated error comment from generate
* Eliminate zlexer goroutine
This replaces the zlexer goroutine and channels with a zlexer struct
that maintains state and provides a channel-like API.
* Eliminate klexer goroutine
This replaces the klexer goroutine and channels with a klexer struct
that maintains state and provides a channel-like API.
* Merge scan into zlexer and klexer
This does result in tokenText existing twice, but it's pretty simple
and small so it's not that bad.
* Avoid using text/scanner.Position to track position
* Track escape within zlexer.Next
* Avoid zl.commt check on space and tab in zlexer
* Track stri within zlexer.Next
* Track comi within zlexer.Next
There is one special case at the start of a comment that needs to be
handled, otherwise this is as simple as stri was.
* Use a single token buffer in zlexer
This is safe as there is never both a non-empty string buffer and a
non-empty comment buffer.
* Don't hardcode length of zl.tok in zlexer
* Eliminate lex.length field
This is always set to len(l.token) and is only queried in a few places.
It was added in 47cc5b052d without any
obvious need.
* Add whitespace to klexer.Next
* Track lex within klexer.Next
* Use a strings.Builder in klexer.Next
* Simplify : case in klexer.Next
* Add whitespace to zlexer.Next
* Change for loop style in zlexer.Next and klexer.Next
* Surface read errors in zlexer
* Surface read errors from klexer
* Remove debug line from parseKey
* Rename tokenText to readByte
* Make readByte return ok bool
Also change the for loop style to match the Next for loops.
* Make readByte errors sticky
klexer.Next calls readByte separately from within the loop. Without
readByte being sticky, an error that occurs during that readByte call
may be lost.
* Panic in testRR if the error is non-nil
* Add whitespace and unify field setting in zlexer.Next
* Remove eof fields from zlexer and klexer
With readByte having sticky errors, this no longer needed. zl.eof = true
was also in the wrong place and could mask an unbalanced brace error.
* Merge zl.tok blocks in zlexer.Next
* Split the tok buffer into separate string and comment buffers
The invariant of stri > 0 && comi > 0 never being true was broken when
x == '\n' && !zl.quote && zl.commt && zl.brace != 0 (the
"If not in a brace this ends the comment AND the RR" block).
Split the buffer back out into two separate buffers to avoid clobbering.
* Replace token slices with arrays in zlexer
* Add a NewRR benchmark
* Move token buffers into zlexer.Next
These don't need to be retained across Next calls and can be stack
allocated inside Next. This drastically reduces memory consumption as
they accounted for nearly half of all the memory used.
name old alloc/op new alloc/op delta
NewRR-12 9.72kB ± 0% 4.98kB ± 0% -48.72% (p=0.000 n=10+10)
* Add a ReadRR benchmark
Unlike NewRR, this will use an io.Reader that does not implement any
methods aside from Read. In particular it does not implement
io.ByteReader.
* Avoid using a bufio.Reader for io.ByteReader readers
At the same time use a smaller buffer size of 1KiB rather than the
bufio.NewReader default of 4KiB.
name old time/op new time/op delta
NewRR-12 11.0µs ± 3% 9.5µs ± 2% -13.77% (p=0.000 n=9+10)
ReadRR-12 11.2µs ±16% 9.8µs ± 1% -13.03% (p=0.000 n=10+10)
name old alloc/op new alloc/op delta
NewRR-12 4.98kB ± 0% 0.81kB ± 0% -83.79% (p=0.000 n=10+10)
ReadRR-12 4.87kB ± 0% 1.82kB ± 0% -62.73% (p=0.000 n=10+10)
name old allocs/op new allocs/op delta
NewRR-12 19.0 ± 0% 17.0 ± 0% -10.53% (p=0.000 n=10+10)
ReadRR-12 19.0 ± 0% 19.0 ± 0% ~ (all equal)
ReadRR-12 11.2µs ±16% 9.8µs ± 1% -13.03% (p=0.000 n=10+10)
* Surface any remaining comment from zlexer.Next
* Improve comment handling in zlexer.Next
This both fixes a regression where comments could be lost under certain
circumstances and now emits comments that occur within braces.
* Remove outdated comment from zlexer.Next and klexer.Next
* Delay converting LF to space in braced comment
* Fixup TestParseZoneComments
* Remove tokenUpper field from lex
Not computing this for every token, and instead only
when needed is a substantial performance improvement.
name old time/op new time/op delta
NewRR-12 9.56µs ± 0% 6.30µs ± 1% -34.08% (p=0.000 n=9+10)
ReadRR-12 9.93µs ± 1% 6.67µs ± 1% -32.77% (p=0.000 n=10+10)
name old alloc/op new alloc/op delta
NewRR-12 824B ± 0% 808B ± 0% -1.94% (p=0.000 n=10+10)
ReadRR-12 1.83kB ± 0% 1.82kB ± 0% -0.87% (p=0.000 n=10+10)
name old allocs/op new allocs/op delta
NewRR-12 17.0 ± 0% 17.0 ± 0% ~ (all equal)
ReadRR-12 19.0 ± 0% 19.0 ± 0% ~ (all equal)
* Update ParseZone documentation to match comment changes
The zlexer code was changed to return comments more often, so update the
ParseZone documentation to match.