If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

NEW
Assigned to

Status

Testing
General
7 years ago
2 years ago

People

(Reporter: Justin Samuel, Assigned: Justin Samuel)

Tracking

(Blocks: 2 bugs)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

7 years ago
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
(Assignee)

Comment 1

7 years ago
Created attachment 462479 [details] [diff] [review]
socksd.js, nsISOCKSProxyServer.idl, and tests
(Assignee)

Comment 2

7 years ago
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.
(Assignee)

Comment 3

7 years ago
CC'ing a few people who might be interested.
(Assignee)

Updated

7 years ago
Attachment #462479 - Flags: feedback?(jwalden+bmo)
(Assignee)

Updated

7 years ago
Blocks: 354493
How does this relate to the server implemented in attachment 450585 [details] [diff] [review] on bug 280661?
(Assignee)

Comment 5

7 years ago
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).
(Assignee)

Comment 6

7 years ago
Created attachment 462574 [details] [diff] [review]
socksd.js, nsISOCKSProxyServer.idl, and tests

Forgot to refresh before posting the earlier patch.
Attachment #462479 - Attachment is obsolete: true
Attachment #462574 - Flags: feedback?(jwalden+bmo)
Attachment #462479 - Flags: feedback?(jwalden+bmo)
(Assignee)

Updated

7 years ago
Blocks: 585191
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 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.
Attachment #462574 - Flags: feedback?(jwalden+bmo) → feedback+
(Assignee)

Comment 9

7 years ago
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.
Attachment #462574 - Flags: review?(michal.novotny)
Assignee: nobody → bugs
Status: UNCONFIRMED → NEW
Ever confirmed: true
Version: unspecified → Trunk
(Assignee)

Comment 10

7 years ago
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.
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.
(Assignee)

Comment 12

7 years ago
Created attachment 505629 [details] [diff] [review]
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.
Attachment #462574 - Attachment is obsolete: true
Attachment #462574 - Flags: review?(michal.novotny)
See Also: → bug 1208087
You need to log in before you can comment on or make changes to this bug.