584155 - Add a scriptable SOCKS proxy server to allow testing of SOCKS client code

Status: RESOLVED WONTFIX

(Testing :: General, enhancement, P5)

Description

[Justin Samuel]

User-Agent:       Mozilla/5.0 (X11; Linux x86_64; rv:2.0b3pre) Gecko/20100729 Minefield/4.0b3pre
Build Identifier: 

As far as I know, there are currently no tests of the SOCKS client implementation. There are various open bugs related to SOCKS, many of which would benefit from the existence of a SOCKS proxy server that can be used in tests. This has been noted in bug 280661, comment 25.

The existence of a SOCKS proxy server that can be running during mochitests will also be needed for me to properly test bug 354493. Currently, all requests in mochitests go through an HTTP proxy. This results in not knowing the remote address for any host. However, when using a SOCKS proxy without proxy-side address resolution, the remote address is known and thus bug 354493 can be tested.

Reproducible: Always

Comment 1

[Justin Samuel]

Attachment: socksd.js, nsISOCKSProxyServer.idl, and tests

Comment 2

[Justin Samuel]

I've implemented a SOCKS proxy server modeled after httpd.js which can be used in xpcshell tests as well as mochitests. The attached patch doesn't enable them in mochitests yet; it only adds the SOCKS proxy component and the xpcshell tests of the proxy itself.

My feeling was that a separate bug should be used for enabling this SOCKS proxy in mochitests, but let me know if it should all be done in this bug. When added to mochitests, this SOCKS proxy would only be used for a few specific destination hosts that are not used by any existing mochitests.

Comment 3

[Justin Samuel]

CC'ing a few people who might be interested.

Comment 4

[(not currently active) Ted Mielczarek]

How does this relate to the server implemented in attachment 450585 [details] [diff] [review] on bug 280661?

Comment 5

[Justin Samuel]

The test server in attachment 450585 [details] [diff] [review] isn't really a SOCKS proxy, rather it mimics the communication a client would have with one. For example, rather than proxying requests, after completing the SOCKS handshake with the client it then expects specific data from the client and sends back a hard-coded response that appears to the client to be the proxied response. The proxy-mimicking code is also tied in with the individual tests and isn't written with the intention of being general-purpose. Thus, the pseudo-proxy in that test does perform useful testing of bug 280661, but it does so in a way that isn't useful to testing other SOCKS bugs.

If desired, socksd.js can be used in place of much of the code in attachment 450585 [details] [diff] [review]. If socksd.js is ultimately accepted and bug 280661 is still open at that time, my intention is to then add a test to bug 280661 which replaces the current tests with one that uses socksd.js (or just have it be an additional test if that is preferred to replacing the current test).

Comment 6

[Justin Samuel]

Attachment: socksd.js, nsISOCKSProxyServer.idl, and tests

Forgot to refresh before posting the earlier patch.

Comment 7

[Jeff Walden [:Waldo]]

An analogy based on comment 5: the SOCKS proxy here is to the one in attachment 450585 [details] [diff] [review] as httpd.js is to netwerk/test/unit/head_http_server.js:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/test/unit/Attic/head_http_server.js&rev=1.6&root=/cvsroot

Starting to look at this now...

Comment 8

[Jeff Walden [:Waldo]]

Comment on attachment 462574 [details] [diff] [review]
socksd.js, nsISOCKSProxyServer.idl, and tests

Looks broadly reasonable to me, some style things I'd change if I were writing it (but I'm not, and whoever owns the code makes the rules), but basically looks okay.  (It appears that it could use more assertions, to my untutored eye, regardless of style decisions -- paranoia is always a virtue when writing code like this, and when debugging it later.)

I don't know SOCKS, so I didn't look very hard (at all, really, except a few cases at the understanding of the idioms async code structured in this manner would follow) at the implementation and its correctness -- you want someone else to do that.

Comment 9

[Justin Samuel]

Comment on attachment 462574 [details] [diff] [review]
socksd.js, nsISOCKSProxyServer.idl, and tests

Requesting review from Michal. He said it might be a while before he can get to it, which is fine.

Comment 10

[Justin Samuel]

This patch works when run locally but not when run on the build/try servers. When run on the build servers, the tests can't find the socks files. I should be able to work on this with the copy of the tryserver files I have from a failed run.

Comment 11

[(not currently active) Ted Mielczarek]

If you're adding scriptable interfaces, you'll probably need to add to the hacks here:
http://mxr.mozilla.org/mozilla-central/source/testing/xpcshell/Makefile.in#81

any .xpt files that aren't packaged into the Firefox build itself need to be added to the test package, that's currently the best way I have to do it.

Comment 12

[Justin Samuel]

Attachment: socksd.js, nsISOCKSProxyServer.idl, and tests

Fixed so that the xpcshell tests for the socks proxy server will run correctly from packaged builds (that is, so they run correctly on tryserver). I needed to add to the TEST_HARNESS_COMPONENTS, as suggested, as well as to have runxpcshelltests.py invoke xpcshell with "-r /path/to/socksd.manifest" in the same way that it currently passes "-r /path/to/httpd.manifest".

I haven't been able to run the xpcshell tests remotely (that is, I haven't figured out how to successfully use remotexpcshelltests.py). I see that remotexpcshelltests.py's buildXpcsCmd() isn't passing "-r /path/to/httpd.manifest" when xpcshell is invoked. I need to look into this more to see if I need to make any changes for remote socks proxy xpcshell tests.

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:ahal, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 14

[Andrew Halberstadt [:ahal]]

No real activity in 7 years, closing.