Skip to content
This repository has been archived by the owner on May 21, 2024. It is now read-only.

Conversation

haimrubinstein
Copy link

following this issue.

in the new standard (RFC 5424) it stated that 'Facility and Severity values are not normative but often used.' meaning that the pri header is not mandatory.
as a result, multiple products are sending Syslog messages without the pri header.
Also, RFC3164 in the pri header section talks about relay messages and implies that it might not be mandatory in other non-relaying options.

so I added a new option to allow skipping the pri header.
this is highly important for our use case.

@haimrubinstein
Copy link
Author

haimrubinstein commented Jan 7, 2024

@leodido @powersj @jdstrand I saw that you were the last to comment/confirm something on this repository. I would greatly appreciate it if one of you could spare a few minutes for this

Copy link

@powersj powersj left a comment

Choose a reason for hiding this comment

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

Hi @haim6678,

I've only landed a security fix for this repo ;) so can't say I know the content or structure very well.

Unfortunately, there do not seem to be any automated CI tess, but I did check out your branch and ran make build && make tests, and seemed to run into a couple failures:

❯ make tests
env GOTRACEBACK=all GO111MODULE=on go test  ./...
?   	github.com/influxdata/go-syslog/v3	[no test files]
ok  	github.com/influxdata/go-syslog/v3/common	(cached)
ok  	github.com/influxdata/go-syslog/v3/nontransparent	0.419s
?   	github.com/influxdata/go-syslog/v3/testing	[no test files]
ok  	github.com/influxdata/go-syslog/v3/octetcounting	0.666s
--- FAIL: Example_currentyear (0.00s)
got:
(*rfc3164.SyslogMessage)({
 Base: (syslog.Base) {
  Facility: (*uint8)(1),
  Severity: (*uint8)(5),
  Priority: (*uint8)(13),
  Timestamp: (*time.Time)(2024-12-02 16:31:03 +0000 UTC),
  Hostname: (*string)((len=4) "host"),
  Appname: (*string)((len=3) "app"),
  ProcID: (*string)(<nil>),
  MsgID: (*string)(<nil>),
  Message: (*string)((len=4) "Test")
 }
})
want:
(*rfc3164.SyslogMessage)({
 Base: (syslog.Base) {
  Facility: (*uint8)(1),
  Severity: (*uint8)(5),
  Priority: (*uint8)(13),
  Timestamp: (*time.Time)(2021-12-02 16:31:03 +0000 UTC),
  Hostname: (*string)((len=4) "host"),
  Appname: (*string)((len=3) "app"),
  ProcID: (*string)(<nil>),
  MsgID: (*string)(<nil>),
  Message: (*string)((len=4) "Test")
 }
})
FAIL
FAIL	github.com/influxdata/go-syslog/v3/rfc3164	0.004s
--- FAIL: TestParserParse (0.00s)
    --- FAIL: TestParserParse/1_2003-10-11T22:14:15.003Z_mymachine.example.com_s (0.00s)
        parser_test.go:39: 
            	Error Trace:	parser_test.go:39
            	Error:      	Expected nil, but got: &errors.errorString{s:"expecting a priority value within angle brackets [col 0]"}
            	Test:       	TestParserParse/1_2003-10-11T22:14:15.003Z_mymachine.example.com_s
        parser_test.go:40: 
            	Error Trace:	parser_test.go:40
            	Error:      	Should NOT be empty, but was <nil>
            	Test:       	TestParserParse/1_2003-10-11T22:14:15.003Z_mymachine.example.com_s
        parser_test.go:45: 
            	Error Trace:	parser_test.go:45
            	Error:      	Not equal: 
            	            	expected: *rfc5424.SyslogMessage(&rfc5424.SyslogMessage{Base:syslog.Base{Facility:(*uint8)(0xc00012ac79), Severity:(*uint8)(0xc00012ac7a), Priority:(*uint8)(0xc00012ac78), Timestamp:time.Date(2003, time.October, 11, 22, 14, 15, 3000000, time.UTC), Hostname:(*string)(0xc00011eae0), Appname:(*string)(0xc00011eaf0), ProcID:(*string)(nil), MsgID:(*string)(0xc00011eb00), Message:(*string)(0xc00011eb10)}, Version:0x1, StructuredData:(*map[string]map[string]string)(nil)})
            	            	actual  : <nil>(<nil>)
            	Test:       	TestParserParse/1_2003-10-11T22:14:15.003Z_mymachine.example.com_s
FAIL
FAIL	github.com/influxdata/go-syslog/v3/rfc5424	0.025s
FAIL
make: *** [makefile:50: tests] Error 1

For what it is worth, it looks like Example_currentyear fails on develop branch as well, but not the others.

@haimrubinstein
Copy link
Author

it doesn't seem related to my changes, as I didn't change the parser code.
but I'll try to run it locally

@haimrubinstein
Copy link
Author

@powersj
thank you very much for taking a look.
the issue was that the parser test was using the machine test cases. it seems to be OK now.

image

@haimrubinstein
Copy link
Author

haimrubinstein commented Jan 18, 2024

BTW, how do you build the project? running 'make build' doesn't seem to work for me. it's not running the ' ragel -I common -Z -G2 -e -o rfc3164/machine.go rfc3164/machine.go.rl' command and as a result, I don't see my changes reflecting in the machine.go file.
only after running the command manually it seems to work.

@powersj
Copy link

powersj commented Jan 22, 2024

the issue was that the parser test was using the machine test cases. it seems to be OK now.

Thanks for looking into it!

BTW, how do you build the project? running 'make build' doesn't seem to work for me. it's not running...

heh, I basically ran make build and make tests when I took my first look at this.

@leodido
Copy link
Collaborator

leodido commented May 19, 2024

image

I don't have anymore write/maintain access here. So, I can't help/maintain it. It's unfortunate but it is what it is... Feel free to redirect this PR to https://github.com/leodido/go-syslog. I plan to keep evolving this project there, on my GitHub. Thank you

@powersj
Copy link

powersj commented May 20, 2024

I don't have anymore write/maintain access here. So, I can't help/maintain it. It's unfortunate but it is what it is...

Your last commit was from 3 years ago :) If this is something you are interested in picking back up again, then we can revisit that. However, happy to see you pick up your own fork as well.

@tkyocum tkyocum closed this May 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants