We can make the implementation more robust by adjusting our assumptions
and leverage explicit file modes for syncing. Additionally we going to
assume that we want to clean up and can't really recover if thos
operations (file close and removal) fail.
* utilise file mode for majority of concerns
* improve test coverage by covering more assumptions
* signature parity with ioutil.WriteFile
* always clean up
Replaces #160
Fixes#169
Fixes https://github.com/tendermint/tendermint/issues/1322
The previous code was very trusting assuming that
rational actors will use this code. However, Byzantine
actors don't care and in the case of the linked issue
negative lengths can be sent to this code unfettered
having been received from a peer.
This code is essentially just a sign change from
`==`
to
`<=`
and we've gutted out that attack by being more defensive.
We can make the implementation more robust by adjusting our assumptions
and leverage explicit file modes for syncing. Additionally we going to
assume that we want to clean up and can't really recover if thos
operations (file close and removal) fail.
* utilise file mode for majority of concerns
* improve test coverage by covering more assumptions
* signature parity with ioutil.WriteFile
* always clean up
Replaces #160
Fixes#134
Those functions are unused in the whole Tendermint Github
organization plus they were unnecessariy verbose and could
have been concisely replaced with
```go
func RightPadString(s string, totalLength uint) string {
return fmt.Sprintf("% *s", totalLength, s)
}
func LeftPadString(s string, totalLength uint) string {
return fmt.Sprintf("% -*s", totalLength, s)
}
```
delete them anyways
Fixes#169
Fixes https://github.com/tendermint/tendermint/issues/1322
The previous code was very trusting assuming that
rational actors will use this code. However, Byzantine
actors don't care and in the case of the linked issue
negative lengths can be sent to this code unfettered
having been received from a peer.
This code is essentially just a sign change from
`==`
to
`<=`
and we've gutted out that attack by being more defensive.
fix RepeatTimer memory leak (Refs #137)
* test case
* drain channels on reset
Leaking memory:
```
leaktest.go:144: leaktest: leaked goroutine: goroutine 116 [chan send]:
github.com/tendermint/tmlibs/common.(*RepeatTimer).fireRoutine(0xc42006a410, 0xc4203403c0, 0xc42031b2c0)
/go/src/github.com/tendermint/tmlibs/common/repeat_timer.go:160 +0x6e
created by github.com/tendermint/tmlibs/common.(*RepeatTimer).reset
/go/src/github.com/tendermint/tmlibs/common/repeat_timer.go:196 +0xe9
```
The alternative solution could be draining channels on the client side.
* add one more select instead of draining
thanks to Jae
Fixes https://github.com/tendermint/tmlibs/issues/145
Fixes https://github.com/tendermint/tmlibs/issues/146
The code in here has been fragile when it comes to nil
but these edge cases were never tested, although they've
showed up in the wild and were only noticed because
the reporter actually read the logs otherwise
we'd have never known.
This changes covers some of these cases and adds some tests.
Ethan BuchmanandGitHub
MohansonandGitHub
Thomas CorbièreandAnton Kaliaev
Jae Kwon
Alexander SimmerlandEthan Buchman
Emmanuel T OdekeandEthan Buchman
Zaki ManianandAnton Kaliaev
Emmanuel Odeke