Skip to content

Makes NewNginxParser tolerant of log_format directives with newlines#44

Open
dcarney wants to merge 2 commits intosatyrius:masterfrom
dcarney:dcarney/gonx-nginx-conf-parsing
Open

Makes NewNginxParser tolerant of log_format directives with newlines#44
dcarney wants to merge 2 commits intosatyrius:masterfrom
dcarney:dcarney/gonx-nginx-conf-parsing

Conversation

@dcarney
Copy link

@dcarney dcarney commented Jun 4, 2019

This PR makes slight modifications to the regex used in the NewNginxParser func, so that it correctly parses nginx log_format directives that contain newlines after the name.

This is best illustrated with an example.

This is perfectly valid nginx config (elided for brevity):

http {
  ...

  log_format main 
    '$remote_addr - $remote_user [$time_local] '
    '"$request" $status '
    '"$http_referer" "$http_user_agent"';

  ... 
}

Previously, the NewNginxParser regex would not properly match the log_format main line in such a case, since it expected to match one or more characters in a capture group before the end of the line. This would work if we "hacked" the line to include trailing whitespace, but that solution is error-prone and literally invisible to the naked eye. 😄

This PR changes that, and makes one more trivial change to that particular regex: it uses the standard %s formatting verb when building the regex, rather than the %v.

The rest of the PR consists of comments and additional test cases.

Let me know if you have any concerns, or if you'd like further modifications to be made. Thanks for looking!

@satyrius
Copy link
Owner

Thanks the the fix. Could you please make Travis green before we merge it?

@bobby-stripe
Copy link

to swoop in here, I just looked - I think the Travis troubles are larger/independent of this PR and due to Go 1.13 modules (only tip is failing, and the last numbered Go version tested is Go 1.5):

$ make examples
go run ./example/common/common.go
unexpected directory layout:
	import path: _/home/travis/gopath/src/github.com/satyrius/gonx
	root: /home/travis/gopath/src
	dir: /home/travis/gopath/src/github.com/satyrius/gonx
	expand root: /home/travis/gopath
	expand dir: /home/travis/gopath/src/github.com/satyrius/gonx
	separator: /
make: *** [examples] Error 1
The command "make examples" exited with 2.

@dcarney
Copy link
Author

dcarney commented Jun 24, 2019

@satyrius What do you think about the comment above from @bobby-stripe ?

It does appear that the CI failure is independent of my particular PR changes. Also, it would likely be helpful to change the CI build to test new Go versions > 1.5. I think that should be it's own PR however, instead of being co-mingled into this PR.

Let me know what you think, so that we can keep progress moving towards getting this PR merged. Thanks!

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.

3 participants