Handling signals in terminal.ReadPassword

[Tibor]

Hi,

Currently, terminal.ReadPassword has a defer function to "restore" the termios's initial state.

However, that defer gets not called when a SIGINT is fired. I'd argue that doing Ctrl+C is a common thing when the enduser is prompted something (especially a password).

How could we deal with that?

1. Handle signals within terminal package

2. Add a GetState() function so that the user of the package has the responsibility to deal with that case.

I did #2 and wanted some feedback from you. This is what I added to the terminal package on my local machine:

// GetState returns the state of the given terminal.

func GetState(fd int) (*State, error) {

        var state State

        if _, _, err := syscall.Syscall6(syscall.SYS_IOCTL, uintptr(fd), uintptr(syscall.TCGETS), uintptr(unsafe.Pointer(&state.termios)), 0, 0, 0); err != 0 {

                return nil, err

        }

        return &state, nil

}

This way, one can just do the following:

        fd := os.Stdin.Fd()

        oldState, err := terminal.GetState(fd)

        if err != nil {

                panic("Could not get state of terminal: " + err.Error())

        }
        defer terminal.Restore(fd, oldState)

        sigch := make(chan os.Signal, 1)

        signal.Notify(sigch, os.Interrupt)

        go func() {

                for _ = range sigch {

                        terminal.Restore(fd, oldState)

                        os.Exit(1)

                }

        }()

        p, err := terminal.ReadPassword(fd)
        fmt.Println()
        if err != nil {

                panic("Failed to read password: " + err.Error())

        }

        // string(p) is now the password

What do you think?

Thanks,

Tibor

[Tibor]

Sorry, I forgot to say that the reason why I need the termios to be reset to the initial state is because, when my program is SIGINT'd, my terminal still has the ECHO off option which is rather annoying for the user.

[agl]

On Wednesday, December 19, 2012 10:56:09 PM UTC-5, Tibor wrote:

2. Add a GetState() function so that the user of the package has the responsibility to deal with that case.

You shouldn't need a GetState function.

If you're using this with a local terminal then you have a MakeRaw call somewhere, no? That returns the state that you can restore.

I think the larger question is why SIGINT doesn't cause an unwind. I don't know whether a signal handler for SIGINT has been considered before.

Cheers

AGL 

[Ian Lance Taylor]

On Thu, Dec 20, 2012 at 7:54 AM, agl <a...@golang.org> wrote:
>
> I think the larger question is why SIGINT doesn't cause an unwind. I don't
> know whether a signal handler for SIGINT has been considered before.

SIGINT shouldn't cause an unwind. If you want to handle SIGINT or
other signals, call os/signals.Notify and get the signals delivered to
you on a channel.

http://golang.org/pkg/os/signal/

Ian

[Tibor Vass]

Yes, that is the reason why I handled it myself with signals.

However, the problem I have with MakeRaw is that it somehow does not work with ReadPassword. Once the terminal is MakeRaw'd, every single keystroke makes ReadPassword return. So I actually don't want to do a MakeRaw, I just want to get the current state of the Terminal and restore it after ReadPassword even when catching a SIGINT.

Tibor Vass

Ian

--

[agl]

On Thursday, December 20, 2012 3:17:14 PM UTC-5, Tibor wrote:

However, the problem I have with MakeRaw is that it somehow does not work with ReadPassword. Once the terminal is MakeRaw'd, every single keystroke makes ReadPassword return. So I actually don't want to do a MakeRaw, I just want to get the current state of the Terminal and restore it after ReadPassword even when catching a SIGINT.

ReadPassword should work when the terminal is raw but, even so, I think your GetState is sensible. I'm not sure whether Ian is correct in asserting that signals shouldn't unwind the stack by default, but let's assume that he is.

I've addressed both in https://codereview.appspot.com/6990043/, which is out for review.

Cheers

AGL

[Tibor Vass]

Thank you very much!

Tibor Vass

AGL

--