Skip to content
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

Infinite Loop in the _read_init_initialization_line function #69

Open
MHC03 opened this issue Sep 20, 2017 · 6 comments
Open

Infinite Loop in the _read_init_initialization_line function #69

MHC03 opened this issue Sep 20, 2017 · 6 comments

Comments

@MHC03
Copy link

MHC03 commented Sep 20, 2017

When trying to Debug the Issue #66, my initial problem was, that I was stuck in an infinite Loop. This mainly was caused by having no specific setting for the tcsh Shell. But nevertheless the function _read_init_initialization_line does not catch one specific case where line may be False, that means that it may contain nothing. This case may not ever occur on a normal shell configuration, but it still should be catched:

def _read_int_initialization_line(output_file):
    while True:
        line = output_file.readline().strip()
        if line:  # <------------------------- line may be empty
            try:
                return int(line)
            except ValueError:
                raise CommandInitializationError(line)
@mristin
Copy link

mristin commented Oct 30, 2018

Hi,
I'm having the same issue. This is the code I call:

    with spur.SshShell(
            hostname='some_hostname', username='some_user',
            private_key_file=os.path.expanduser("~/.ssh/id_rsa")) as shell:
        shell.run(["echo", "oi"])

I sprinkled the spur code (spur/ssh.py) with prints:

    def spawn(self, command, *args, **kwargs):
        print("spur ssh spawn enter")
        stdout = kwargs.pop("stdout", None)
        stderr = kwargs.pop("stderr", None)
        allow_error = kwargs.pop("allow_error", False)
        store_pid = kwargs.pop("store_pid", False)
        use_pty = kwargs.pop("use_pty", False)
        encoding = kwargs.pop("encoding", None)
        cwd = kwargs.get('cwd')
        command_in_cwd = self._shell_type.generate_run_command(command, *args, store_pid=store_pid, **kwargs)
        try:
            print("spur ssh spawn: open session")
            channel = self._get_ssh_transport().open_session()
            print("spur ssh spawn: opened session")
        except EOFError as error:
            raise self._connection_error(error)
        if use_pty:
            channel.get_pty()
        print("spur ssh spawn: exec command")
        channel.exec_command(command_in_cwd)
        print("spur ssh spawn: execed command")

        print("spur ssh spawn: channel.makefile")
        process_stdout = channel.makefile('rb')
        print("spur ssh spawn: channel.makefile done")

        print("spur ssh spawn: store pid")
        if store_pid:
            pid = _read_int_initialization_line(process_stdout)
        print("spur ssh spawn: stored pid")

        print("spur ssh spawn: cwd")
        if cwd is not None:
            cd_output = []
            while True:
                line = process_stdout.readline()
                if line.startswith(b"spur-cd: "):
                    if line.strip() == b"spur-cd: 0":
                        break
                    else:
                        raise CouldNotChangeDirectoryError(cwd, b"".join(cd_output))
                else:
                    cd_output.append(line)

        print("spur ssh spawn: supports which")
        if self._shell_type.supports_which:
            print("spur ssh spawn: which_return_code")
            which_return_code = _read_int_initialization_line(process_stdout)

            print("spur ssh spawn: which_return_code done")

            if which_return_code != 0:
                raise NoSuchCommandError(command[0])

        print("spur ssh spawn: starting a process")
        process = SshProcess(
            channel,
            allow_error=allow_error,
            process_stdout=process_stdout,
            stdout=stdout,
            stderr=stderr,
            encoding=encoding,
            shell=self,
        )
        if store_pid:
            process.pid = pid
        
        return process

Here is the output I get (with paramiko DEBUG logs):

INFO:paramiko.transport:Authentication (publickey) successful!
DEBUG:paramiko.transport:[chan 0] Max packet in: 32768 bytes
DEBUG:paramiko.transport:[chan 0] Max packet out: 32768 bytes
DEBUG:paramiko.transport:Secsh channel 0 opened.
spur ssh spawn: opened session
spur ssh spawn: exec command
DEBUG:paramiko.transport:[chan 0] Sesch channel 0 request ok
spur ssh spawn: execed command
spur ssh spawn: channel.makefile
spur ssh spawn: channel.makefile done
spur ssh spawn: store pid
spur ssh spawn: stored pid
spur ssh spawn: cwd
spur ssh spawn: supports which
spur ssh spawn: which_return_code
DEBUG:paramiko.transport:[chan 0] EOF received (0)
DEBUG:paramiko.transport:[chan 0] EOF sent (0)

Same as in the report by @MHC03 who reported the bug, the execution is blocked in the endless loop in spur.ssh._read_int_initialization_line(.).

@mwilliamson could you explain me briefly what _read_int_initialization_line is actually meant to achieve? What is the expected behavior if the line read in _read_int_initialization_line is empty?

I would gladly submit a pull request to fix the bug, but I'm totally ignorant at the moment what is causing it.

@mwilliamson
Copy link
Owner

Exactly what output is read depends on how you're using Spur. For instance, if you want to be able to kill the process, the PID is needed.

Are you having the problem when using this with a Bourne shell? If not, then changing the shell type should fix the issue.

@mristin
Copy link

mristin commented Nov 2, 2018

@mwilliamson I didn't need the PID, it's really just a sequence of commands like:

with spur.SshShell(
            hostname='some_hostname', username='some_user',
            private_key_file=os.path.expanduser("~/.ssh/id_rsa")) as shell:
    shell.run(["echo", "oi"])

I logged into the machine with Linux ssh and:

$ echo $0
-tcsh

But I suppose it's related to Bourne shell?

Do you want me to make a pull request that I change the infinite loop in _read_int_initialization_line to some 10 checks and then throw an error (RuntimeError?) that explains that the return code could not be read from the initialization line and that probably a minimal shell should be used?

@bdwiel
Copy link

bdwiel commented Apr 8, 2019

I am experiencing this issue when using tcsh, too. Unfortunately changing my default shell on the host is too disruptive. Are there workarounds other than using the minimal shell?

@mwilliamson
Copy link
Owner

The alternative would be to implement a shell type specifically for tcsh. Is there a reason the minimal shell doesn't work for you?

@hasnul
Copy link

hasnul commented Jun 9, 2020

This issue also occurs if the remote is running fish shell. Setting shell type to minimal seems to work with fish.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants