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

Allow use of fs.FS for $INCLUDE and wrap errors #1526

Merged
merged 2 commits into from
Jan 15, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion dnssec_keyscan.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ func parseKey(r io.Reader, file string) (map[string]string, error) {
k = l.token
case zValue:
if k == "" {
return nil, &ParseError{file, "no private key seen", l}
return nil, &ParseError{file: file, err: "no private key seen", lex: l}
}

m[strings.ToLower(k)] = l.token
Expand Down
2 changes: 1 addition & 1 deletion generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func (r *generateReader) parseError(msg string, end int) *ParseError {
l.token = r.s[r.si-1 : end]
l.column += r.si // l.column starts one zBLANK before r.s

return &ParseError{r.file, msg, l}
return &ParseError{file: r.file, err: msg, lex: l}
}

func (r *generateReader) Read(p []byte) (int, error) {
Expand Down
2 changes: 1 addition & 1 deletion privaterr.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ Fetch:

err := r.Data.Parse(text)
if err != nil {
return &ParseError{"", err.Error(), l}
return &ParseError{wrappedErr: err, lex: l}
}

return nil
Expand Down
67 changes: 46 additions & 21 deletions scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bufio"
"fmt"
"io"
"io/fs"
"os"
"path/filepath"
"strconv"
Expand Down Expand Up @@ -64,20 +65,26 @@ const (
// ParseError is a parsing error. It contains the parse error and the location in the io.Reader
// where the error occurred.
type ParseError struct {
file string
err string
lex lex
file string
err string
wrappedErr error
lex lex
}

func (e *ParseError) Error() (s string) {
if e.file != "" {
s = e.file + ": "
}
if e.err == "" && e.wrappedErr != nil {
e.err = e.wrappedErr.Error()
}
s += "dns: " + e.err + ": " + strconv.QuoteToASCII(e.lex.token) + " at line: " +
strconv.Itoa(e.lex.line) + ":" + strconv.Itoa(e.lex.column)
return
}

func (e *ParseError) Unwrap() error { return e.wrappedErr }

type lex struct {
token string // text of the token
err bool // when true, token text has lexer error
Expand Down Expand Up @@ -168,8 +175,9 @@ type ZoneParser struct {
// sub is used to parse $INCLUDE files and $GENERATE directives.
// Next, by calling subNext, forwards the resulting RRs from this
// sub parser to the calling code.
sub *ZoneParser
osFile *os.File
sub *ZoneParser
r io.Reader
fsys fs.FS

includeDepth uint8

Expand All @@ -188,7 +196,7 @@ func NewZoneParser(r io.Reader, origin, file string) *ZoneParser {
if origin != "" {
origin = Fqdn(origin)
if _, ok := IsDomainName(origin); !ok {
pe = &ParseError{file, "bad initial origin name", lex{}}
pe = &ParseError{file: file, err: "bad initial origin name"}
}
}

Expand Down Expand Up @@ -220,6 +228,12 @@ func (zp *ZoneParser) SetIncludeAllowed(v bool) {
zp.includeAllowed = v
}

// SetIncludeAllowedFS controls whether $INCLUDE directives are allowed, and
// specifies an [fs.FS] from which to open and read files.
func (zp *ZoneParser) SetIncludeAllowedFS(v bool, fsys fs.FS) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the fence if we need a whole new method for this. Should we give SetIncludeAllowed an optional fs.FS parameter instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a new method because I didn't want to break the method signature for existing library users.

We could change SetIncludeAllowed(v bool) to SetIncludeAllowed(v bool, fs ...fs.FS), which should be backwards compatible, but would need to decide how to handle multiple fs.FS being provided. (Try each one in sequence until $INCLUDE is found?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shipping separate methods as a non-breaking change, and then consolidating them if/when you release a v2 is an option.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack. Think my main objection is the overlap in functionality. Maybe a better idea is to have a seperate method to set the underlying FS? SetIncludeFS that just sets the FS

zp.includeAllowed, zp.fsys = v, fsys
}

// Err returns the first non-EOF error that was encountered by the
// ZoneParser.
func (zp *ZoneParser) Err() error {
Expand All @@ -237,7 +251,7 @@ func (zp *ZoneParser) Err() error {
}

func (zp *ZoneParser) setParseError(err string, l lex) (RR, bool) {
zp.parseErr = &ParseError{zp.file, err, l}
zp.parseErr = &ParseError{file: zp.file, err: err, lex: l}
return nil, false
}

Expand All @@ -260,9 +274,11 @@ func (zp *ZoneParser) subNext() (RR, bool) {
return rr, true
}

if zp.sub.osFile != nil {
zp.sub.osFile.Close()
zp.sub.osFile = nil
if zp.sub.r != nil {
if c, ok := zp.sub.r.(io.Closer); ok {
c.Close()
}
zp.sub.r = nil
}

if zp.sub.Err() != nil {
Expand Down Expand Up @@ -406,20 +422,29 @@ func (zp *ZoneParser) Next() (RR, bool) {
includePath = filepath.Join(filepath.Dir(zp.file), includePath)
}

r1, e1 := os.Open(includePath)
var r1 io.Reader
var e1 error
if zp.fsys != nil {
r1, e1 = zp.fsys.Open(includePath)
} else {
r1, e1 = os.Open(includePath)
}
if e1 != nil {
var as string
if !filepath.IsAbs(l.token) {
as = fmt.Sprintf(" as `%s'", includePath)
}

msg := fmt.Sprintf("failed to open `%s'%s: %v", l.token, as, e1)
return zp.setParseError(msg, l)
zp.parseErr = &ParseError{
file: zp.file,
wrappedErr: fmt.Errorf("failed to open `%s'%s: %w", l.token, as, e1),
lex: l,
}
return nil, false
}

zp.sub = NewZoneParser(r1, neworigin, includePath)
zp.sub.defttl, zp.sub.includeDepth, zp.sub.osFile = zp.defttl, zp.includeDepth+1, r1
zp.sub.SetIncludeAllowed(true)
zp.sub.defttl, zp.sub.includeDepth, zp.sub.r = zp.defttl, zp.includeDepth+1, r1
zp.sub.SetIncludeAllowedFS(true, zp.fsys)
return zp.subNext()
case zExpectDirTTLBl:
if l.value != zBlank {
Expand Down Expand Up @@ -1326,12 +1351,12 @@ func slurpRemainder(c *zlexer) *ParseError {
case zBlank:
l, _ = c.Next()
if l.value != zNewline && l.value != zEOF {
return &ParseError{"", "garbage after rdata", l}
return &ParseError{err: "garbage after rdata", lex: l}
}
case zNewline:
case zEOF:
default:
return &ParseError{"", "garbage after rdata", l}
return &ParseError{err: "garbage after rdata", lex: l}
}
return nil
}
Expand All @@ -1340,16 +1365,16 @@ func slurpRemainder(c *zlexer) *ParseError {
// Used for NID and L64 record.
func stringToNodeID(l lex) (uint64, *ParseError) {
if len(l.token) < 19 {
return 0, &ParseError{l.token, "bad NID/L64 NodeID/Locator64", l}
return 0, &ParseError{file: l.token, err: "bad NID/L64 NodeID/Locator64", lex: l}
}
// There must be three colons at fixes positions, if not its a parse error
if l.token[4] != ':' && l.token[9] != ':' && l.token[14] != ':' {
return 0, &ParseError{l.token, "bad NID/L64 NodeID/Locator64", l}
return 0, &ParseError{file: l.token, err: "bad NID/L64 NodeID/Locator64", lex: l}
}
s := l.token[0:4] + l.token[5:9] + l.token[10:14] + l.token[15:19]
u, err := strconv.ParseUint(s, 16, 64)
if err != nil {
return 0, &ParseError{l.token, "bad NID/L64 NodeID/Locator64", l}
return 0, &ParseError{file: l.token, err: "bad NID/L64 NodeID/Locator64", lex: l}
}
return u, nil
}