Davidgamba/expose message struct fields

[DavidGamba]

Depends on https://github.com/nathanaelle/syslog5424/pull/5 to be able to see the actual changes.

[nathanaelle]

I'm ok with : – renaming Stamp -> Timestamp – the getters idea.

But I need to clarify my thinking because something annoy me here with the use of "Get" or "Set" prefix. It's some terminology / semantic issue not technical issue. I wanted to redesign this point for severals months…

The redesign will also permit to implement some RFC to add to the library features like crypto signature and improved Structured Data.

[nathanaelle]

to explain the main issue as a security design issue :

the same message can't be in the same time readable and writable.

1. if you write the message, you don't need to read it because you already know what's it says.
2. if you read a message, for security/integrity reason, you must not modify it, you may in some case only append it.

My actual code annoys me because it doesn't respect this design. So, my idea of redesign was to separate these both cases.

I may provide an implementation in a separate branch in few days.

Do you have any comment, suggestion, idea, warning, etc , about this new design ?

[DavidGamba]

But I need to clarify my thinking because something annoy me here with the use of "Get" or "Set" prefix. It's some terminology / semantic issue not technical issue.

I based the naming on the golang best practices: https://golang.org/doc/effective_go.html#Getters Basically, no Get prefix for getters but do add the Set prefix for setters.

1. if you write the message, you don't need to read it because you already know what's it says.

Don't know if this is totally necessary, I haven't personally ran into issues where this is needed but I don't do any security heavy coding myself.

I would need to better understand the case where this is needed to provide any feedback.

2. if you read a message, for security/integrity reason, you must not modify it, you may in some case only append it.

I do see value in this. An immutable structure where if you want to append or modify you create a new one.

the same message can't be in the same time readable and writable.

Seems like this would require two separate message types (sharing the same underlying struct), something like WriteMessage that doesn't allow reads and ReadOnlyMessage that doesn't allow mods.

But with the points above, I would probably settle with Message that allows for read and write, and ImmutableMessage.

Hope that helps!

[DavidGamba]

NOTE: Since you merged #5 I rebased this PR so it is easier to see the changes on the github UI.

[nathanaelle]

I began to write a patch with ImmutableMessage.

the new version allow me to finish the decoding of structured datas.

[nathanaelle]

I will push this PoC before the end of the week in a separate branch.

[DavidGamba]

Since you are going to change the way the library works, you should tag a release, maybe v0.1.0, to ensure that people relying on the old version have a way to keep using that.

[nathanaelle]

I pushed my PoC.

Known issues : – All the new import paths are the ugly "relative path" form (but easier to use in case of local test). – remove the joke in a dead code – there is some deprecated code (moved with git mv)

[nathanaelle]

timeQuality is a structured data specified in the RFC 5424. I wrote this to provide an working example of structured data.

[DavidGamba]

It is looking great so far.

I had to fix the import paths to run the tests and use it (but you had already mentioned it).

The only gotcha so far is that the methods of the Priority struct work on the pointer, so I had to create a copy of it before being able to access it (for a MessageImmutable response from the receiver.Receive()).

Basically, I had to:

priority := msg.Priority()
(&priority).String()

Instead of:

msg.Priority().String()

I'll verify that my old client code still works and let you know if I run into any other issues.

Thanks for a great library!

[nathanaelle]

I pushed some code and completed the roadmap in the readme.

the Priority follow now your suggestion and msg.Priority().String() works.

[DavidGamba]

You should create a WIP PR so it is easier to provide feedback and there is a little more visibility. We can also close this PR with that.