GitHub PullRequestReviewCommentEvent at 2014-11-03 16:04:38
Repository: uproxy-networking
Actor: iislucas
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/uProxy/uproxy-networking/pulls/comments/19741988","id":19741988,"diff_hunk":"@@ -70,63 +83,83 @@ module RtcToNet {\n     // DataChannel and PeerConnection to be used directly and not via a freedom\n     // interface. Then all work can be done by promise binding and this can be\n     // removed.\n-    private sessions_ :{ [channelLabel:string] : Session }\n-\n-    // SocsToRtc server is given a localhost transport address (endpoint) to\n-    // start a socks server listening to, and a config for setting up a peer-\n-    // connection. If the given port is zero, platform chooses a port and this\n-    // listening port is returned by the promise.\n-    //\n-    // TODO: add checking of fingerprints.\n+    private sessions_ :{ [channelLabel:string] : Session } = {};\n+\n+    // As configure() but handles creation of peerconnection.\n     constructor(\n-        pcConfig:WebRtc.PeerConnectionConfig,\n-        proxyConfig:ProxyConfig,\n+        pcConfig?:WebRtc.PeerConnectionConfig,\n+        proxyConfig?:ProxyConfig,\n         obfuscate?:boolean) {\n-      // Messages received via signalling channel must reach the remote peer\n-      // through something other than the peerconnection. (e.g. XMPP). This is\n-      // the Freedom channel object to sends signalling messages to the peer.\n-      // SOCKS sessions biject to peerconnection datachannels.\n-      this.sessions_ = {};\n+      if (pcConfig) {\n+        this.start(\n+            proxyConfig,\n+            obfuscate ?\n+              freedom.churn(pcConfig) :\n+              freedom['core.uproxypeerconnection'](pcConfig));\n+      }\n+    }\n+\n+    // Starts with the supplied peerconnection.\n+    // Returns this.onceReady.\n+    public start = (\n+        proxyConfig:ProxyConfig,\n+        peerconnection:freedom_UproxyPeerConnection.Pc)\n+        : Promise<void> => {\n+      if (this.peerConnection_) {\n+        throw new Error('already configured');\n+      }\n       this.proxyConfig = proxyConfig;\n-      this.peerConnection_ = obfuscate ?\n-          freedom.churn(pcConfig) :\n-          freedom['core.uproxypeerconnection'](pcConfig);\n+      this.peerConnection_ = peerconnection;\n+\n       this.peerConnection_.on('dataFromPeer', this.onDataFromPeer_);\n-      this.peerConnection_.on('peerOpenedChannel', (channelLabel:string) => {\n-        if(channelLabel === '_control_') {\n-          log.debug('control channel openned.');\n-          return;\n-        }\n-        // TODO: This can be removed once https://github.com/uProxy/uproxy/issues/347 is fixed.\n-        if(channelLabel === '') {\n-          log.debug('dummy init channel.');\n-          return;\n-        }\n-        var session = new Session(this.peerConnection_, channelLabel,\n-          proxyConfig, this.bytesReceivedFromPeer, this.bytesSentToPeer);\n-        this.sessions_[channelLabel] = session;\n-        session.onceClosed.then(() => {\n-          delete this.sessions_[channelLabel];\n-        });\n-      });\n       this.peerConnection_.on('signalForPeer', this.signalsForPeer.handle);\n+      this.peerConnection_.on('peerOpenedChannel', this.onPeerOpenedChannel_);\n+\n       this.onceReady = this.peerConnection_.onceConnected().then(() => {});\n-      this.onceClosed = this.peerConnection_.onceDisconnected();\n-      // TODO: add checking that the peer's fingerprint matches the provided\n-      // fingerprint.\n+      this.onceReady.catch(this.fulfillStopping_);\n+      this.peerConnection_.onceDisconnected()\n+          .then(this.fulfillStopping_, this.fulfillStopping_);\n+      this.onceClosed = this.onceStopping_.then(this.stopResources_);\n+\n+      return this.onceReady;\n     }\n \n-    // Close the peer-connection (and hence all data channels) and all\n-    // associated TCP connections. Note: once closed, cannot be openned again.\n-    public close = () => {\n-      this.peerConnection_.close();\n-      // CONSIDER: will peerConnection's closing of channels make this un-\n-      // needed? is it better to include this anyway?\n-      var channelLabel :string;\n-      for (channelLabel in this.sessions_) {\n-        this.sessions_[channelLabel].close();\n+    private onPeerOpenedChannel_ = (channelLabel:string) : void => {\n+      log.debug('creating new session for channel ' + channelLabel);\n+\n+      var session = new Session(\n+          this.peerConnection_,\n+          channelLabel,\n+          this.proxyConfig,\n+          this.bytesReceivedFromPeer,\n+          this.bytesSentToPeer);\n+      this.sessions_[channelLabel] = session;\n+\n+      var discard = () => {\n         delete this.sessions_[channelLabel];\n-      }\n+        log.debug('discarded session ' + channelLabel + ' (' +\n+            Object.keys(this.sessions_).length + ' sessions remaining)');\n+      };\n+      session.onceClosed.then(discard, (e:Error) => {\n+        log.warn('session ' + channelLabel + ' closed with error: ' + e.message);\n+        discard();\n+      });\n+    }\n+\n+    // Initiates shutdown of the peerconnection.\n+    // Returns onceClosed.\n+    // TODO: rename stop, ala SocksToRtc (API breakage).\n+    public close = () : Promise<void> => {\n+      this.fulfillStopping_();\n+      return this.onceClosed;\n+    }\n+\n+    // Shuts down the peerconnection, fulfilling it has terminated.\n+    // Since its close() method should ever reject, this should never reject.\n+    public stopResources_ = () => {\n+      // uproxypeerconnection doesn't allow us query whether the\n+      // peerconnection has shut down but the call is explicitly idempodent.\n+      return this.peerConnection_.close();","path":"src/rtc-to-net/rtc-to-net.ts","position":179,"original_position":179,"commit_id":"e705c12e24b4c77f13abd27da80f4d81bd98dce2","original_commit_id":"1f4ebc40af9ab49461091fb68c34ced1e8aec31c","user":{"login":"iislucas","id":415450,"avatar_url":"https://avatars.githubusercontent.com/u/415450?v=2","gravatar_id":"","url":"https://api.github.com/users/iislucas","html_url":"https://github.com/iislucas","followers_url":"https://api.github.com/users/iislucas/followers","following_url":"https://api.github.com/users/iislucas/following{/other_user}","gists_url":"https://api.github.com/users/iislucas/gists{/gist_id}","starred_url":"https://api.github.com/users/iislucas/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/iislucas/subscriptions","organizations_url":"https://api.github.com/users/iislucas/orgs","repos_url":"https://api.github.com/users/iislucas/repos","events_url":"https://api.github.com/users/iislucas/events{/privacy}","received_events_url":"https://api.github.com/users/iislucas/received_events","type":"User","site_admin":false},"body":"I'd expect a session close here - why peer connection close? ","created_at":"2014-11-03T16:04:38Z","updated_at":"2014-11-03T16:04:38Z","html_url":"https://github.com/uProxy/uproxy-networking/pull/146#discussion_r19741988","pull_request_url":"https://api.github.com/repos/uProxy/uproxy-networking/pulls/146","_links":{"self":{"href":"https://api.github.com/repos/uProxy/uproxy-networking/pulls/comments/19741988"},"html":{"href":"https://github.com/uProxy/uproxy-networking/pull/146#discussion_r19741988"},"pull_request":{"href":"https://api.github.com/repos/uProxy/uproxy-networking/pulls/146"}}},"pull_request":{"url":"https://api.github.com/repos/uProxy/uproxy-networking/pulls/146","id":23563124,"html_url":"https://github.com/uProxy/uproxy-networking/pull/146","diff_url":"https://github.com/uProxy/uproxy-networking/pull/146.diff","patch_url":"https://github.com/uProxy/uproxy-networking/pull/146.patch","issue_url":"https://api.github.com/repos/uProxy/uproxy-networking/issues/146","number":146,"state":"open","locked":false,"title":"SocksToRtc-style lifecycle and unit tests for RtcToNet","user":{"login":"trevj","id":5142116,"avatar_url":"https://avatars.githubusercontent.com/u/5142116?v=2","gravatar_id":"","url":"https://api.github.com/users/trevj","html_url":"https://github.com/trevj","followers_url":"https://api.github.com/users/trevj/followers","following_url":"https://api.github.com/users/trevj/following{/other_user}","gists_url":"https://api.github.com/users/trevj/gists{/gist_id}","starred_url":"https://api.github.com/users/trevj/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/trevj/subscriptions","organizations_url":"https://api.github.com/users/trevj/orgs","repos_url":"https://api.github.com/users/trevj/repos","events_url":"https://api.github.com/users/trevj/events{/privacy}","received_events_url":"https://api.github.com/users/trevj/received_events","type":"User","site_admin":false},"body":"This pull request is to RtcToNet as the following pull request was to:\r\nhttps://github.com/uProxy/uproxy-networking/pull/137\r\n\r\nOf course, the RtcToNet \"server\" has only one dependency: the peerconnection. Also remove code related to control channel which is now handled by peerconnection.\r\n\r\nTested with `curl` and a quick browsing session on Firefox.\r\n\r\nI will tackle RtcToNet sessions in a followup.","created_at":"2014-10-29T20:43:07Z","updated_at":"2014-11-03T16:04:38Z","closed_at":null,"merged_at":null,"merge_commit_sha":"a1570a938beab39d91630d02c1b2c5d5565ed387","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/uProxy/uproxy-networking/pulls/146/commits","review_comments_url":"https://api.github.com/repos/uProxy/uproxy-networking/pulls/146/comments","review_comment_url":"https://api.github.com/repos/uProxy/uproxy-networking/pulls/comments/{number}","comments_url":"https://api.github.com/repos/uProxy/uproxy-networking/issues/146/comments","statuses_url":"https://api.github.com/repos/uProxy/uproxy-networking/statuses/e705c12e24b4c77f13abd27da80f4d81bd98dce2","head":{"label":"uProxy:trevj-rtctonet-tests","ref":"trevj-rtctonet-tests","sha":"e705c12e24b4c77f13abd27da80f4d81bd98dce2","user":{"login":"uProxy","id":6548286,"avatar_url":"https://avatars.githubusercontent.com/u/6548286?v=2","gravatar_id":"","url":"https://api.github.com/users/uProxy","html_url":"https://github.com/uProxy","followers_url":"https://api.github.com/users/uProxy/followers","following_url":"https://api.github.com/users/uProxy/following{/other_user}","gists_url":"https://api.github.com/users/uProxy/gists{/gist_id}","starred_url":"https://api.github.com/users/uProxy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/uProxy/subscriptions","organizations_url":"https://api.github.com/users/uProxy/orgs","repos_url":"https://api.github.com/users/uProxy/repos","events_url":"https://api.github.com/users/uProxy/events{/privacy}","received_events_url":"https://api.github.com/users/uProxy/received_events","type":"Organization","site_admin":false},"repo":{"id":16489216,"name":"uproxy-networking","full_name":"uProxy/uproxy-networking","owner":{"login":"uProxy","id":6548286,"avatar_url":"https://avatars.githubusercontent.com/u/6548286?v=2","gravatar_id":"","url":"https://api.github.com/users/uProxy","html_url":"https://github.com/uProxy","followers_url":"https://api.github.com/users/uProxy/followers","following_url":"https://api.github.com/users/uProxy/following{/other_user}","gists_url":"https://api.github.com/users/uProxy/gists{/gist_id}","starred_url":"https://api.github.com/users/uProxy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/uProxy/subscriptions","organizations_url":"https://api.github.com/users/uProxy/orgs","repos_url":"https://api.github.com/users/uProxy/repos","events_url":"https://api.github.com/users/uProxy/events{/privacy}","received_events_url":"https://api.github.com/users/uProxy/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/uProxy/uproxy-networking","description":"Socks proxy over WebRTC","fork":false,"url":"https://api.github.com/repos/uProxy/uproxy-networking","forks_url":"https://api.github.com/repos/uProxy/uproxy-networking/forks","keys_url":"https://api.github.com/repos/uProxy/uproxy-networking/keys{/key_id}","collaborators_url":"https://api.github.com/repos/uProxy/uproxy-networking/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/uProxy/uproxy-networking/teams","hooks_url":"https://api.github.com/repos/uProxy/uproxy-networking/hooks","issue_events_url":"https://api.github.com/repos/uProxy/uproxy-networking/issues/events{/number}","events_url":"https://api.github.com/repos/uProxy/uproxy-networking/events","assignees_url":"https://api.github.com/repos/uProxy/uproxy-networking/assignees{/user}","branches_url":"https://api.github.com/repos/uProxy/uproxy-networking/branches{/branch}","tags_url":"https://api.github.com/repos/uProxy/uproxy-networking/tags","blobs_url":"https://api.github.com/repos/uProxy/uproxy-networking/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/uProxy/uproxy-networking/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/uProxy/uproxy-networking/git/refs{/sha}","trees_url":"https://api.github.com/repos/uProxy/uproxy-networking/git/trees{/sha}","statuses_url":"https://api.github.com/repos/uProxy/uproxy-networking/statuses/{sha}","languages_url":"https://api.github.com/repos/uProxy/uproxy-networking/languages","stargazers_url":"https://api.github.com/repos/uProxy/uproxy-networking/stargazers","contributors_url":"https://api.github.com/repos/uProxy/uproxy-networking/contributors","subscribers_url":"https://api.github.com/repos/uProxy/uproxy-networking/subscribers","subscription_url":"https://api.github.com/repos/uProxy/uproxy-networking/subscription","commits_url":"https://api.github.com/repos/uProxy/uproxy-networking/commits{/sha}","git_commits_url":"https://api.github.com/repos/uProxy/uproxy-networking/git/commits{/sha}","comments_url":"https://api.github.com/repos/uProxy/uproxy-networking/comments{/number}","issue_comment_url":"https://api.github.com/repos/uProxy/uproxy-networking/issues/comments/{number}","contents_url":"https://api.github.com/repos/uProxy/uproxy-networking/contents/{+path}","compare_url":"https://api.github.com/repos/uProxy/uproxy-networking/compare/{base}...{head}","merges_url":"https://api.github.com/repos/uProxy/uproxy-networking/merges","archive_url":"https://api.github.com/repos/uProxy/uproxy-networking/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/uProxy/uproxy-networking/downloads","issues_url":"https://api.github.com/repos/uProxy/uproxy-networking/issues{/number}","pulls_url":"https://api.github.com/repos/uProxy/uproxy-networking/pulls{/number}","milestones_url":"https://api.github.com/repos/uProxy/uproxy-networking/milestones{/number}","notifications_url":"https://api.github.com/repos/uProxy/uproxy-networking/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/uProxy/uproxy-networking/labels{/name}","releases_url":"https://api.github.com/repos/uProxy/uproxy-networking/releases{/id}","created_at":"2014-02-03T18:51:12Z","updated_at":"2014-09-09T15:19:19Z","pushed_at":"2014-11-03T15:55:04Z","git_url":"git://github.com/uProxy/uproxy-networking.git","ssh_url":"git@github.com:uProxy/uproxy-networking.git","clone_url":"https://github.com/uProxy/uproxy-networking.git","svn_url":"https://github.com/uProxy/uproxy-networking","homepage":"","size":4000,"stargazers_count":26,"watchers_count":26,"language":"TypeScript","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":3,"mirror_url":null,"open_issues_count":5,"forks":3,"open_issues":5,"watchers":26,"default_branch":"dev"}},"base":{"label":"uProxy:dev","ref":"dev","sha":"bc44e988bd5378e1683c83ec61f17192791bd677","user":{"login":"uProxy","id":6548286,"avatar_url":"https://avatars.githubusercontent.com/u/6548286?v=2","gravatar_id":"","url":"https://api.github.com/users/uProxy","html_url":"https://github.com/uProxy","followers_url":"https://api.github.com/users/uProxy/followers","following_url":"https://api.github.com/users/uProxy/following{/other_user}","gists_url":"https://api.github.com/users/uProxy/gists{/gist_id}","starred_url":"https://api.github.com/users/uProxy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/uProxy/subscriptions","organizations_url":"https://api.github.com/users/uProxy/orgs","repos_url":"https://api.github.com/users/uProxy/repos","events_url":"https://api.github.com/users/uProxy/events{/privacy}","received_events_url":"https://api.github.com/users/uProxy/received_events","type":"Organization","site_admin":false},"repo":{"id":16489216,"name":"uproxy-networking","full_name":"uProxy/uproxy-networking","owner":{"login":"uProxy","id":6548286,"avatar_url":"https://avatars.githubusercontent.com/u/6548286?v=2","gravatar_id":"","url":"https://api.github.com/users/uProxy","html_url":"https://github.com/uProxy","followers_url":"https://api.github.com/users/uProxy/followers","following_url":"https://api.github.com/users/uProxy/following{/other_user}","gists_url":"https://api.github.com/users/uProxy/gists{/gist_id}","starred_url":"https://api.github.com/users/uProxy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/uProxy/subscriptions","organizations_url":"https://api.github.com/users/uProxy/orgs","repos_url":"https://api.github.com/users/uProxy/repos","events_url":"https://api.github.com/users/uProxy/events{/privacy}","received_events_url":"https://api.github.com/users/uProxy/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/uProxy/uproxy-networking","description":"Socks proxy over WebRTC","fork":false,"url":"https://api.github.com/repos/uProxy/uproxy-networking","forks_url":"https://api.github.com/repos/uProxy/uproxy-networking/forks","keys_url":"https://api.github.com/repos/uProxy/uproxy-networking/keys{/key_id}","collaborators_url":"https://api.github.com/repos/uProxy/uproxy-networking/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/uProxy/uproxy-networking/teams","hooks_url":"https://api.github.com/repos/uProxy/uproxy-networking/hooks","issue_events_url":"https://api.github.com/repos/uProxy/uproxy-networking/issues/events{/number}","events_url":"https://api.github.com/repos/uProxy/uproxy-networking/events","assignees_url":"https://api.github.com/repos/uProxy/uproxy-networking/assignees{/user}","branches_url":"https://api.github.com/repos/uProxy/uproxy-networking/branches{/branch}","tags_url":"https://api.github.com/repos/uProxy/uproxy-networking/tags","blobs_url":"https://api.github.com/repos/uProxy/uproxy-networking/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/uProxy/uproxy-networking/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/uProxy/uproxy-networking/git/refs{/sha}","trees_url":"https://api.github.com/repos/uProxy/uproxy-networking/git/trees{/sha}","statuses_url":"https://api.github.com/repos/uProxy/uproxy-networking/statuses/{sha}","languages_url":"https://api.github.com/repos/uProxy/uproxy-networking/languages","stargazers_url":"https://api.github.com/repos/uProxy/uproxy-networking/stargazers","contributors_url":"https://api.github.com/repos/uProxy/uproxy-networking/contributors","subscribers_url":"https://api.github.com/repos/uProxy/uproxy-networking/subscribers","subscription_url":"https://api.github.com/repos/uProxy/uproxy-networking/subscription","commits_url":"https://api.github.com/repos/uProxy/uproxy-networking/commits{/sha}","git_commits_url":"https://api.github.com/repos/uProxy/uproxy-networking/git/commits{/sha}","comments_url":"https://api.github.com/repos/uProxy/uproxy-networking/comments{/number}","issue_comment_url":"https://api.github.com/repos/uProxy/uproxy-networking/issues/comments/{number}","contents_url":"https://api.github.com/repos/uProxy/uproxy-networking/contents/{+path}","compare_url":"https://api.github.com/repos/uProxy/uproxy-networking/compare/{base}...{head}","merges_url":"https://api.github.com/repos/uProxy/uproxy-networking/merges","archive_url":"https://api.github.com/repos/uProxy/uproxy-networking/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/uProxy/uproxy-networking/downloads","issues_url":"https://api.github.com/repos/uProxy/uproxy-networking/issues{/number}","pulls_url":"https://api.github.com/repos/uProxy/uproxy-networking/pulls{/number}","milestones_url":"https://api.github.com/repos/uProxy/uproxy-networking/milestones{/number}","notifications_url":"https://api.github.com/repos/uProxy/uproxy-networking/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/uProxy/uproxy-networking/labels{/name}","releases_url":"https://api.github.com/repos/uProxy/uproxy-networking/releases{/id}","created_at":"2014-02-03T18:51:12Z","updated_at":"2014-09-09T15:19:19Z","pushed_at":"2014-11-03T15:55:04Z","git_url":"git://github.com/uProxy/uproxy-networking.git","ssh_url":"git@github.com:uProxy/uproxy-networking.git","clone_url":"https://github.com/uProxy/uproxy-networking.git","svn_url":"https://github.com/uProxy/uproxy-networking","homepage":"","size":4000,"stargazers_count":26,"watchers_count":26,"language":"TypeScript","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":3,"mirror_url":null,"open_issues_count":5,"forks":3,"open_issues":5,"watchers":26,"default_branch":"dev"}},"_links":{"self":{"href":"https://api.github.com/repos/uProxy/uproxy-networking/pulls/146"},"html":{"href":"https://github.com/uProxy/uproxy-networking/pull/146"},"issue":{"href":"https://api.github.com/repos/uProxy/uproxy-networking/issues/146"},"comments":{"href":"https://api.github.com/repos/uProxy/uproxy-networking/issues/146/comments"},"review_comments":{"href":"https://api.github.com/repos/uProxy/uproxy-networking/pulls/146/comments"},"review_comment":{"href":"https://api.github.com/repos/uProxy/uproxy-networking/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/uProxy/uproxy-networking/pulls/146/commits"},"statuses":{"href":"https://api.github.com/repos/uProxy/uproxy-networking/statuses/e705c12e24b4c77f13abd27da80f4d81bd98dce2"}}}}