)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"9aec97df387cd938f508f57cf7232eda71f87a4b","unresolved":false,"context_lines":[{"line_number":13,"context_line":"subclass."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Currently, only VNC is supported, via the configuration"},{"line_number":16,"context_line":"option \u0027novncproxy_security_driver\u0027.  The class to subclass"},{"line_number":17,"context_line":"for VNC security proxy drivers is RFBSecurityProxyHelper."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"The workflow for adding a new VNC security proxy driver is"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":28,"id":"ba8a016a_33edf58e","line":16,"updated":"2015-11-20 23:16:38.000000000","message":"nit: this second sentence could be removed, as it duplicates information which is better described in the next paragraph.","commit_id":"6f56e9cf52cf5274a896e44394061ed6ea29aeff"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"9aec97df387cd938f508f57cf7232eda71f87a4b","unresolved":false,"context_lines":[{"line_number":17,"context_line":"for VNC security proxy drivers is RFBSecurityProxyHelper."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"The workflow for adding a new VNC security proxy driver is"},{"line_number":20,"context_line":"to subclass the the traffic-type-specific security proxy"},{"line_number":21,"context_line":"base classes (e.g. RFBSecurityProxyHelper), and implement"},{"line_number":22,"context_line":"the `choose_security_type` and `security_handshake` methods."},{"line_number":23,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":28,"id":"ba8a016a_934aa1ab","line":20,"updated":"2015-11-20 23:16:38.000000000","message":"nit: s/the the/the/","commit_id":"6f56e9cf52cf5274a896e44394061ed6ea29aeff"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"9aec97df387cd938f508f57cf7232eda71f87a4b","unresolved":false,"context_lines":[{"line_number":21,"context_line":"base classes (e.g. RFBSecurityProxyHelper), and implement"},{"line_number":22,"context_line":"the `choose_security_type` and `security_handshake` methods."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"DocImpact"},{"line_number":25,"context_line":"Implements bp: websocket-proxy-to-host-security"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Change-Id: Id586b2558fd4c7ed0eda3d3555d51fcd019eb414"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":28,"id":"ba8a016a_b38805c8","line":24,"updated":"2015-11-20 23:16:38.000000000","message":"You should add the SecurityImpact tag to get more security reviews","commit_id":"6f56e9cf52cf5274a896e44394061ed6ea29aeff"}],"nova/cmd/novncproxy.py":[{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"fc77f83a730ff1ddf0b5a97c5aed073ec09123a2","unresolved":false,"context_lines":[{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    # Create and start the NovaWebSockets proxy"},{"line_number":75,"context_line":"    server \u003d websocketproxy.NovaWebSocketProxy("},{"line_number":76,"context_line":"                traffic_type\u003d\u0027vnc\u0027,"},{"line_number":77,"context_line":"                listen_host\u003dCONF.novncproxy_host,"},{"line_number":78,"context_line":"                listen_port\u003dCONF.novncproxy_port,"},{"line_number":79,"context_line":"                source_is_ipv6\u003dCONF.source_is_ipv6,"}],"source_content_type":"text/x-python","patch_set":2,"id":"1abeadc6_37da3e78","line":76,"updated":"2014-08-29 10:35:36.000000000","message":"Lets define constants for the permissible values eg so you\u0027d do\n\n   traffic_type\u003dwebsocketproxy.TRAFFIC_TYPE_VNC\n\nlikewise throughout all the other files in this patch","commit_id":"a2732bfc82475ae747a8fcdacf2f8ce421649e29"},{"author":{"_account_id":7677,"name":"Solly Ross","email":"sross@redhat.com","username":"sross"},"change_message_id":"9f723d12a40445276b3cb926510c3dc8e06df276","unresolved":false,"context_lines":[{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    # Create and start the NovaWebSockets proxy"},{"line_number":75,"context_line":"    server \u003d websocketproxy.NovaWebSocketProxy("},{"line_number":76,"context_line":"                traffic_type\u003d\u0027vnc\u0027,"},{"line_number":77,"context_line":"                listen_host\u003dCONF.novncproxy_host,"},{"line_number":78,"context_line":"                listen_port\u003dCONF.novncproxy_port,"},{"line_number":79,"context_line":"                source_is_ipv6\u003dCONF.source_is_ipv6,"}],"source_content_type":"text/x-python","patch_set":2,"id":"1abeadc6_63fceeb5","line":76,"in_reply_to":"1abeadc6_37da3e78","updated":"2014-08-29 16:58:40.000000000","message":"Done","commit_id":"a2732bfc82475ae747a8fcdacf2f8ce421649e29"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"4203c0eb641ec5715602857758213bf011190b08","unresolved":false,"context_lines":[{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    # Create and start the NovaWebSockets proxy"},{"line_number":76,"context_line":"    server \u003d websocketproxy.NovaWebSocketProxy("},{"line_number":77,"context_line":"                traffic_type\u003dsec_proxy.TRAFFIC_TYPE_VNC,"},{"line_number":78,"context_line":"                listen_host\u003dCONF.novncproxy_host,"},{"line_number":79,"context_line":"                listen_port\u003dCONF.novncproxy_port,"},{"line_number":80,"context_line":"                source_is_ipv6\u003dCONF.source_is_ipv6,"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_6fa037f5","line":77,"updated":"2014-11-10 13:08:35.000000000","message":"why not add this last?","commit_id":"2196213ba4a6ec6ccfec6b5eba52237a4924404b"},{"author":{"_account_id":7677,"name":"Solly Ross","email":"sross@redhat.com","username":"sross"},"change_message_id":"024929787ac71233166311190d02b29b3261e8f3","unresolved":false,"context_lines":[{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    # Create and start the NovaWebSockets proxy"},{"line_number":76,"context_line":"    server \u003d websocketproxy.NovaWebSocketProxy("},{"line_number":77,"context_line":"                traffic_type\u003dsec_proxy.TRAFFIC_TYPE_VNC,"},{"line_number":78,"context_line":"                listen_host\u003dCONF.novncproxy_host,"},{"line_number":79,"context_line":"                listen_port\u003dCONF.novncproxy_port,"},{"line_number":80,"context_line":"                source_is_ipv6\u003dCONF.source_is_ipv6,"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_02e31834","line":77,"in_reply_to":"5a890539_6fa037f5","updated":"2014-11-18 20:14:07.000000000","message":"Done","commit_id":"2196213ba4a6ec6ccfec6b5eba52237a4924404b"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"4912654f1937343de0f6677fb04e60b4704f4f04","unresolved":false,"context_lines":[{"line_number":87,"context_line":"                web\u003dCONF.web,"},{"line_number":88,"context_line":"                file_only\u003dTrue,"},{"line_number":89,"context_line":"                RequestHandlerClass\u003dwebsocketproxy.NovaProxyRequestHandler,"},{"line_number":90,"context_line":"                traffic_type\u003dsec_proxy.TRAFFIC_TYPE_VNC)"},{"line_number":91,"context_line":"    server.start_server()"}],"source_content_type":"text/x-python","patch_set":12,"id":"5a890539_9fbea7d2","line":90,"updated":"2014-12-04 11:05:02.000000000","message":"Instead of passing in traffic_type\u003d we should directly pass in the desired security proxy impl name eg\n\n   security_proxy\u003dCONF.novncproxy_security_driver","commit_id":"5f702f3a49db7ab71cf5ffcf6d759bcebc8dfe94"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"eb4ed455de1349597545cdce346db94b9f35fd68","unresolved":false,"context_lines":[{"line_number":31,"context_line":"               default\u003d6080,"},{"line_number":32,"context_line":"               help\u003d\u0027Port on which to listen for incoming requests\u0027),"},{"line_number":33,"context_line":"    cfg.StrOpt(\u0027novncproxy_security_driver\u0027,"},{"line_number":34,"context_line":"               default\u003dNone,"},{"line_number":35,"context_line":"               help\u003d(\"The VNC/Spice security proxy driver to use\""},{"line_number":36,"context_line":"                     \"to proxy security at the websocket proxy\")),"},{"line_number":37,"context_line":"    ]"}],"source_content_type":"text/x-python","patch_set":21,"id":"da86d52c_4195a83e","line":34,"updated":"2015-02-10 15:16:35.000000000","message":"by default the default value is None. This is not needed here","commit_id":"810835a6d98f88e272a43772f09b6bd3e7abeb7b"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"753807c86c306acf47a56add4f495e7f3e9dd79e","unresolved":false,"context_lines":[{"line_number":36,"context_line":"               max\u003d65535,"},{"line_number":37,"context_line":"               help\u003d\u0027Port on which to listen for incoming requests\u0027),"},{"line_number":38,"context_line":"    cfg.StrOpt(\u0027novncproxy_security_driver\u0027,"},{"line_number":39,"context_line":"               default\u003dNone,"},{"line_number":40,"context_line":"               help\u003d(\"The VNC/Spice security proxy driver to use\""},{"line_number":41,"context_line":"                     \"to proxy security at the websocket proxy\")),"},{"line_number":42,"context_line":"    ]"}],"source_content_type":"text/x-python","patch_set":25,"id":"9a1a9d01_6d783490","line":39,"updated":"2015-09-26 20:48:05.000000000","message":"this id not required the default value is None","commit_id":"4efbf28e8aaa8d6ec4d1768d0d165b71891f2cc6"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"753807c86c306acf47a56add4f495e7f3e9dd79e","unresolved":false,"context_lines":[{"line_number":37,"context_line":"               help\u003d\u0027Port on which to listen for incoming requests\u0027),"},{"line_number":38,"context_line":"    cfg.StrOpt(\u0027novncproxy_security_driver\u0027,"},{"line_number":39,"context_line":"               default\u003dNone,"},{"line_number":40,"context_line":"               help\u003d(\"The VNC/Spice security proxy driver to use\""},{"line_number":41,"context_line":"                     \"to proxy security at the websocket proxy\")),"},{"line_number":42,"context_line":"    ]"},{"line_number":43,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"9a1a9d01_0d6f38c6","line":40,"updated":"2015-09-26 20:48:05.000000000","message":"missing space at the end of the line","commit_id":"4efbf28e8aaa8d6ec4d1768d0d165b71891f2cc6"}],"nova/cmd/serialproxy.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"4203c0eb641ec5715602857758213bf011190b08","unresolved":false,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    # Create and start the NovaWebSockets proxy"},{"line_number":69,"context_line":"    server \u003d websocketproxy.NovaWebSocketProxy("},{"line_number":70,"context_line":"        traffic_type\u003dsec_proxy.TRAFFIC_TYPE_SERIAL,"},{"line_number":71,"context_line":"        listen_host\u003dCONF.serial_console.serialproxy_host,"},{"line_number":72,"context_line":"        listen_port\u003dCONF.serial_console.serialproxy_port,"},{"line_number":73,"context_line":"        source_is_ipv6\u003dCONF.source_is_ipv6,"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_afaa1fd2","line":70,"updated":"2014-11-10 13:08:35.000000000","message":"ditto?","commit_id":"2196213ba4a6ec6ccfec6b5eba52237a4924404b"},{"author":{"_account_id":7677,"name":"Solly Ross","email":"sross@redhat.com","username":"sross"},"change_message_id":"024929787ac71233166311190d02b29b3261e8f3","unresolved":false,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    # Create and start the NovaWebSockets proxy"},{"line_number":69,"context_line":"    server \u003d websocketproxy.NovaWebSocketProxy("},{"line_number":70,"context_line":"        traffic_type\u003dsec_proxy.TRAFFIC_TYPE_SERIAL,"},{"line_number":71,"context_line":"        listen_host\u003dCONF.serial_console.serialproxy_host,"},{"line_number":72,"context_line":"        listen_port\u003dCONF.serial_console.serialproxy_port,"},{"line_number":73,"context_line":"        source_is_ipv6\u003dCONF.source_is_ipv6,"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_c2bf50fb","line":70,"in_reply_to":"5a890539_afaa1fd2","updated":"2014-11-18 20:14:07.000000000","message":"Done","commit_id":"2196213ba4a6ec6ccfec6b5eba52237a4924404b"}],"nova/cmd/spicehtml5proxy.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"4203c0eb641ec5715602857758213bf011190b08","unresolved":false,"context_lines":[{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    # Create and start the NovaWebSockets proxy"},{"line_number":77,"context_line":"    server \u003d websocketproxy.NovaWebSocketProxy("},{"line_number":78,"context_line":"                traffic_type\u003dsec_proxy.TRAFFIC_TYPE_SPICE,"},{"line_number":79,"context_line":"                listen_host\u003dCONF.spice.html5proxy_host,"},{"line_number":80,"context_line":"                listen_port\u003dCONF.spice.html5proxy_port,"},{"line_number":81,"context_line":"                source_is_ipv6\u003dCONF.source_is_ipv6,"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_cfae8be7","line":78,"updated":"2014-11-10 13:08:35.000000000","message":"ditto","commit_id":"2196213ba4a6ec6ccfec6b5eba52237a4924404b"},{"author":{"_account_id":7677,"name":"Solly Ross","email":"sross@redhat.com","username":"sross"},"change_message_id":"024929787ac71233166311190d02b29b3261e8f3","unresolved":false,"context_lines":[{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    # Create and start the NovaWebSockets proxy"},{"line_number":77,"context_line":"    server \u003d websocketproxy.NovaWebSocketProxy("},{"line_number":78,"context_line":"                traffic_type\u003dsec_proxy.TRAFFIC_TYPE_SPICE,"},{"line_number":79,"context_line":"                listen_host\u003dCONF.spice.html5proxy_host,"},{"line_number":80,"context_line":"                listen_port\u003dCONF.spice.html5proxy_port,"},{"line_number":81,"context_line":"                source_is_ipv6\u003dCONF.source_is_ipv6,"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_223654b5","line":78,"in_reply_to":"5a890539_cfae8be7","updated":"2014-11-18 20:14:07.000000000","message":"Done","commit_id":"2196213ba4a6ec6ccfec6b5eba52237a4924404b"}],"nova/conf/vnc.py":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"b7438a1f452ba241f51aa16e608dcb9e08ece94c","unresolved":false,"context_lines":[{"line_number":184,"context_line":"novncproxy_security_driver \u003d cfg.StrOpt("},{"line_number":185,"context_line":"    \u0027novncproxy_security_driver\u0027,"},{"line_number":186,"context_line":"    help\u003d\"\"\""},{"line_number":187,"context_line":"The VNC/Spice security proxy driver to use"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"The VNC proxy normally communicates with the compute"},{"line_number":190,"context_line":"nodes in clear text. The security driver allows"}],"source_content_type":"text/x-python","patch_set":35,"id":"9a061dce_88098c34","line":187,"updated":"2016-04-04 16:10:45.000000000","message":"There is a new template used for the help message of new options introduced. You should probably use it here.","commit_id":"0338d9c432d7ed75e9acc96ae563685f4bad25ce"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7414c527301b053695918161c0235c492f391a0b","unresolved":false,"context_lines":[{"line_number":204,"context_line":""},{"line_number":205,"context_line":"Possible values:"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"* a dot separated python class subclassing WebSocketSecurityProxyHelper"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"Services which consume this:"},{"line_number":210,"context_line":""}],"source_content_type":"text/x-python","patch_set":42,"id":"3aaa91ec_1bf06b66","line":207,"updated":"2016-07-01 00:55:52.000000000","message":"This is adding another class path loader? I don\u0027t think we should do that.","commit_id":"913dcd77505e5c0fa039334db57c6d5cfe17c463"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"df8e33550bf1b89157ce260d2dba527da9e10842","unresolved":false,"context_lines":[{"line_number":204,"context_line":""},{"line_number":205,"context_line":"Possible values:"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"* a dot separated python class subclassing WebSocketSecurityProxyHelper"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"Services which consume this:"},{"line_number":210,"context_line":""}],"source_content_type":"text/x-python","patch_set":42,"id":"dada55a8_a9b1b091","line":207,"in_reply_to":"3aaa91ec_1bf06b66","updated":"2016-07-19 09:16:24.000000000","message":"Yeah, that kind of shows how many years these patches have been trying to get reviewer attention....\n\nI\u0027ll rewrite this to not use a loader.","commit_id":"913dcd77505e5c0fa039334db57c6d5cfe17c463"}],"nova/console/security_proxy_helper.py":[{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"0a393e57c9cf232dbc57bf59058155a2b24999bd","unresolved":false,"context_lines":[{"line_number":16,"context_line":"import socket"},{"line_number":17,"context_line":"import struct"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from oslo.config import cfg"},{"line_number":20,"context_line":"import six"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"from nova import exception"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_30de236e","line":19,"range":{"start_line":19,"start_character":0,"end_line":19,"end_character":27},"updated":"2014-08-20 10:11:42.000000000","message":"F401 \u0027cfg\u0027 imported but unused","commit_id":"d15a69d5e84cb8c03902c44b64d847de8e11e1ec"},{"author":{"_account_id":7677,"name":"Solly Ross","email":"sross@redhat.com","username":"sross"},"change_message_id":"7c66d8d9a71d47627f772a873899401741671d83","unresolved":false,"context_lines":[{"line_number":16,"context_line":"import socket"},{"line_number":17,"context_line":"import struct"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from oslo.config import cfg"},{"line_number":20,"context_line":"import six"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"from nova import exception"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_82ecfff4","line":19,"in_reply_to":"1abeadc6_30de236e","updated":"2014-08-20 17:09:04.000000000","message":"Done","commit_id":"d15a69d5e84cb8c03902c44b64d847de8e11e1ec"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"fc77f83a730ff1ddf0b5a97c5aed073ec09123a2","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        LOG.debug(\"Passed version string \u0027%s\u0027 from \""},{"line_number":101,"context_line":"                  \"client to server\", client_version[:-1])"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"        self._parse_version(client_version)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"        if self.rfb_version \u003c 3.7:"},{"line_number":106,"context_line":"            # Tell the client there\u0027s a problem"}],"source_content_type":"text/x-python","patch_set":2,"id":"1abeadc6_774e7639","line":103,"updated":"2014-08-29 10:35:36.000000000","message":"I think having a method called \u0027parse_version\u0027 which sets a property is a little misleading since the name doesn\u0027t suggest side-effects on global state. Lets make it work like\n\n   self.rfb_version \u003d self._parse_version(client_version)\n\nso _parse_version is side-effect fre","commit_id":"a2732bfc82475ae747a8fcdacf2f8ce421649e29"},{"author":{"_account_id":7677,"name":"Solly Ross","email":"sross@redhat.com","username":"sross"},"change_message_id":"14edd8d165e14f63379d84529975e44d14a926d8","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        LOG.debug(\"Passed version string \u0027%s\u0027 from \""},{"line_number":101,"context_line":"                  \"client to server\", client_version[:-1])"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"        self._parse_version(client_version)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"        if self.rfb_version \u003c 3.7:"},{"line_number":106,"context_line":"            # Tell the client there\u0027s a problem"}],"source_content_type":"text/x-python","patch_set":2,"id":"1abeadc6_43fa9243","line":103,"in_reply_to":"1abeadc6_774e7639","updated":"2014-08-29 17:03:56.000000000","message":"Done","commit_id":"a2732bfc82475ae747a8fcdacf2f8ce421649e29"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"fc77f83a730ff1ddf0b5a97c5aed073ec09123a2","unresolved":false,"context_lines":[{"line_number":139,"context_line":"        LOG.debug(\"The server send security types %s\", sec_types)"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"        # Negotiate security with client before we say \"ok\" to the server"},{"line_number":142,"context_line":"        self.client_sock.sendall(chr(1) + chr(1))  # send a choice of 1:[None]"},{"line_number":143,"context_line":"        client_auth \u003d ord(self.client_sock.recv(1))"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        if client_auth !\u003d 1:"}],"source_content_type":"text/x-python","patch_set":2,"id":"1abeadc6_37f6bed7","line":142,"updated":"2014-08-29 10:35:36.000000000","message":"Can you define constants for the common auth types eg\n\n  AUTH_TYPE_INVALID \u003d 0\n  AUTH_TYPE_NONE \u003d 1\n  AUTH_TYPE_VNC \u003d 2\n\nand use these instead of magic numbers in this file.","commit_id":"a2732bfc82475ae747a8fcdacf2f8ce421649e29"},{"author":{"_account_id":7677,"name":"Solly Ross","email":"sross@redhat.com","username":"sross"},"change_message_id":"9f723d12a40445276b3cb926510c3dc8e06df276","unresolved":false,"context_lines":[{"line_number":139,"context_line":"        LOG.debug(\"The server send security types %s\", sec_types)"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"        # Negotiate security with client before we say \"ok\" to the server"},{"line_number":142,"context_line":"        self.client_sock.sendall(chr(1) + chr(1))  # send a choice of 1:[None]"},{"line_number":143,"context_line":"        client_auth \u003d ord(self.client_sock.recv(1))"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        if client_auth !\u003d 1:"}],"source_content_type":"text/x-python","patch_set":2,"id":"1abeadc6_4347f2bb","line":142,"in_reply_to":"1abeadc6_37f6bed7","updated":"2014-08-29 16:58:40.000000000","message":"Done","commit_id":"a2732bfc82475ae747a8fcdacf2f8ce421649e29"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"fc77f83a730ff1ddf0b5a97c5aed073ec09123a2","unresolved":false,"context_lines":[{"line_number":142,"context_line":"        self.client_sock.sendall(chr(1) + chr(1))  # send a choice of 1:[None]"},{"line_number":143,"context_line":"        client_auth \u003d ord(self.client_sock.recv(1))"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        if client_auth !\u003d 1:"},{"line_number":146,"context_line":"            # let the client and server know something went wrong"},{"line_number":147,"context_line":"            self._fail(_(\"Only the security type None (1) is supported\"))"},{"line_number":148,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1abeadc6_d7dbca4c","line":145,"updated":"2014-08-29 10:35:36.000000000","message":"if client_auth !\u003d AUTH_TYPE_NONE","commit_id":"a2732bfc82475ae747a8fcdacf2f8ce421649e29"},{"author":{"_account_id":7677,"name":"Solly Ross","email":"sross@redhat.com","username":"sross"},"change_message_id":"9f723d12a40445276b3cb926510c3dc8e06df276","unresolved":false,"context_lines":[{"line_number":142,"context_line":"        self.client_sock.sendall(chr(1) + chr(1))  # send a choice of 1:[None]"},{"line_number":143,"context_line":"        client_auth \u003d ord(self.client_sock.recv(1))"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        if client_auth !\u003d 1:"},{"line_number":146,"context_line":"            # let the client and server know something went wrong"},{"line_number":147,"context_line":"            self._fail(_(\"Only the security type None (1) is supported\"))"},{"line_number":148,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1abeadc6_233cb62e","line":145,"in_reply_to":"1abeadc6_d7dbca4c","updated":"2014-08-29 16:58:40.000000000","message":"Done","commit_id":"a2732bfc82475ae747a8fcdacf2f8ce421649e29"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"fc77f83a730ff1ddf0b5a97c5aed073ec09123a2","unresolved":false,"context_lines":[{"line_number":166,"context_line":""},{"line_number":167,"context_line":"        # security-type-specific stuff"},{"line_number":168,"context_line":"        try:"},{"line_number":169,"context_line":"            self.secure_server_sock \u003d self.security_handshake()"},{"line_number":170,"context_line":"        except exception.RFBSecurityNegotiationFailed as e:"},{"line_number":171,"context_line":"            # let the client and server know something went wrong"},{"line_number":172,"context_line":"            self._fail(e, server_send\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1abeadc6_9706b284","line":169,"updated":"2014-08-29 10:35:36.000000000","message":"Do we actually need to keep a record of the \u0027server_sock\u0027 once we have the secure socket ?  If not, I\u0027d be inclined to get rid of this separate secure_server_sock property and just overwrite the \u0027server_sock\u0027 instead. That way we\u0027ll be sure not subclass will accidentally start sending data on the plain socket after the secure socket has been established.","commit_id":"a2732bfc82475ae747a8fcdacf2f8ce421649e29"},{"author":{"_account_id":7677,"name":"Solly Ross","email":"sross@redhat.com","username":"sross"},"change_message_id":"9f723d12a40445276b3cb926510c3dc8e06df276","unresolved":false,"context_lines":[{"line_number":166,"context_line":""},{"line_number":167,"context_line":"        # security-type-specific stuff"},{"line_number":168,"context_line":"        try:"},{"line_number":169,"context_line":"            self.secure_server_sock \u003d self.security_handshake()"},{"line_number":170,"context_line":"        except exception.RFBSecurityNegotiationFailed as e:"},{"line_number":171,"context_line":"            # let the client and server know something went wrong"},{"line_number":172,"context_line":"            self._fail(e, server_send\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1abeadc6_431e12bc","line":169,"in_reply_to":"1abeadc6_9706b284","updated":"2014-08-29 16:58:40.000000000","message":"Done","commit_id":"a2732bfc82475ae747a8fcdacf2f8ce421649e29"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"fc77f83a730ff1ddf0b5a97c5aed073ec09123a2","unresolved":false,"context_lines":[{"line_number":189,"context_line":"        \"\"\""},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        # the default is to use \"None\""},{"line_number":192,"context_line":"        return 1"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"    def security_handshake(self):"},{"line_number":195,"context_line":"        \"\"\"Perform security-type-specific functionality."}],"source_content_type":"text/x-python","patch_set":2,"id":"1abeadc6_97d5d21d","line":192,"updated":"2014-08-29 10:35:36.000000000","message":"return AUTH_TYPE_NONE","commit_id":"a2732bfc82475ae747a8fcdacf2f8ce421649e29"},{"author":{"_account_id":7677,"name":"Solly Ross","email":"sross@redhat.com","username":"sross"},"change_message_id":"9f723d12a40445276b3cb926510c3dc8e06df276","unresolved":false,"context_lines":[{"line_number":189,"context_line":"        \"\"\""},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        # the default is to use \"None\""},{"line_number":192,"context_line":"        return 1"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"    def security_handshake(self):"},{"line_number":195,"context_line":"        \"\"\"Perform security-type-specific functionality."}],"source_content_type":"text/x-python","patch_set":2,"id":"1abeadc6_63422eac","line":192,"in_reply_to":"1abeadc6_97d5d21d","updated":"2014-08-29 16:58:40.000000000","message":"Done","commit_id":"a2732bfc82475ae747a8fcdacf2f8ce421649e29"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"535bb8f37abddb659aa1cfcc0ec91d80ae699aff","unresolved":false,"context_lines":[{"line_number":50,"context_line":"        elif traffic_type \u003d\u003d TRAFFIC_TYPE_SPICE:"},{"line_number":51,"context_line":"            helper_cls \u003d self.SPICE_HELPER"},{"line_number":52,"context_line":"        else:"},{"line_number":53,"context_line":"            helper_cls \u003d self.GENERIC_HELPER"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        self.traffic_type \u003d traffic_type"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_faeb1ea1","line":53,"updated":"2014-11-18 19:27:52.000000000","message":"The existing code requires modification if additional helper classes are added (beyond VNC, SPICE, and generic helpers). Why not specialize (i.e., subclass) WebSocketSecurityProxy for each traffic type?","commit_id":"2196213ba4a6ec6ccfec6b5eba52237a4924404b"},{"author":{"_account_id":7677,"name":"Solly Ross","email":"sross@redhat.com","username":"sross"},"change_message_id":"024929787ac71233166311190d02b29b3261e8f3","unresolved":false,"context_lines":[{"line_number":50,"context_line":"        elif traffic_type \u003d\u003d TRAFFIC_TYPE_SPICE:"},{"line_number":51,"context_line":"            helper_cls \u003d self.SPICE_HELPER"},{"line_number":52,"context_line":"        else:"},{"line_number":53,"context_line":"            helper_cls \u003d self.GENERIC_HELPER"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        self.traffic_type \u003d traffic_type"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_a2f7a4d3","line":53,"in_reply_to":"5a890539_faeb1ea1","updated":"2014-11-18 20:14:07.000000000","message":"A deployer may be using both VNC and SPICE, so you want to be able to say \"use TLS\" and have that work with both.","commit_id":"2196213ba4a6ec6ccfec6b5eba52237a4924404b"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"535bb8f37abddb659aa1cfcc0ec91d80ae699aff","unresolved":false,"context_lines":[{"line_number":92,"context_line":"    AUTH_TYPE_NONE \u003d 1"},{"line_number":93,"context_line":"    AUTH_TYPE_VNC \u003d 2"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"    def __init__(self, client_sock, server_sock):"},{"line_number":96,"context_line":"        self.client_sock \u003d client_sock"},{"line_number":97,"context_line":"        self.server_sock \u003d server_sock"},{"line_number":98,"context_line":"        self.rfb_version \u003d None"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_7ab80e75","line":95,"updated":"2014-11-18 19:27:52.000000000","message":"It looks like the client and server sockets have already been opened. If so, why are they closed on failures below?\n\nIn general, I believe that sockets, file handles, etc. should be closed by the same class that opened them.","commit_id":"2196213ba4a6ec6ccfec6b5eba52237a4924404b"},{"author":{"_account_id":7677,"name":"Solly Ross","email":"sross@redhat.com","username":"sross"},"change_message_id":"024929787ac71233166311190d02b29b3261e8f3","unresolved":false,"context_lines":[{"line_number":92,"context_line":"    AUTH_TYPE_NONE \u003d 1"},{"line_number":93,"context_line":"    AUTH_TYPE_VNC \u003d 2"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"    def __init__(self, client_sock, server_sock):"},{"line_number":96,"context_line":"        self.client_sock \u003d client_sock"},{"line_number":97,"context_line":"        self.server_sock \u003d server_sock"},{"line_number":98,"context_line":"        self.rfb_version \u003d None"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_e29d2c68","line":95,"in_reply_to":"5a890539_7ab80e75","updated":"2014-11-18 20:14:07.000000000","message":"fair enough","commit_id":"2196213ba4a6ec6ccfec6b5eba52237a4924404b"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"2bc85b9335dfa503ed1294f5b6d584069aaf8e28","unresolved":false,"context_lines":[{"line_number":50,"context_line":"        elif traffic_type \u003d\u003d TRAFFIC_TYPE_SPICE:"},{"line_number":51,"context_line":"            helper_cls \u003d self.SPICE_HELPER"},{"line_number":52,"context_line":"        else:"},{"line_number":53,"context_line":"            helper_cls \u003d self.GENERIC_HELPER"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        self.traffic_type \u003d traffic_type"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_ba309108","line":53,"updated":"2014-11-19 13:56:29.000000000","message":"Following the comment on patch set 7...\n\nSorry to be dense, but I\u0027m not sure that I follow this response.... a specialized class could work for multiple protocols.\n\nWhat if a different traffic type (i.e., not VNC or SPICE) is used, but we don\u0027t want to use the generic helper? Modifications would be required to this code; modifications that might be avoided if subclasses are created for each traffic type and instantiated directly rather than using if...elif...else logic. (The existing code seems to violate the open/closed principle -- see http://en.wikipedia.org/wiki/Open/closed_principle -- which should probably be avoided unless it\u0027s impractical to follow in this case.)","commit_id":"214a1c84a39968eb7350b8e7c8aff1ceafd2b441"},{"author":{"_account_id":7677,"name":"Solly Ross","email":"sross@redhat.com","username":"sross"},"change_message_id":"bb6e7cebc2bc9a19b9daca9bc574a981faf401df","unresolved":false,"context_lines":[{"line_number":50,"context_line":"        elif traffic_type \u003d\u003d TRAFFIC_TYPE_SPICE:"},{"line_number":51,"context_line":"            helper_cls \u003d self.SPICE_HELPER"},{"line_number":52,"context_line":"        else:"},{"line_number":53,"context_line":"            helper_cls \u003d self.GENERIC_HELPER"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        self.traffic_type \u003d traffic_type"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_dafd5870","line":53,"in_reply_to":"5a890539_ba309108","updated":"2014-11-19 15:47:50.000000000","message":"I\u0027m a bit confused as to how this violates the open/closed principle.  The way you\u0027d use this class, in general, would be to subclass it, and set the VNC_HELPER, SPICE_HELPER, or GENERIC_HELPER  (which is really designed for traffic that doesn\u0027t have a connection protocol, like the serial console traffic) class variables.\n\nIf you introduced another traffic type, you could either a) subclass the class and override init or b) modify this class to have another category (if, say, you wanted to allow all the different subclasses of this class to support having a new option), c): subclass this class and make it another generic class that had the extra option.\n\nI\u0027m a bit unclear as to what your are suggestiong as an alternative.  Are you suggesting having a separate config option for each traffic type as an alternative?","commit_id":"214a1c84a39968eb7350b8e7c8aff1ceafd2b441"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"c71e153a4550e0fd8717b07bfbabf41d8cd689c0","unresolved":false,"context_lines":[{"line_number":71,"context_line":"                security_type\u003dtype(self).__name__)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"class RFBSecurityProxyHelper(object):"},{"line_number":75,"context_line":"    \"\"\"RFB Security Proxy Negotiation Helper."},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"    This class proxies the initial setup of the RFB"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a890539_b3ab3388","line":74,"updated":"2014-11-20 14:24:34.000000000","message":"I think this split of WebSocketSecurityProxy vs RBSecurityProxyHelper vs NOPSecurityProxy (and later  VeNCryptSecurityProxyHelper \u0026 TLSSecurityProxy) classes is a solution in search of a problem really.\n\nI understand why you\u0027ve done this - you\u0027re trying to make the *SecurityProxy class be able to provide multiple different helpers for VNC, SPICE, etc at the same time, but I don\u0027t think that\u0027s actually a problem that needs solving.\n\nI think we just want a much simplified setup consisting of RFBSecurityProxy and VeNCryptSecurityProxy subclassing it, and configure each server separately, instead of trying use a single nova.conf option to configure all consoles at once.","commit_id":"f5698202efb8435e4dd94abab9a1e7db35bdd0d6"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"4912654f1937343de0f6677fb04e60b4704f4f04","unresolved":false,"context_lines":[{"line_number":77,"context_line":"        else:"},{"line_number":78,"context_line":"            raise exception.SecurityProxyingNotImplementedError("},{"line_number":79,"context_line":"                traffic_type\u003dself.traffic_type,"},{"line_number":80,"context_line":"                security_type\u003dtype(self).__name__)"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"class RFBSecurityProxyHelper(object):"}],"source_content_type":"text/x-python","patch_set":12,"id":"5a890539_df0f4fde","line":80,"updated":"2014-12-04 11:05:02.000000000","message":"I think this entire class can be simplified to just a single method\n\n  def connect(self):\n      raise NotImplementedException()\n\nAnd RFBSecurityProxyHelper should just sub-class it, thus implementing the \u0027connect\u0027 method.  Then the desired security proxy class impl can be passed straight intot he constructor of NovaWebSocketProxy","commit_id":"5f702f3a49db7ab71cf5ffcf6d759bcebc8dfe94"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"672f8db80bc1c5c501d3b9fb2ef06004a753bf59","unresolved":false,"context_lines":[{"line_number":24,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"class RFBSecurityProxyHelper(object):"},{"line_number":28,"context_line":"    \"\"\"RFB Security Proxy Negotiation Helper."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    This class proxies the initial setup of the RFB"}],"source_content_type":"text/x-python","patch_set":13,"id":"5a890539_5b1a667d","line":27,"updated":"2014-12-05 12:32:03.000000000","message":"As mentioned in the previous review there should be a base class defining the API contract that NovaProxyRequestHandlerBase is expecting, and this class should subclass \u0026 implement that contract.","commit_id":"a02e6c856996c979314de4460c5480707d153c36"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"c183302342e1266d9c0bcd7fe2b8479c04f0b58b","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        raise NotImplementedError()"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"class RFBSecurityProxyHelper(object):"},{"line_number":43,"context_line":"    \"\"\"RFB Security Proxy Negotiation Helper."},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    This class proxies the initial setup of the RFB"}],"source_content_type":"text/x-python","patch_set":14,"id":"5a890539_9d84b85e","line":42,"updated":"2014-12-05 17:16:16.000000000","message":"s/object/WebsocketSecurityProxyHelper/","commit_id":"ee06b1494977caf1e9ed76015d7d69ca21896610"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"601e45a28a1c251ce930ece062ed6ab580396582","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    Note: only versions 3.7 onward are supported."},{"line_number":56,"context_line":"    \"\"\""},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    AUTH_TYPE_INVALID \u003d 0"},{"line_number":59,"context_line":"    AUTH_TYPE_NONE \u003d 1"},{"line_number":60,"context_line":"    AUTH_TYPE_VNC \u003d 2"},{"line_number":61,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"3a961159_3f96d63e","line":58,"updated":"2015-01-06 14:16:42.000000000","message":"Unused constant?!","commit_id":"e19b4318d82a0912dd9b35714d9c7dcf4f9597af"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"601e45a28a1c251ce930ece062ed6ab580396582","unresolved":false,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    AUTH_TYPE_INVALID \u003d 0"},{"line_number":59,"context_line":"    AUTH_TYPE_NONE \u003d 1"},{"line_number":60,"context_line":"    AUTH_TYPE_VNC \u003d 2"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def __init__(self, client_sock, server_sock):"},{"line_number":63,"context_line":"        self.client_sock \u003d client_sock"}],"source_content_type":"text/x-python","patch_set":18,"id":"3a961159_bfa9e67d","line":60,"updated":"2015-01-06 14:16:42.000000000","message":"Ditto","commit_id":"e19b4318d82a0912dd9b35714d9c7dcf4f9597af"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"601e45a28a1c251ce930ece062ed6ab580396582","unresolved":false,"context_lines":[{"line_number":140,"context_line":""},{"line_number":141,"context_line":"        sec_types \u003d [ord(t) for t in self.server_sock.recv(sec_types_cnt)]"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"        LOG.debug(\"The server send security types %s\", sec_types)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        # Negotiate security with client before we say \"ok\" to the server"},{"line_number":146,"context_line":"        # send 1:[None]"}],"source_content_type":"text/x-python","patch_set":18,"id":"3a961159_5fe9e2b5","line":143,"updated":"2015-01-06 14:16:42.000000000","message":"Typo: send -\u003e sent","commit_id":"e19b4318d82a0912dd9b35714d9c7dcf4f9597af"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"601e45a28a1c251ce930ece062ed6ab580396582","unresolved":false,"context_lines":[{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        # Negotiate security with client before we say \"ok\" to the server"},{"line_number":146,"context_line":"        # send 1:[None]"},{"line_number":147,"context_line":"        self.client_sock.sendall(chr(1) + chr(self.AUTH_TYPE_NONE))"},{"line_number":148,"context_line":"        client_auth \u003d ord(self.client_sock.recv(1))"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"        if client_auth !\u003d self.AUTH_TYPE_NONE:"}],"source_content_type":"text/x-python","patch_set":18,"id":"3a961159_ff2b6ee8","line":147,"updated":"2015-01-06 14:16:42.000000000","message":"Why \"chr(1)\" instead of \"\\x01\" as above?","commit_id":"e19b4318d82a0912dd9b35714d9c7dcf4f9597af"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"601e45a28a1c251ce930ece062ed6ab580396582","unresolved":false,"context_lines":[{"line_number":153,"context_line":"                       % self.AUTH_TYPE_NONE)"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"            # raise an exception in the proxy"},{"line_number":156,"context_line":"            reason \u003d _(\"Client requrested a security type other than \""},{"line_number":157,"context_line":"                       \" None (%(none_code)s): \""},{"line_number":158,"context_line":"                       \"%(auth_type)s\") % {\u0027auth_type\u0027: client_auth,"},{"line_number":159,"context_line":"                                           \u0027none_code\u0027: self.AUTH_TYPE_NONE}"}],"source_content_type":"text/x-python","patch_set":18,"id":"3a961159_1f311a5a","line":156,"updated":"2015-01-06 14:16:42.000000000","message":"Typo: requrested -\u003e requested","commit_id":"e19b4318d82a0912dd9b35714d9c7dcf4f9597af"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"601e45a28a1c251ce930ece062ed6ab580396582","unresolved":false,"context_lines":[{"line_number":170,"context_line":"        self.server_sock.sendall(chr(chosen_security_type))"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        LOG.debug(\u0027Using security type %s with server\u0027, chosen_security_type)"},{"line_number":173,"context_line":"        LOG.debug(\"Using security type None (1) with client\")"},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"        # security-type-specific stuff"},{"line_number":176,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":18,"id":"3a961159_5f37224d","line":173,"updated":"2015-01-06 14:16:42.000000000","message":"Avoid hardcoding \"1\" when a constant is defined (AUTH_TYPE_NONE).","commit_id":"e19b4318d82a0912dd9b35714d9c7dcf4f9597af"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"24625d29ceab52ff5a8ea82de6de76627462ece2","unresolved":false,"context_lines":[{"line_number":79,"context_line":"        if server_send:"},{"line_number":80,"context_line":"            # Tell the server that there\u0027s been a problem"},{"line_number":81,"context_line":"            # by sending the \"Invalid\" security type"},{"line_number":82,"context_line":"            self.server_sock.sendall(b\"\\x00\")"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    def _parse_version(self, version_str):"},{"line_number":85,"context_line":"        num_str \u003d version_str[4:11]"}],"source_content_type":"text/x-python","patch_set":20,"id":"3a961159_655b4186","line":82,"updated":"2015-01-07 22:09:53.000000000","message":"What are these constants -- \\x00, \\x01, etc.? Should there be a constant for them?","commit_id":"eaf63d864018f11083ee41ee7c19ba87e5c8b787"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"24625d29ceab52ff5a8ea82de6de76627462ece2","unresolved":false,"context_lines":[{"line_number":117,"context_line":"            reason_str \u003d _(\"Security proxying requires RFB protocol \""},{"line_number":118,"context_line":"                           \"version 3.7 or above\")"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"            self.client_sock.sendall(b\"\\x00\" + self._make_var_str(reason_str))"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"            reason \u003d _(\"Security proxying requires RFB protocol \""},{"line_number":123,"context_line":"                       \"version 3.7 or above, but the client and server \""}],"source_content_type":"text/x-python","patch_set":20,"id":"3a961159_85558d51","line":120,"updated":"2015-01-07 22:09:53.000000000","message":"Ditto","commit_id":"eaf63d864018f11083ee41ee7c19ba87e5c8b787"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"24625d29ceab52ff5a8ea82de6de76627462ece2","unresolved":false,"context_lines":[{"line_number":132,"context_line":"            reason_len \u003d struct.unpack(\u0027!I\u0027, reason_len_raw)[0]"},{"line_number":133,"context_line":"            reason \u003d self.server_sock.recv(reason_len)"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"            self.client_sock.sendall(b\"\\x00\" + reason_len_raw + reason)"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"            raise exception.RFBSecurityNegotiationFailed(reason\u003dreason)"},{"line_number":138,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"3a961159_a550c95e","line":135,"updated":"2015-01-07 22:09:53.000000000","message":"Ditto","commit_id":"eaf63d864018f11083ee41ee7c19ba87e5c8b787"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"24625d29ceab52ff5a8ea82de6de76627462ece2","unresolved":false,"context_lines":[{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        # Negotiate security with client before we say \"ok\" to the server"},{"line_number":145,"context_line":"        # send 1:[None]"},{"line_number":146,"context_line":"        self.client_sock.sendall(b\"\\x01\" + six.int2byte(self.AUTH_TYPE_NONE))"},{"line_number":147,"context_line":"        client_auth \u003d six.byte2int(self.client_sock.recv(1))"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"        if client_auth !\u003d self.AUTH_TYPE_NONE:"}],"source_content_type":"text/x-python","patch_set":20,"id":"3a961159_05c99d25","line":146,"updated":"2015-01-07 22:09:53.000000000","message":"Ditto","commit_id":"eaf63d864018f11083ee41ee7c19ba87e5c8b787"},{"author":{"_account_id":7677,"name":"Solly Ross","email":"sross@redhat.com","username":"sross"},"change_message_id":"eb4b3a663df364b84ff1c3dfe074d2bd0cc6625f","unresolved":false,"context_lines":[{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        # Negotiate security with client before we say \"ok\" to the server"},{"line_number":145,"context_line":"        # send 1:[None]"},{"line_number":146,"context_line":"        self.client_sock.sendall(b\"\\x01\" + six.int2byte(self.AUTH_TYPE_NONE))"},{"line_number":147,"context_line":"        client_auth \u003d six.byte2int(self.client_sock.recv(1))"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"        if client_auth !\u003d self.AUTH_TYPE_NONE:"}],"source_content_type":"text/x-python","patch_set":20,"id":"3a961159_c77f8415","line":146,"in_reply_to":"3a961159_05c99d25","updated":"2015-01-08 15:34:50.000000000","message":"This is not a constant in the traditional sense -- it\u0027s just the number of security types that we\u0027re sending (as noted in the comment above -- sorry if that\u0027s unclear)","commit_id":"eaf63d864018f11083ee41ee7c19ba87e5c8b787"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"864b7b113c34dce07dd5e44fed4e4cc13f2169f4","unresolved":false,"context_lines":[{"line_number":25,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"class WebsocketSecurityProxyHelper(object):"},{"line_number":29,"context_line":"    \"\"\"A WebSocket Security Proxy Helper"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    WebSocket security proxy helpers should sublclass"}],"source_content_type":"text/x-python","patch_set":25,"id":"9a1a9d01_3375c897","line":28,"updated":"2015-09-25 23:05:21.000000000","message":"what about marking this class as abstract?","commit_id":"4efbf28e8aaa8d6ec4d1768d0d165b71891f2cc6"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"864b7b113c34dce07dd5e44fed4e4cc13f2169f4","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        object to use to communicate with the server securely."},{"line_number":99,"context_line":"        \"\"\""},{"line_number":100,"context_line":"        # Server ProtocolVersion --\u003e Client"},{"line_number":101,"context_line":"        server_version \u003d self.server_sock.recv(12)"},{"line_number":102,"context_line":"        self.client_sock.sendall(server_version)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        LOG.debug(\"Passed version string \u0027%s\u0027 from \""}],"source_content_type":"text/x-python","patch_set":25,"id":"9a1a9d01_b3b0f844","line":101,"updated":"2015-09-25 23:05:21.000000000","message":"could you move 12 to constant?","commit_id":"4efbf28e8aaa8d6ec4d1768d0d165b71891f2cc6"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"864b7b113c34dce07dd5e44fed4e4cc13f2169f4","unresolved":false,"context_lines":[{"line_number":105,"context_line":"                  \"server to client\", server_version[:-1])"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        # Client ProtocolVersion --\u003e Server"},{"line_number":108,"context_line":"        client_version \u003d self.client_sock.recv(12)"},{"line_number":109,"context_line":"        self.server_sock.sendall(client_version)"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"        LOG.debug(\"Passed version string \u0027%s\u0027 from \""}],"source_content_type":"text/x-python","patch_set":25,"id":"9a1a9d01_93b5b432","line":108,"updated":"2015-09-25 23:05:21.000000000","message":"ditto","commit_id":"4efbf28e8aaa8d6ec4d1768d0d165b71891f2cc6"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"753807c86c306acf47a56add4f495e7f3e9dd79e","unresolved":false,"context_lines":[{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        LOG.debug(\"Using security type %s with server\", chosen_security_type)"},{"line_number":173,"context_line":"        LOG.debug(\"Using security type None (%s) \""},{"line_number":174,"context_line":"                  \"with client\" % self.AUTH_TYPE_NONE)"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"        # security-type-specific stuff"},{"line_number":177,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":25,"id":"9a1a9d01_ed6444e6","line":174,"updated":"2015-09-26 20:48:05.000000000","message":"please use a , instead of the %","commit_id":"4efbf28e8aaa8d6ec4d1768d0d165b71891f2cc6"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"753807c86c306acf47a56add4f495e7f3e9dd79e","unresolved":false,"context_lines":[{"line_number":176,"context_line":"        # security-type-specific stuff"},{"line_number":177,"context_line":"        try:"},{"line_number":178,"context_line":"            self.server_sock \u003d self.security_handshake()"},{"line_number":179,"context_line":"        except exception.RFBSecurityNegotiationFailed as e:"},{"line_number":180,"context_line":"            # let the client and server know something went wrong"},{"line_number":181,"context_line":"            self._fail(e, server_send\u003dFalse)"},{"line_number":182,"context_line":"            raise"}],"source_content_type":"text/x-python","patch_set":25,"id":"9a1a9d01_6d9fd4b4","line":179,"updated":"2015-09-26 20:48:05.000000000","message":"why not use: with excutils.save_and_reraise_exception():","commit_id":"4efbf28e8aaa8d6ec4d1768d0d165b71891f2cc6"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"9aec97df387cd938f508f57cf7232eda71f87a4b","unresolved":false,"context_lines":[{"line_number":26,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"class WebsocketSecurityProxyHelper(object):"},{"line_number":30,"context_line":"    \"\"\"A WebSocket Security Proxy Helper"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    WebSocket security proxy helpers should sublclass"}],"source_content_type":"text/x-python","patch_set":28,"id":"ba8a016a_1348f186","line":29,"updated":"2015-11-20 23:16:38.000000000","message":"as in the comments below, it would be better if this was \"WebSocket...\" with a capital \u0027S\u0027 in \"Socket\".","commit_id":"6f56e9cf52cf5274a896e44394061ed6ea29aeff"},{"author":{"_account_id":8190,"name":"Drew Thorstensen (thorst)","email":"thorst@us.ibm.com","username":"thorst"},"change_message_id":"fb5a186a714d8f5f2b40faf69efb3e5a7e4597cf","unresolved":false,"context_lines":[{"line_number":29,"context_line":"class WebsocketSecurityProxyHelper(object):"},{"line_number":30,"context_line":"    \"\"\"A WebSocket Security Proxy Helper"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    WebSocket security proxy helpers should sublclass"},{"line_number":33,"context_line":"    this class and implement a generic `connect`"},{"line_number":34,"context_line":"    for the particular protocol being used."},{"line_number":35,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"ba8a016a_c2bd0bd9","line":32,"updated":"2015-11-19 13:35:33.000000000","message":"Typo: subclass.","commit_id":"6f56e9cf52cf5274a896e44394061ed6ea29aeff"},{"author":{"_account_id":8190,"name":"Drew Thorstensen (thorst)","email":"thorst@us.ibm.com","username":"thorst"},"change_message_id":"fb5a186a714d8f5f2b40faf69efb3e5a7e4597cf","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    WebSocket security proxy helpers should sublclass"},{"line_number":33,"context_line":"    this class and implement a generic `connect`"},{"line_number":34,"context_line":"    for the particular protocol being used."},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    Security drivers can then sublcass the"},{"line_number":37,"context_line":"    protocol-specific helper class."}],"source_content_type":"text/x-python","patch_set":28,"id":"ba8a016a_e28e0f2d","line":34,"updated":"2015-11-19 13:35:33.000000000","message":"Might be good to explain what this connect method is supposed to do.  I would assume handle the security exchange between the source and the target.  Or is it meant to be even more generic than that?","commit_id":"6f56e9cf52cf5274a896e44394061ed6ea29aeff"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"9aec97df387cd938f508f57cf7232eda71f87a4b","unresolved":false,"context_lines":[{"line_number":54,"context_line":"    After security negotiation, normal proxying can"},{"line_number":55,"context_line":"    be used."},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    Note: only versions 3.7 onward are supported."},{"line_number":58,"context_line":"    \"\"\""},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    AUTH_TYPE_NONE \u003d 1"}],"source_content_type":"text/x-python","patch_set":28,"id":"ba8a016a_0e244aad","line":57,"updated":"2015-11-20 23:16:38.000000000","message":"s/versions/RFB versions/ to make it clearer what you\u0027re talking about.","commit_id":"6f56e9cf52cf5274a896e44394061ed6ea29aeff"},{"author":{"_account_id":8190,"name":"Drew Thorstensen (thorst)","email":"thorst@us.ibm.com","username":"thorst"},"change_message_id":"fb5a186a714d8f5f2b40faf69efb3e5a7e4597cf","unresolved":false,"context_lines":[{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    VERSION_LENGTH \u003d 12"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    AUTH_FAIL \u003d b\"\\x00\""},{"line_number":65,"context_line":"    AUTH_PASS \u003d b\"\\x01\""},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    def __init__(self, client_sock, server_sock):"}],"source_content_type":"text/x-python","patch_set":28,"id":"ba8a016a_02ad5387","line":64,"updated":"2015-11-19 13:35:33.000000000","message":"Might be good to reference the spec in this class where these constants are defined.","commit_id":"6f56e9cf52cf5274a896e44394061ed6ea29aeff"},{"author":{"_account_id":8190,"name":"Drew Thorstensen (thorst)","email":"thorst@us.ibm.com","username":"thorst"},"change_message_id":"fb5a186a714d8f5f2b40faf69efb3e5a7e4597cf","unresolved":false,"context_lines":[{"line_number":187,"context_line":"        except exception.RFBSecurityNegotiationFailed as e:"},{"line_number":188,"context_line":"            # let the client and server know something went wrong"},{"line_number":189,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":190,"context_line":"                self._fail(e, server_send\u003dFalse)"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"        LOG.debug(\"Finished security handshake, resuming normal proxy \""},{"line_number":193,"context_line":"                  \"mode using secured socket\")"}],"source_content_type":"text/x-python","patch_set":28,"id":"ba8a016a_820b837c","line":190,"updated":"2015-11-19 13:35:33.000000000","message":"just curious why the server doesn\u0027t send the error here?  Rather it waits for the socket to flush.","commit_id":"6f56e9cf52cf5274a896e44394061ed6ea29aeff"},{"author":{"_account_id":8190,"name":"Drew Thorstensen (thorst)","email":"thorst@us.ibm.com","username":"thorst"},"change_message_id":"0a547a0d07523635d400c80693b16863f875b9af","unresolved":false,"context_lines":[{"line_number":29,"context_line":"class WebSocketSecurityProxyHelper(object):"},{"line_number":30,"context_line":"    \"\"\"A WebSocket Security Proxy Helper"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    WebSocket security proxy helpers should sublclass"},{"line_number":33,"context_line":"    this class and implement a generic `connect`"},{"line_number":34,"context_line":"    for the particular protocol being used."},{"line_number":35,"context_line":""}],"source_content_type":"text/x-python","patch_set":32,"id":"9a68dd71_0c80dc45","line":32,"range":{"start_line":32,"start_character":44,"end_line":32,"end_character":53},"updated":"2016-01-26 15:48:30.000000000","message":"Typo - subclass.","commit_id":"397b6ff181d8d6530ce7205e593591f8cf3b2eb6"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"b7438a1f452ba241f51aa16e608dcb9e08ece94c","unresolved":false,"context_lines":[{"line_number":111,"context_line":""},{"line_number":112,"context_line":"        This method performs the initial ProtocolVersion"},{"line_number":113,"context_line":"        and Security messaging, and returns the socket-like"},{"line_number":114,"context_line":"        object to use to communicate with the server securely."},{"line_number":115,"context_line":"        \"\"\""},{"line_number":116,"context_line":"        # Server ProtocolVersion --\u003e Client"},{"line_number":117,"context_line":"        server_version \u003d self.server_sock.recv(self.VERSION_LENGTH)"}],"source_content_type":"text/x-python","patch_set":35,"id":"9a061dce_0b529613","line":114,"updated":"2016-04-04 16:10:45.000000000","message":"This method is returning a socket, I think you should to update the doctext to refer it OR Probably remove the return of this method and refer what this method can raises whether something wrong happens (RFBSecurityNegotiationFailed).","commit_id":"0338d9c432d7ed75e9acc96ae563685f4bad25ce"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7414c527301b053695918161c0235c492f391a0b","unresolved":false,"context_lines":[{"line_number":172,"context_line":"        client_auth \u003d six.byte2int(self.client_sock.recv(1))"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"        if client_auth !\u003d self.AUTH_TYPE_NONE:"},{"line_number":175,"context_line":"            # let the client and server know something went wrong"},{"line_number":176,"context_line":"            self._fail(_(\"Only the security type None (%s) is supported\") %"},{"line_number":177,"context_line":"                       self.AUTH_TYPE_NONE)"},{"line_number":178,"context_line":""}],"source_content_type":"text/x-python","patch_set":42,"id":"3aaa91ec_7b5b6f54","line":175,"updated":"2016-07-01 00:55:52.000000000","message":"nit: This comment and the ones on L135,L179 are pretty useless, IMHO","commit_id":"913dcd77505e5c0fa039334db57c6d5cfe17c463"}],"nova/console/websocketproxy.py":[{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"0a393e57c9cf232dbc57bf59058155a2b24999bd","unresolved":false,"context_lines":[{"line_number":134,"context_line":"                try:"},{"line_number":135,"context_line":"                    tsock \u003d sec_proxy.connect()"},{"line_number":136,"context_line":"                except exception.RFBSecurityNegotiationFailed:"},{"line_number":137,"context_line":"                    LOG.exception(_(\"Unable to perform security proxying, \""},{"line_number":138,"context_line":"                                    \"shutting down connection\"))"},{"line_number":139,"context_line":"                    raise"},{"line_number":140,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_30faa3bd","line":137,"range":{"start_line":137,"start_character":34,"end_line":137,"end_character":36},"updated":"2014-08-20 10:11:42.000000000","message":"Please use here `_LE`","commit_id":"d15a69d5e84cb8c03902c44b64d847de8e11e1ec"},{"author":{"_account_id":7677,"name":"Solly Ross","email":"sross@redhat.com","username":"sross"},"change_message_id":"7c66d8d9a71d47627f772a873899401741671d83","unresolved":false,"context_lines":[{"line_number":134,"context_line":"                try:"},{"line_number":135,"context_line":"                    tsock \u003d sec_proxy.connect()"},{"line_number":136,"context_line":"                except exception.RFBSecurityNegotiationFailed:"},{"line_number":137,"context_line":"                    LOG.exception(_(\"Unable to perform security proxying, \""},{"line_number":138,"context_line":"                                    \"shutting down connection\"))"},{"line_number":139,"context_line":"                    raise"},{"line_number":140,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_e2a93391","line":137,"in_reply_to":"1abeadc6_30faa3bd","updated":"2014-08-20 17:09:04.000000000","message":"Done","commit_id":"d15a69d5e84cb8c03902c44b64d847de8e11e1ec"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"c71e153a4550e0fd8717b07bfbabf41d8cd689c0","unresolved":false,"context_lines":[{"line_number":40,"context_line":"    cfg.StrOpt(\u0027console_security_proxy_driver\u0027,"},{"line_number":41,"context_line":"               default\u003dNone,"},{"line_number":42,"context_line":"               help\u003d(\"The VNC/Spice security proxy driver to use\""},{"line_number":43,"context_line":"                     \"to proxy security at the websocket proxy\"))"},{"line_number":44,"context_line":"]"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a890539_13c5e73c","line":43,"updated":"2014-11-20 14:24:34.000000000","message":"This is bad design IMHO. For a start it is misleading because when you configure it to point to TLSSecurityProxy the admin has the impression they\u0027ll be getting TLS for SPICE and VNC, whereas in fact they\u0027ll only get it for VNC because the SPICE impl is stubbed out to nothing.\n\nI really think we need to have separate options here for each,under the respective group  too. eg\n\n[vnc]\nsecurity_proxy\u003dRFBSecrityProxy|VeNCryptSecurityProxy\n\n[spice]\nsecurity_proxy\u003d\u003csome future impl\u003e\n\n\nThis makes it absolutely clear what protocol is getting what security settings, and doesn\u0027t needlessly force us into a setup where all protocols must be configured the same way.","commit_id":"f5698202efb8435e4dd94abab9a1e7db35bdd0d6"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"4912654f1937343de0f6677fb04e60b4704f4f04","unresolved":false,"context_lines":[{"line_number":140,"context_line":"        sec_proxy \u003d console_sec_proxy.WebSocketSecurityProxy("},{"line_number":141,"context_line":"                self.server.traffic_type, fake_sock, tsock)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"        if sec_proxy.traffic_supported:"},{"line_number":144,"context_line":"            try:"},{"line_number":145,"context_line":"                tsock \u003d sec_proxy.connect()"},{"line_number":146,"context_line":"            except exception.RFBSecurityNegotiationFailed:"}],"source_content_type":"text/x-python","patch_set":12,"id":"5a890539_1f78f729","line":143,"updated":"2014-12-04 11:05:02.000000000","message":"This can be changed to\n\n  if self.server.security_proxy is not None:\n     security_proxy \u003d importutils.import_class(self.server.security_proxy, fake_sock, tsock)","commit_id":"5f702f3a49db7ab71cf5ffcf6d759bcebc8dfe94"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"4912654f1937343de0f6677fb04e60b4704f4f04","unresolved":false,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":"class NovaWebSocketProxy(websockify.WebSocketProxy):"},{"line_number":182,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":183,"context_line":"        self.traffic_type \u003d kwargs.pop(\u0027traffic_type\u0027, None)"},{"line_number":184,"context_line":"        super(NovaWebSocketProxy, self).__init__(*args, **kwargs)"},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":12,"id":"5a890539_bfa8c38c","line":183,"updated":"2014-12-04 11:05:02.000000000","message":"This can be  replaced by\n\n  security_proxy \u003d kwargs.pop(\u0027security_proxy\u0027, None)","commit_id":"5f702f3a49db7ab71cf5ffcf6d759bcebc8dfe94"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"601e45a28a1c251ce930ece062ed6ab580396582","unresolved":false,"context_lines":[{"line_number":137,"context_line":"                    break"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"        if self.server.security_driver is not None:"},{"line_number":140,"context_line":"            fake_sock \u003d FakeSock(self)"},{"line_number":141,"context_line":"            sec_proxy \u003d importutils.import_class(self.server.security_driver,"},{"line_number":142,"context_line":"                                                 fake_sock, tsock)"},{"line_number":143,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"3a961159_5a0bb023","line":140,"updated":"2015-01-06 14:16:42.000000000","message":"Does not appear to be closed unless there is an exception due to security negotiation failure.","commit_id":"e19b4318d82a0912dd9b35714d9c7dcf4f9597af"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"601e45a28a1c251ce930ece062ed6ab580396582","unresolved":false,"context_lines":[{"line_number":139,"context_line":"        if self.server.security_driver is not None:"},{"line_number":140,"context_line":"            fake_sock \u003d FakeSock(self)"},{"line_number":141,"context_line":"            sec_proxy \u003d importutils.import_class(self.server.security_driver,"},{"line_number":142,"context_line":"                                                 fake_sock, tsock)"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"            try:"},{"line_number":145,"context_line":"                tsock \u003d sec_proxy.connect()"}],"source_content_type":"text/x-python","patch_set":18,"id":"3a961159_7f25bec0","line":142,"updated":"2015-01-06 14:16:42.000000000","message":"Looks like this tsock isn\u0027t closed (including by the sec_proxy), which would be a resource leak. Am I missing something?","commit_id":"e19b4318d82a0912dd9b35714d9c7dcf4f9597af"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"24625d29ceab52ff5a8ea82de6de76627462ece2","unresolved":false,"context_lines":[{"line_number":121,"context_line":""},{"line_number":122,"context_line":"        # Connect to the target"},{"line_number":123,"context_line":"        self.msg(_(\"connecting to: %(host)s:%(port)s\") % {\u0027host\u0027: host,"},{"line_number":124,"context_line":"                                                          \u0027port\u0027: port})"},{"line_number":125,"context_line":"        tsock \u003d self.socket(host, port, connect\u003dTrue)"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        # Handshake as necessary"}],"source_content_type":"text/x-python","patch_set":20,"id":"3a961159_e51811cf","line":124,"updated":"2015-01-07 22:09:53.000000000","message":"Where is this socket closed?","commit_id":"eaf63d864018f11083ee41ee7c19ba87e5c8b787"},{"author":{"_account_id":7677,"name":"Solly Ross","email":"sross@redhat.com","username":"sross"},"change_message_id":"eb4b3a663df364b84ff1c3dfe074d2bd0cc6625f","unresolved":false,"context_lines":[{"line_number":121,"context_line":""},{"line_number":122,"context_line":"        # Connect to the target"},{"line_number":123,"context_line":"        self.msg(_(\"connecting to: %(host)s:%(port)s\") % {\u0027host\u0027: host,"},{"line_number":124,"context_line":"                                                          \u0027port\u0027: port})"},{"line_number":125,"context_line":"        tsock \u003d self.socket(host, port, connect\u003dTrue)"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        # Handshake as necessary"}],"source_content_type":"text/x-python","patch_set":20,"id":"3a961159_07f98c7f","line":124,"in_reply_to":"3a961159_e51811cf","updated":"2015-01-08 15:34:50.000000000","message":"see comment below.","commit_id":"eaf63d864018f11083ee41ee7c19ba87e5c8b787"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"24625d29ceab52ff5a8ea82de6de76627462ece2","unresolved":false,"context_lines":[{"line_number":137,"context_line":"                    break"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"        if self.server.security_driver is not None:"},{"line_number":140,"context_line":"            fake_sock \u003d FakeSock(self)"},{"line_number":141,"context_line":"            sec_proxy \u003d importutils.import_class(self.server.security_driver,"},{"line_number":142,"context_line":"                                                 fake_sock, tsock)"},{"line_number":143,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"3a961159_650d018f","line":140,"updated":"2015-01-07 22:09:53.000000000","message":"Who is responsible for closing this socket? It looks like it remains open *unless* there\u0027s a security negotiation failure.","commit_id":"eaf63d864018f11083ee41ee7c19ba87e5c8b787"},{"author":{"_account_id":7677,"name":"Solly Ross","email":"sross@redhat.com","username":"sross"},"change_message_id":"67356973edb8c67d3cc251fa496d7624b8efea1b","unresolved":false,"context_lines":[{"line_number":137,"context_line":"                    break"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"        if self.server.security_driver is not None:"},{"line_number":140,"context_line":"            fake_sock \u003d FakeSock(self)"},{"line_number":141,"context_line":"            sec_proxy \u003d importutils.import_class(self.server.security_driver,"},{"line_number":142,"context_line":"                                                 fake_sock, tsock)"},{"line_number":143,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"3a961159_fbe07b28","line":140,"in_reply_to":"3a961159_3b012305","updated":"2015-01-09 16:26:41.000000000","message":"Only a no-op sec proxy would return the origin tsock.  For instance, the VeNCrypt sec proxy in the next commit returns the result of `ssl.wrap_socket` (which is not the original socket).","commit_id":"eaf63d864018f11083ee41ee7c19ba87e5c8b787"},{"author":{"_account_id":7677,"name":"Solly Ross","email":"sross@redhat.com","username":"sross"},"change_message_id":"eb4b3a663df364b84ff1c3dfe074d2bd0cc6625f","unresolved":false,"context_lines":[{"line_number":137,"context_line":"                    break"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"        if self.server.security_driver is not None:"},{"line_number":140,"context_line":"            fake_sock \u003d FakeSock(self)"},{"line_number":141,"context_line":"            sec_proxy \u003d importutils.import_class(self.server.security_driver,"},{"line_number":142,"context_line":"                                                 fake_sock, tsock)"},{"line_number":143,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"3a961159_a708b891","line":140,"in_reply_to":"3a961159_650d018f","updated":"2015-01-08 15:34:50.000000000","message":"This isn\u0027t an actual socket, so we don\u0027t need to close it.  The underlying socket gets closed below in the exception handler (under `except Exception` by websockify raising the `CClose` exception), as before.","commit_id":"eaf63d864018f11083ee41ee7c19ba87e5c8b787"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"0c16fefa5c8f75a01fdb546c129d0127781695be","unresolved":false,"context_lines":[{"line_number":137,"context_line":"                    break"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"        if self.server.security_driver is not None:"},{"line_number":140,"context_line":"            fake_sock \u003d FakeSock(self)"},{"line_number":141,"context_line":"            sec_proxy \u003d importutils.import_class(self.server.security_driver,"},{"line_number":142,"context_line":"                                                 fake_sock, tsock)"},{"line_number":143,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"3a961159_3b012305","line":140,"in_reply_to":"3a961159_a708b891","updated":"2015-01-09 16:21:36.000000000","message":"Correct me if I\u0027m wrong, but sec_proxy.connect() returns the original tsock so the line in the try block below isn\u0027t strictly necessary?","commit_id":"eaf63d864018f11083ee41ee7c19ba87e5c8b787"},{"author":{"_account_id":19452,"name":"Danielle Carmon","email":"dcarmon@us.ibm.com"},"change_message_id":"64f11d3cde6249effd6de0ff1a868f71c4dc3011","unresolved":false,"context_lines":[{"line_number":197,"context_line":""},{"line_number":198,"context_line":"        if self.server.security_driver is not None:"},{"line_number":199,"context_line":"            fake_sock \u003d FakeSock(self)"},{"line_number":200,"context_line":"            sec_proxy \u003d importutils.import_class(self.server.security_driver,"},{"line_number":201,"context_line":"                                                 fake_sock, tsock)"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":28,"id":"ba8a016a_d671a645","line":200,"updated":"2015-11-23 22:09:35.000000000","message":"This should be \"importutils.import_object\", not \"importutils.import_class\".","commit_id":"6f56e9cf52cf5274a896e44394061ed6ea29aeff"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7414c527301b053695918161c0235c492f391a0b","unresolved":false,"context_lines":[{"line_number":191,"context_line":"        if self.server.security_driver is not None:"},{"line_number":192,"context_line":"            fake_sock \u003d FakeSock(self)"},{"line_number":193,"context_line":"            sec_proxy \u003d importutils.import_object(self.server.security_driver,"},{"line_number":194,"context_line":"                                                  fake_sock, tsock)"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"            try:"},{"line_number":197,"context_line":"                tsock \u003d sec_proxy.connect()"}],"source_content_type":"text/x-python","patch_set":42,"id":"3aaa91ec_3b13a766","line":194,"updated":"2016-07-01 00:55:52.000000000","message":"I don\u0027t think we should add another arbitrary class loader.","commit_id":"913dcd77505e5c0fa039334db57c6d5cfe17c463"}],"nova/exception.py":[{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"e47e631e84856d7ce04d91c2d0e50e33ea1bd644","unresolved":false,"context_lines":[{"line_number":1806,"context_line":"class InstanceQuiesceNotSupported(Invalid):"},{"line_number":1807,"context_line":"    msg_fmt \u003d _(\u0027Quiescing is not supported in instance %(instance_id)s: \u0027"},{"line_number":1808,"context_line":"                \u0027%(reason)s\u0027)"},{"line_number":1809,"context_line":""},{"line_number":1810,"context_line":"class RFBSecurityNegotiationFailed(NovaException):"},{"line_number":1811,"context_line":"    msg_fmt \u003d _(\"Failed to negotiate security type with server: %(reason)s\")"},{"line_number":1812,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"3a961159_fd900f45","line":1809,"updated":"2014-12-10 09:42:16.000000000","message":"pep8 is complaining as you need 2 blank lines","commit_id":"f1159e8d0b461d17fb10ce2fd824f57ba43f87d6"}],"nova/tests/console/test_security_proxy.py":[{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"0a393e57c9cf232dbc57bf59058155a2b24999bd","unresolved":false,"context_lines":[{"line_number":60,"context_line":"        self.client_sock.recv(12).AndReturn(full_version_str)"},{"line_number":61,"context_line":"        self.server_sock.sendall(full_version_str)"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    def test_fail_37(self):"},{"line_number":64,"context_line":"        self.proxy.rfb_version \u003d 3.7"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        self.client_sock.sendall(\"\\x00\\x00\\x00\\x01\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_90ba9708","line":63,"updated":"2014-08-20 10:11:42.000000000","message":"What the reason use `mox` instead `mock` library?","commit_id":"d15a69d5e84cb8c03902c44b64d847de8e11e1ec"},{"author":{"_account_id":7677,"name":"Solly Ross","email":"sross@redhat.com","username":"sross"},"change_message_id":"7c66d8d9a71d47627f772a873899401741671d83","unresolved":false,"context_lines":[{"line_number":60,"context_line":"        self.client_sock.recv(12).AndReturn(full_version_str)"},{"line_number":61,"context_line":"        self.server_sock.sendall(full_version_str)"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    def test_fail_37(self):"},{"line_number":64,"context_line":"        self.proxy.rfb_version \u003d 3.7"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        self.client_sock.sendall(\"\\x00\\x00\\x00\\x01\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_227ecb19","line":63,"in_reply_to":"1abeadc6_90ba9708","updated":"2014-08-20 17:09:04.000000000","message":"a) I was more familiar with mox, as it\u0027s used heavily in the libvirt driver unit tests\nb) Mox is IMHO, much easier to use in this case, and requires much less code to be written to achieve the desired results","commit_id":"d15a69d5e84cb8c03902c44b64d847de8e11e1ec"},{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"0a393e57c9cf232dbc57bf59058155a2b24999bd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"1abeadc6_90e37732","line":244,"updated":"2014-08-20 10:11:42.000000000","message":"W391 blank line at end of file","commit_id":"d15a69d5e84cb8c03902c44b64d847de8e11e1ec"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"535bb8f37abddb659aa1cfcc0ec91d80ae699aff","unresolved":false,"context_lines":[{"line_number":19,"context_line":"import struct"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"import mock"},{"line_number":22,"context_line":"import mox"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"from nova.console import security_proxy_helper as sec_proxy"},{"line_number":25,"context_line":"from nova import exception"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_5c43496c","line":22,"updated":"2014-11-18 19:27:52.000000000","message":"Echoing the prior comment: mock should be used for new tests instead of mox -- see http://lists.openstack.org/pipermail/openstack-dev/2013-July/012484.html","commit_id":"2196213ba4a6ec6ccfec6b5eba52237a4924404b"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"4203c0eb641ec5715602857758213bf011190b08","unresolved":false,"context_lines":[{"line_number":19,"context_line":"import struct"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"import mock"},{"line_number":22,"context_line":"import mox"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"from nova.console import security_proxy_helper as sec_proxy"},{"line_number":25,"context_line":"from nova import exception"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_2f8d4f7c","line":22,"updated":"2014-11-10 13:08:35.000000000","message":"do we need mox for new tests? i think that you can use mock instead","commit_id":"2196213ba4a6ec6ccfec6b5eba52237a4924404b"},{"author":{"_account_id":7677,"name":"Solly Ross","email":"sross@redhat.com","username":"sross"},"change_message_id":"024929787ac71233166311190d02b29b3261e8f3","unresolved":false,"context_lines":[{"line_number":19,"context_line":"import struct"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"import mock"},{"line_number":22,"context_line":"import mox"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"from nova.console import security_proxy_helper as sec_proxy"},{"line_number":25,"context_line":"from nova import exception"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_828708d7","line":22,"in_reply_to":"5a890539_5c43496c","updated":"2014-11-18 20:14:07.000000000","message":"This code was originally written before that policy was put into place.  I\u0027ll update it (probably the revision after next).","commit_id":"2196213ba4a6ec6ccfec6b5eba52237a4924404b"}],"nova/tests/unit/console/test_security_proxy.py":[{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"db9f4020d24fc980dd94256634e7d628d93d2589","unresolved":false,"context_lines":[{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        self.proxy._fail(\u0027blah\u0027, False)"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        self.assertEqual(self.expected_calls, self.manager.mock_calls)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def test_fail_38(self):"},{"line_number":79,"context_line":"        self.proxy.rfb_version \u003d 3.8"}],"source_content_type":"text/x-python","patch_set":20,"id":"3a961159_1f9c7069","line":76,"updated":"2015-01-08 16:52:31.000000000","message":"might be better as: self.manager.assert_has_calls(self.expected_calls)\n\nditto, for the rest of the cases below.","commit_id":"eaf63d864018f11083ee41ee7c19ba87e5c8b787"}]}