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

Makes NewNginxParser tolerant of log_format directives with newlines #44

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func (parser *Parser) ParseString(line string) (entry *Entry, err error) {
// the given log format.
func NewNginxParser(conf io.Reader, name string) (parser *Parser, err error) {
scanner := bufio.NewScanner(conf)
re := regexp.MustCompile(fmt.Sprintf(`^\s*log_format\s+%v\s+(.+)\s*$`, name))
re := regexp.MustCompile(fmt.Sprintf(`^\s*log_format\s+%s\s*(.*)\s*$`, name))
found := false
var format string
for scanner.Scan() {
Expand All @@ -81,13 +81,18 @@ func NewNginxParser(conf io.Reader, name string) (parser *Parser, err error) {
continue
}
found = true
// remove the "log_format <name>" from the line, leaving only the (potential)
// formatting directives.
line = formatDef[1]
} else {
line = scanner.Text()
}
// Look for a definition end
// Look for the end of the definition
re = regexp.MustCompile(`^\s*(.*?)\s*(;|$)`)
lineSplit := re.FindStringSubmatch(line)

// If there are any formatting directives on this line,
// add them to the format string
if l := len(lineSplit[1]); l > 2 {
format += lineSplit[1][1 : l-1]
}
Expand Down
44 changes: 37 additions & 7 deletions parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,9 @@ func TestParser(t *testing.T) {

Convey("Nginx format parser", func() {
expected := "$remote_addr - $remote_user [$time_local] \"$request\" $status \"$http_referer\" \"$http_user_agent\""
conf := strings.NewReader(`
http {

testCases := []string{
`http {
include conf/mime.types;
log_format minimal '$remote_addr [$time_local] "$request"';
log_format main '$remote_addr - $remote_user [$time_local] '
Expand All @@ -92,11 +93,40 @@ func TestParser(t *testing.T) {
'"$request" $status $bytes_sent '
'"$http_referer" "$http_user_agent" '
'"$http_range" "$sent_http_content_range"';
}
`)
parser, err := NewNginxParser(conf, "main")
So(err, ShouldBeNil)
So(parser.format, ShouldEqual, expected)
}`,

// Note that the line containing the log_format directive ("log_format main") has NO trailing whitespace.
// This is perfectly valid nginx config.
`http {
include conf/mime.types;
log_format minimal '$remote_addr [$time_local] "$request"';
log_format main
'$remote_addr - $remote_user [$time_local] '
'"$request" $status '
'"$http_referer" "$http_user_agent"';
}`,

// Similar to the above, but contains both trailing whitespace and empty lines in the log_format def
// This is perfectly valid nginx config.
`
http {
include conf/mime.types;
log_format minimal '$remote_addr [$time_local] "$request"';
log_format main

'$remote_addr - $remote_user [$time_local] '

'"$request" $status '

'"$http_referer" "$http_user_agent"';
}`,
}

for _, conf := range testCases {
parser, err := NewNginxParser(strings.NewReader(conf), "main")
So(err, ShouldBeNil)
So(parser.format, ShouldEqual, expected)
}
})
})
}