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

Upgrade to Python3 + Tornado6 #12

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ctrlcctrlv
Copy link

No description provided.

@benjamincburns
Copy link
Owner

Thanks so much for the contribution. This looks awesome. I'll give it a test tonight, and provided it's all working correctly I'll be very happy to merge.

@benjamincburns
Copy link
Owner

Also before I merge, I've been meaning to change the license of this project to MIT. Would you still be willing to make this contribution under those terms?

@ctrlcctrlv
Copy link
Author

@benjamincburns Oh, yes, of course. The delay was no problem in the end, I realized that I broke on accident backwards compatibility because I wanted to run this as non-root user (most of my changes allow for that)… but I forgot to make sure to change the port back to 80 for the public version. (As of ctrlcctrlv/websockproxy@915aafb I can just do --ws-port=8080 😄)

self.tun.up()
tap_name = tornado.options.options.tap_name or "tap0"
self.tun = TunTapDevice(name=tap_name, flags= (IFF_TAP | IFF_NO_PI))
if tornado.options.options.tap_name is None:
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically this change allows the non-root user to run the server.

You have to set up the TAP device as root, and assign it to your user account, but you don't need to run the server as root. :-)

So if you pass --tap-name=tap0 (or w/e, tap1 etc.) it'll Just Work™

@benjamincburns
Copy link
Owner

I'm not sure exactly why, but I'm afraid that this PR doesn't seem to work correctly when I run it on the relay. I see connections logged, but I never see the [IP] using mac [mac_addr] log lines, and jor1k running under jor1k.com fails to get a DHCP assignment.

@benjamincburns
Copy link
Owner

Apologies for the commit noise - just figured I'd save you the effort of rebasing this on current main so you could see exactly what I tested.

@ctrlcctrlv
Copy link
Author

It may help for me to say why I did this work.

I was playing around with JSLinux and wanted to be able to download faster.

So what's the best way for me to test that stuff? (jor1k?)

RUN apt-get update && apt-get install -y python2 python2-dev iptables dnsmasq uml-utilities net-tools build-essential curl && apt-get clean

RUN curl https://bootstrap.pypa.io/pip/2.7/get-pip.py --output get-pip.py && python2 get-pip.py && rm get-pip.py
RUN apt-get update && apt-get install -y python2 python2-dev iptables dnsmasq python3-pip uml-utilities net-tools build-essential curl && apt-get clean

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this install python3 python3-dev instead, now?

@BelleNottelling
Copy link

So I am able to identify two issues that results in this PR not working.

  1. The usage of ord is invalid for python 3. Those functions can just be removed for python 3 compatibility.
  2. After upgrading the Tornado version past 4, you run into a 403 HTTP code because they changed the behavior to not allow websocket connections from other origin sites.

Both of these are fixable and I am able to then get the client to connect, however I can't get any communication to work correctly. This happens as well if you try to perform a upgrade of the existing code without apply any of the functional changes seen in this PR 🤷‍♀️

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

Successfully merging this pull request may close these issues.

4 participants