-
Notifications
You must be signed in to change notification settings - Fork 30
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
[WIP] Implement the auditor-server application layer #193
base: master
Are you sure you want to change the base?
Conversation
7117f50
to
be900a4
Compare
utils/binutils/encoding.go
Outdated
} | ||
|
||
// MarshalSTRToFile serializes the given STR to the given path. | ||
func MarshalSTRToFile(str *protocol.DirSTR, path string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Related: #137 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely. This particular function is just for the test implementation, but in implementing the auditor, having a persistent storage backend is becoming more and more necessary. We should get back to implementing that soon.
1ab0a2a
to
df1762c
Compare
94beaec
to
7466552
Compare
utils/binutils/logger.go
Outdated
@@ -0,0 +1,134 @@ | |||
package binutils |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already have application/logger.go
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugh, thanks for catching that. I thought I removed the binutils
package when I rebased.
…into auditor-server-cli
application/encoding.go
Outdated
@@ -77,8 +77,12 @@ func UnmarshalResponse(t int, msg []byte) *protocol.Response { | |||
Error: res.Error, | |||
} | |||
if err := response.Validate(); err != nil { | |||
return &protocol.Response{ | |||
Error: protocol.ErrMalformedMessage, | |||
// we don't want to return an ErrMalformedMessage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is covered in https://github.com/coniks-sys/coniks-go/blob/master/protocol/message.go#L272
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but the problem is that this function was still returning an ErrMalformedMessage
even if the error is in errors
. In other words, because Validate()
in message.go returns msg.Error
, which gives err == nil
after Validate()
returns, the return &protocol.Response{Error: protocol.ErrMalformedMessage }
statement was always being called, even when the msg.Error
was in errors
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. I gave it a try in https://github.com/coniks-sys/coniks-go/compare/unmarshalling. If it's ok, feel free to merge to this branch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those changes looks good to me. Thank you! I also like that you re-wrote the test cases. I'll merge this branch when I get a chance.
Part of #151