[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #7944 [Flashproxy]: Standalone flash proxy

#7944: Standalone flash proxy
-------------------------+--------------------------------------------------
 Reporter:  akrey        |          Owner:  dcf           
     Type:  enhancement  |         Status:  needs_revision
 Priority:  normal       |      Milestone:                
Component:  Flashproxy   |        Version:                
 Keywords:               |         Parent:                
   Points:               |   Actualpoints:                
-------------------------+--------------------------------------------------
Changes (by dcf):

  * status:  needs_review => needs_revision


Comment:

 I created a `node.js` branch to start tracking and integrating your
 changes.

 https://gitweb.torproject.org/flashproxy.git/shortlog/refs/heads/node.js

 About the changes generally, I would prefer that there be fewer `if
 (BROWSER)` blocks. There are too many now for this code to be merged.
 Ideally, all the node.js-specific stuff will be in one block at the top of
 the file, and other parts of the code will use abstractions set up by that
 block.

 In
 https://github.com/arlolra/flashproxy/commit/7dab216dce629c0b85f6e72caadbdc5530f8d540,
 which removes use of the `event` parameter in callbacks, a better thing to
 test is `if (event)` rather than `if (!BROWSER)`. The functions should
 still print ''something'' even if they don't have event details, to
 prevent confusion as in comment:13.

 Even having to check `event` seems like a bug in the WebSocket library. My
 preferred solution here is to have the bug reported upstream and not to
 work around it here. The lack of `bufferedAmount` in
 https://github.com/arlolra/flashproxy/commit/4110c0f96273259b162d33ee3a7ed7542d0294c6
 also seems like a bug, and ignoring it means we potentially use unbounded
 memory. The need to set `{ binary: true }` looks like another bug. We
 could build an abstraction to hide the differences, but I don't want a
 wrapper function doing something that should be fundamental like `send`.
 Better to get these bugs fixed.

 In summary: please report what look like bugs to the authors of the
 WebSocket library, and try to move the `if (BROWSER)` blocks into one
 place.

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/7944#comment:15>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs