commands: print IP addresses in error logs #2570
Conversation
commands/commands.go
Outdated
| continue | ||
| } | ||
| s = s + ip.String() + " " | ||
| } |
There was a problem hiding this comment.
I got heavily inspired here: https://stackoverflow.com/a/23558495 ;-)
There was a problem hiding this comment.
@larsxschneider great work here, I have a few comments below, but after addressing those this should be ready to merge.
| case *net.IPAddr: | ||
| ip = v.IP | ||
| } | ||
| if ip == nil || ip.IsLoopback() { |
There was a problem hiding this comment.
Should this check go before the switch above? You could have something like (*net.IPNet)(nil), which would panic on L362 after trying to dereference it.
There was a problem hiding this comment.
I am not sure if I understand you right, but this doesn't panic:
addr = nil
var ip net.IP
switch v := addr.(type) {
case *net.IPNet:
ip = v.IP
case *net.IPAddr:
ip = v.IP
}
There was a problem hiding this comment.
😱 totally disregard my suggestion -- I misread your code as if addr == nill || addr.IsLoopback(), instead of ip. This is all good 👍
commands/commands.go
Outdated
| if ip == nil || ip.IsLoopback() { | ||
| continue | ||
| } | ||
| s = s + ip.String() + " " |
There was a problem hiding this comment.
Instead of adding a " " on the end of s here, could you use a []string and call strings.Join(slice, " ") to prevent the trailing whitespace?
It can be hard to trace Git LFS errors on the server without client IP address. Fix this by adding the IP address to the error log output.
larsxschneider
force-pushed
the
lars/ips
branch
from
4755072 to
97cbac2
Compare
|
@ttaylorr I fixed the string issue in a rebase! |
It can be hard to trace Git LFS errors on the server without client IP
address. Fix this by adding the IP address to the error log output.
Attention
Is this OK from a privacy perspective? I think it is OK as we log the file to the local machine that knows the IP address anyways. A user that is concerned about the IP address could remove it from an error report. The same is already the case if the error output contains sensitive filenames of a repo.