Opened 5 years ago

Last modified 4 years ago

#7883 needs_review task

Consider and potentially implement the integration of pyptlib in flashproxy

Reported by: asn Owned by: asn
Priority: Medium Milestone:
Component: Archived/Flashproxy Version:
Severity: Keywords:
Cc: dcf, karsten Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

We should look at whether pyptlib can completely replace the managed-proxy environment parsing code of flashproxy. If yes, we should consider using pyptlib in flashproxy. If for some reason pyptlib is not sufficiently robust for flashproxy's use case, we should look at making pyptlib more generic.

We should also look at the best way of integrating pyptlib in flashproxy? Should we dump the code of pyptlib in flashproxy, or should we have it as an external dependency?

Child Tickets

Change History (6)

comment:1 Changed 5 years ago by asn

Owner: changed from dcf to asn
Status: newassigned

comment:2 Changed 5 years ago by asn

Status: assignedneeds_review

Please see branch pyptlib in https://git.gitorious.org/flashproxy/flashproxy.git.

It assumes that pyptlib is already installed in the system. I didn't embed pyptlib in flashproxy's codebase because you might want to do this differently.

comment:3 Changed 5 years ago by dcf

The patch looks overall nice.

The changes in flashproxy-reg-email don't look right. That program is not a transport plugin and it shouldn't be calling pyptlib.client.init. flashproxy-reg-email is forked by flashproxy-client and inherits its environment variables, including TOR_PT_STATE_LOCATION, which it uses. flashproxy-reg-email shouldn't be doing the whole PT negotiation.

I thought about keeping a copy of pyptlib in the flashproxy tree. The trivial thing doesn't work because there is an extra level of pyptlib directory before reaching __init__.py.

$ ./flashproxy-client
Traceback (most recent call last):
  File "./flashproxy-client", line 23, in <module>
    import pyptlib.client
ImportError: No module named pyptlib.client

If we include pyptlib in this way, then there is the problem of what make install should do. It can copy its own copy of pyptlib to /usr/local/lib/python, but there might already be a copy of pyptlib there.

I guess the best thing in the log run is to require pyptlib to be installed already. To that end, in your branch would you add links and instructions for installing pyptlib to README and to a message that gets printed when a pyptlib import fails? I'm still not totally decided how to handle all this.

comment:4 in reply to:  3 Changed 5 years ago by asn

Status: needs_reviewneeds_revision

Replying to dcf:

The patch looks overall nice.

The changes in flashproxy-reg-email don't look right. That program is not a transport plugin and it shouldn't be calling pyptlib.client.init. flashproxy-reg-email is forked by flashproxy-client and inherits its environment variables, including TOR_PT_STATE_LOCATION, which it uses. flashproxy-reg-email shouldn't be doing the whole PT negotiation.

Oh, I see. Well, I guess in this case, it would make sense to leave flashproxy-reg-email intact (as it was before my changes) and assume that the PT negotiation was done by flashproxy-client. I will update my branch soon.

I thought about keeping a copy of pyptlib in the flashproxy tree. The trivial thing doesn't work because there is an extra level of pyptlib directory before reaching __init__.py.

$ ./flashproxy-client
Traceback (most recent call last):
  File "./flashproxy-client", line 23, in <module>
    import pyptlib.client
ImportError: No module named pyptlib.client

If we include pyptlib in this way, then there is the problem of what make install should do. It can copy its own copy of pyptlib to /usr/local/lib/python, but there might already be a copy of pyptlib there.

I guess the best thing in the log run is to require pyptlib to be installed already. To that end, in your branch would you add links and instructions for installing pyptlib to README and to a message that gets printed when a pyptlib import fails? I'm still not totally decided how to handle all this.

I see your concerns and I agree. For pyobfsproxy, I have decided to assume that pyptlib is installed already.

For flashproxy, I think assuming that pyptlib is present makes a bit more sense, since flashproxy-client and flashproxy-server will only be used by clients which should be using bundles anyway. So the bundle maker has the responsibility of installing pyptlib properly. For advanced clients who want to setup the thing on their own, we should update the README appropriately.

In any case, the decision is up to you on whether you want to use pyptlib or not. Maybe you want to skip it, till pyptlib gets some more features (like Extended ORPort support), and that's fine.

I plan to update the README, and add an import error message, soon.

comment:5 Changed 5 years ago by asn

Status: needs_revisionneeds_review

Pushed fixes to the issues above to pyptlib_take2 in https://git.gitorious.org/flashproxy/flashproxy.git.

comment:6 Changed 4 years ago by dcf

In comment:8:ticket:8549, it's stated that pyptlib may require Python 2.7. I think I'd like to keep the flashproxy programs compatible with 2.6, so I'd like pyptlib to be also if possible.

I only get two errors when trying to run the tests with 2.6, which seem trivial to remove:

======================================================================
ERROR: Disabled TOR_PT_EXTENDED_SERVER_PORT.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "pyptlib/test/test_server.py", line 136, in test_disabled_extorport
    self.assertIsNone(retval['ext_orport'])
AttributeError: 'testServer' object has no attribute 'assertIsNone'

======================================================================
ERROR: Application only supports some transport.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "pyptlib/test/test_server.py", line 162, in test_matched_transports
    self.assertIn("midnight",retval['transports'])
AttributeError: 'testServer' object has no attribute 'assertIn'

----------------------------------------------------------------------
Note: See TracTickets for help on using tickets.