1
\$\begingroup\$

I've made a simple netcat in Python and would appreciate any advice/opinions of how I've implemented it.

test.py:

#!/usr/bin/env python
# -*- coding: utf-8 -*-

import sys

import netcat


def main(*args):
    netcat.connect('localhost', 4444)
    netcat.listen('', 4444)


if __name__ == '__main__':
    sys.exit(main(*sys.argv))

netcat.py:

# -*- coding: utf-8 -*-

import sys
import select
import socket
from contextlib import closing

from servers import Server
from cat import cat_line


def netcat(sock, infile=sys.stdin, outfile=sys.stdout):
    with closing(sock) as s:
        sf = s.makefile()
        while True:
            r, w, e = select.select([s, infile], [], [])
            if s in r:
                if not cat_line(sf, outfile):
                    break
            if infile in r:
                if not cat_line(infile, sf):
                    break


def listen(lhost, lport, *args, **kwargs):
    with Server(lhost, lport) as server:
        client = server.get_client()
        netcat(client, *args, **kwargs)


def connect(rhost, rport, *args, **kwargs):
    server = socket.create_connection((rhost, rport))
    with closing(server) as s:
        netcat(s, *args, **kwargs)

servers.py:

# -*- coding: utf-8 -*-

import socket


class Server(object):

    def __init__(self, lhost, lport):
        self._sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
        self._sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
        self._sock.bind((lhost, lport))
        self._sock.listen(1)

    def __iter__(self):
        return self

    def next(self):
        return self.get_client()

    def __enter__(self):
        return self

    def __exit__(self, *exc):
        self.close()

    def get_client(self):
        return self._sock.accept()[0]

    def close(self):
        self._sock.close()

cat.py:

# -*- coding: utf-8 -*-

import sys


def cat_line(infile=sys.stdin, outfile=sys.stdout):
    line = infile.readline()
    if not line:
        return False
    outfile.write(line)
    outfile.flush()
    return True


def cat(infile=sys.stdin, outfile=sys.stdout):
    while True:
        if not cat_line(infile, outfile):
            break

Edit: Maybe I should take the responsibility of closing the socket from the netcat function...

def netcat(sock, infile=sys.stdin, outfile=sys.stdout):
    sf = sock.makefile()
    while True:
        r, w, e = select.select([sock, infile], [], [])
        if sock in r and not cat_line(sf, outfile):
                break
        if infile in r and not cat_line(infile, sf):
                break

and instead, put that functionality into the listen function?

def listen(lhost, lport, *args, **kwargs):
    with Server(lhost, lport) as server:
        client = server.get_client()
    with closing(client) as c:
        netcat(c, *args, **kwargs)

Also...

def cat(infile=sys.stdin, outfile=sys.stdout):
    while True:
        if not cat_line(infile, outfile):
            break

could be

def cat(infile=sys.stdin, outfile=sys.stdout):
    while cat_line(infile, outfile):
        pass

github

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

test.py

If you look at the definition of main(), you will see that it doesn't explicitly return anything. By default, functions return None, so you will always be exiting with None. Since it is always the same, it is useless even to use sys.exit(). The program will exit once the file has been read, so you don't need to tell Python to do that. If you want to use exit codes, you'll need main() to return them.

main() doesn't use its arguments, so why give it any? If sys.argv is ignored, take it out and remove the useless import of sys.

netcat.py

if s in r:
    if not cat_line(sf, outfile):
        break

The only time your program does something in the case of s in r is when not cat_line(sf, outfile). Since you want to handle only the cases where both are true, use and:

if s in r and not cat_line(sf, outfile):
    break

Same with the next if block. I wouldn't recommend it because it makes your lines too long, but you could put both if blocks in one:

if ((s in r) and not cat_line(sf, outfile)) or ((infile in r) and not cat_line(infile, sf):
    break

servers.py and cat.py look okay to me, but my opinion is that you don't need multiple files. You don't have a bunch of lines that need to be split up, so you might as well put all of it in one file (not including test.py).

\$\endgroup\$
1
  • \$\begingroup\$ Thank you for you feedback, I don't know why I did that with main, I don't usually do it but I think I'd seen it in someone's code recently. I'm going to change those if statements and put all files into one. Thanks :) \$\endgroup\$
    – brenw0rth
    Mar 19, 2016 at 16:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.