NordSecurity / nordvpn-linux

NordVPN Linux client
GNU General Public License v3.0
295 stars 41 forks source link

Check isatty on terminal output #291

Closed Nickwiz closed 5 months ago

Nickwiz commented 6 months ago

The CLI tool output from some commands has preceding bytes even if piped. For example:

$ nordvpn status | xxd -g1 -c 12
00000000: 0d 2d 0d 20 20 0d 0d 2d 0d 20 20 0d  .-.  ..-.  .
0000000c: 53 74 61 74 75 73 3a 20 43 6f 6e 6e  Status: Conn
00000018: 65 63 74 65 64 0a 48 6f 73 74 6e 61  ected.Hostna

There is a series of \r-\r \r\r- ... written out. I have still to learn go and do not know the source base, but tracked down this:

https://github.com/NordSecurity/nordvpn-linux/blob/11c55f2693328766325f2710b1683051bd9afa5e/cli/loader.go

that might be the reason. I.e. it writes a series of - \ | / - \ | - ... to show that it is working (ASCII wheel of loading) which is OK in an interactive situation of course.

As of now I use something like this:

sed '1s/^[\r \|/-]*//'

to "fix" the output for consumption in for example awk, logging to files etc. E.g:

nordvpn status| sed '1s/^[\r \|/-]*//' | awk -vFS=':' '{printf("%-20s|%s\n", $1, $2)}'

Question is if one could do an isatty() check on print, and not print the loading sequence if it is not. That is a de-facto standard for terminal tools.

Not a critical issue unless these bytes are written to log-files or other places it should not - but better safe then sorry.

mariusSincovici commented 6 months ago

Hi, thanks for reporting the issue. I'll register a bug for this on our side. We'll also check the other commands to see if we have similar issues.

keliramu commented 5 months ago

Hi, @Nickwiz Check this commit: https://github.com/NordSecurity/nordvpn-linux/commit/f2fbbb4dd87683f67343a08e448f670d42b71903 It should fix a problem reported by you. Binary release coming soon.