extend type t to contain tag and message (both type string), fixes #18

[hannesm]

use rresult and result types (instead of option)

• this also removes two failwith (Invalid_severity / Invalid_facility) by adjusting F|S_of_int to return an F|S option.
• I'm curious about the decode function checking for < 1025 -- is this a limit only for UDP or also for other transports?

There's a comment in the code:

(* FIXME Provide default Ptime.t? Version bellow doesn't work. Option type
let parse ?(ctx={timestamp=(Ptime.of_date_time ((1970, 1, 1), ((0, 0,0), 0))); hostname="-"; set_hostname=false}) data =
*)

You can use Ptime.epoch for this -- but I still wonder what decode should accept!? a syslog packet without timestamp? without hostname? (I suspect the way forward is to move to 5424 ;)

in case this is accepted, I'm happy to adjust my logs-syslog with the revised types

EDIT: the type annotations in syslog_message.ml (: ('a, [> Rresult.R.msg ]) result) are necessary since otherwise the locally defined Error constructor (from type severity) is used.

[hannesm]

this PR splits the message part into tag and message part -- it uses the first N alphanumeric characters as bytes, if N>32, then it fails. on output, there's no separator inserted between tag and message -- should there be a space? or some other character? maybe only if the message does not start with a non-alphanum character?

[verbosemode]

Thanks for the changes and refactoring the code \o/

I'm curious about the decode function checking for < 1025 -- is this a limit only for UDP or also for other transports?

Maybe we should just removed it from decode. It might be better to have code in the individual transport or server implementation to specify what message size is acceptable.

I had just blindly added it without thinking too much, because section 4.1 in the RFC says the message must not exceed 1024 bytes. That would be more relevant to the "encode" function actually.

There's a comment in the code:

(* FIXME Provide default Ptime.t? Version bellow doesn't work. Option type
let parse ?(ctx={timestamp=(Ptime.of_date_time ((1970, 1, 1), ((0, 0,0), 0))); hostname="-"; set_hostname=false}) data =
*)

You can use Ptime.epoch for this -- but I still wonder what decode should accept!? a syslog packet without timestamp? without hostname?

This is indeed not straight forward. The whole ctx record just exists to accommodate for broken messages and to fix them according to section 4.3.2. If the timestamp of the incoming message is invalid, the info provided in ctx is used instead. Since RFC 3164 doesn't have a year in the timestamp, the user provided Ptime.t is also used to validate the incoming timestamp. Hence a good default would be to use Ptime_clock.now, which doesn't support Mirage afaik. I think we should remove the comment, because it might not be possible to have a sane default for timestamp without Ptime_clock.now.

(I suspect the way forward is to move to 5424 ;)
That is absolutely right. I'm channeling some motivation to continue the work on that branch :)

Since there is a tag field now, maybe we should also rename the message field in type t to content. According to the 4.1.3 the message field consists of a tag and a content field.

[hannesm]

makes sense, I pushed a commit which renames message to content :)

[hannesm]

also removed the length checks in decode, and tweaked encode to not trim the message by default. -- maybe we need to add documentation now that the caller has to check length invariants

[verbosemode]

on output, there's no separator inserted between tag and message -- should there be a space? or some other character? maybe only if the message does not start with a non-alphanum character?

According to the RFC, there has to be a separator between tag and content. Maybe defaulting to a space is a good idea, if the first character in the content field isn't non-alphanumeric.

4.1.3 - MSG Part of a syslog Packet

Any non-alphanumeric character will terminate the TAG field and will be assumed to be the starting character of the CONTENT field.

Example: https://tools.ietf.org/html/rfc3164#section-5.3

[hannesm]

@verbosemode done in 9f23be3

[hannesm]

ping

[verbosemode]

Sorry for the delay. I just have a short glimpse over the PR again and merge it afterwards