[GitHub] syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM
syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM URL: https://github.com/apache/cloudstack/pull/2204#discussion_r131290782 ## File path: services/console-proxy/server/src/com/cloud/consoleproxy/vnc/VncClient.java ## @@ -16,6 +16,16 @@ // under the License. package com.cloud.consoleproxy.vnc; +import com.cloud.consoleproxy.ConsoleProxyClientListener; Review comment: Nothing changed in this file, just rearranged a bunch of stuff. Can you revert this file back, we don't need it. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM
syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding
Support for NoVNC Console for KVM
URL: https://github.com/apache/cloudstack/pull/2204#discussion_r131290366
##
File path:
services/console-proxy/server/src/com/cloud/consoleproxy/WebSocketHandlerForNovnc.java
##
@@ -0,0 +1,505 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.consoleproxy;
+
+import com.cloud.consoleproxy.vnc.RfbConstants;
+import org.apache.log4j.Logger;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.websocket.api.Session;
+import org.eclipse.jetty.websocket.api.annotations.OnWebSocketConnect;
+import org.eclipse.jetty.websocket.api.annotations.OnWebSocketClose;
+import org.eclipse.jetty.websocket.api.annotations.OnWebSocketFrame;
+import org.eclipse.jetty.websocket.api.annotations.OnWebSocketError;
+import org.eclipse.jetty.websocket.api.annotations.WebSocket;
+import org.eclipse.jetty.websocket.api.extensions.Frame;
+import org.eclipse.jetty.websocket.common.WebSocketSession;
+import org.eclipse.jetty.websocket.server.WebSocketHandler;
+import org.eclipse.jetty.websocket.servlet.WebSocketServletFactory;
+
+import javax.crypto.Cipher;
+import javax.crypto.SecretKey;
+import javax.crypto.SecretKeyFactory;
+import javax.crypto.spec.DESKeySpec;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.net.Socket;
+import java.nio.ByteBuffer;
+import java.security.spec.KeySpec;
+import java.util.Map;
+
+/**
+ * Created by root on 18/6/17.
+ */
+
+@WebSocket
+public class WebSocketHandlerForNovnc extends WebSocketHandler {
+
+
+public static final Logger s_logger =
Logger.getLogger(WebSocketHandlerForNovnc.class.getSimpleName());
+private Socket vncSocket;
+private DataInputStream is;
+private DataOutputStream os;
+private Session session;
+private boolean isConnectionStart = false;
+private int frameCount;
+private String hostPassword;
+private int authType;
+
+private static enum VncState {
+SERVER_VERSION_SENT, AUTH_TYPES_SENT, AUTH_RESULT_SENT, UNKNOWN
+}
+private static final byte[] M_VNC_AUTH_OK = new byte[]{0, 0, 0, 0};
+private static final byte[] M_VNC_AUTH_TYE_NOAUTH = new byte[]{01, 01};
+private VncState clientState;
+
+/**
+ * Reverse bits in byte, so least significant bit will be most significant
+ * bit. E.g. 01001100 will become 00110010.
+ *
+ * See also: http://www.vidarholen.net/contents/junk/vnc.html ,
+ * http://bytecrafter
+ * .blogspot.com/2010/09/des-encryption-as-used-in-vnc.html
+ *
+ * @param b a byte
+ * @return byte in reverse order
+ */
+private static byte flipByte(byte b) {
+int b1_8 = (b & 0x1) << 7;
+int b2_7 = (b & 0x2) << 5;
+int b3_6 = (b & 0x4) << 3;
+int b4_5 = (b & 0x8) << 1;
+int b5_4 = (b & 0x10) >>> 1;
+int b6_3 = (b & 0x20) >>> 3;
+int b7_2 = (b & 0x40) >>> 5;
+int b8_1 = (b & 0x80) >>> 7;
+byte c = (byte) (b1_8 | b2_7 | b3_6 | b4_5 | b5_4 | b6_3 | b7_2 |
b8_1);
+return c;
+}
+
+@Override
+public void configure(WebSocketServletFactory webSocketServletFactory) {
+webSocketServletFactory.register(WebSocketHandlerForNovnc.class);
+}
+
+@Override
+public void handle(String target, Request baseRequest, HttpServletRequest
request, HttpServletResponse response) throws IOException, ServletException {
+if (this.getWebSocketFactory().isUpgradeRequest(request, response)) {
+response.addHeader("Sec-WebSocket-Protocol", "binary");
+if (this.getWebSocketFactory().acceptWebSocket(request, response))
{
+baseRequest.setHandled(true);
+return;
+}
+
+if (response.isCommitted()) {
+return;
+}
+}
+}
+
+@OnWebSocketConnect
+public void onConnect(final Session session) throws IOException,
InterruptedException {
+
+
[GitHub] syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM
syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding
Support for NoVNC Console for KVM
URL: https://github.com/apache/cloudstack/pull/2204#discussion_r131290552
##
File path:
services/console-proxy/server/src/com/cloud/consoleproxy/WebSocketHandlerForNovnc.java
##
@@ -0,0 +1,505 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.consoleproxy;
+
+import com.cloud.consoleproxy.vnc.RfbConstants;
+import org.apache.log4j.Logger;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.websocket.api.Session;
+import org.eclipse.jetty.websocket.api.annotations.OnWebSocketConnect;
+import org.eclipse.jetty.websocket.api.annotations.OnWebSocketClose;
+import org.eclipse.jetty.websocket.api.annotations.OnWebSocketFrame;
+import org.eclipse.jetty.websocket.api.annotations.OnWebSocketError;
+import org.eclipse.jetty.websocket.api.annotations.WebSocket;
+import org.eclipse.jetty.websocket.api.extensions.Frame;
+import org.eclipse.jetty.websocket.common.WebSocketSession;
+import org.eclipse.jetty.websocket.server.WebSocketHandler;
+import org.eclipse.jetty.websocket.servlet.WebSocketServletFactory;
+
+import javax.crypto.Cipher;
+import javax.crypto.SecretKey;
+import javax.crypto.SecretKeyFactory;
+import javax.crypto.spec.DESKeySpec;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.net.Socket;
+import java.nio.ByteBuffer;
+import java.security.spec.KeySpec;
+import java.util.Map;
+
+/**
+ * Created by root on 18/6/17.
+ */
+
+@WebSocket
+public class WebSocketHandlerForNovnc extends WebSocketHandler {
+
+
+public static final Logger s_logger =
Logger.getLogger(WebSocketHandlerForNovnc.class.getSimpleName());
+private Socket vncSocket;
+private DataInputStream is;
+private DataOutputStream os;
+private Session session;
+private boolean isConnectionStart = false;
+private int frameCount;
+private String hostPassword;
+private int authType;
+
+private static enum VncState {
+SERVER_VERSION_SENT, AUTH_TYPES_SENT, AUTH_RESULT_SENT, UNKNOWN
+}
+private static final byte[] M_VNC_AUTH_OK = new byte[]{0, 0, 0, 0};
+private static final byte[] M_VNC_AUTH_TYE_NOAUTH = new byte[]{01, 01};
+private VncState clientState;
+
+/**
+ * Reverse bits in byte, so least significant bit will be most significant
+ * bit. E.g. 01001100 will become 00110010.
+ *
+ * See also: http://www.vidarholen.net/contents/junk/vnc.html ,
+ * http://bytecrafter
+ * .blogspot.com/2010/09/des-encryption-as-used-in-vnc.html
+ *
+ * @param b a byte
+ * @return byte in reverse order
+ */
+private static byte flipByte(byte b) {
+int b1_8 = (b & 0x1) << 7;
+int b2_7 = (b & 0x2) << 5;
+int b3_6 = (b & 0x4) << 3;
+int b4_5 = (b & 0x8) << 1;
+int b5_4 = (b & 0x10) >>> 1;
+int b6_3 = (b & 0x20) >>> 3;
+int b7_2 = (b & 0x40) >>> 5;
+int b8_1 = (b & 0x80) >>> 7;
+byte c = (byte) (b1_8 | b2_7 | b3_6 | b4_5 | b5_4 | b6_3 | b7_2 |
b8_1);
+return c;
+}
+
+@Override
+public void configure(WebSocketServletFactory webSocketServletFactory) {
+webSocketServletFactory.register(WebSocketHandlerForNovnc.class);
+}
+
+@Override
+public void handle(String target, Request baseRequest, HttpServletRequest
request, HttpServletResponse response) throws IOException, ServletException {
+if (this.getWebSocketFactory().isUpgradeRequest(request, response)) {
+response.addHeader("Sec-WebSocket-Protocol", "binary");
+if (this.getWebSocketFactory().acceptWebSocket(request, response))
{
+baseRequest.setHandled(true);
+return;
+}
+
+if (response.isCommitted()) {
+return;
+}
+}
+}
+
+@OnWebSocketConnect
+public void onConnect(final Session session) throws IOException,
InterruptedException {
+
+
[GitHub] syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM
syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding
Support for NoVNC Console for KVM
URL: https://github.com/apache/cloudstack/pull/2204#discussion_r131289911
##
File path:
services/console-proxy/server/src/com/cloud/consoleproxy/ConsoleProxyBaseServerFactoryImpl.java
##
@@ -33,10 +32,50 @@ public void init(byte[] ksBits, String ksPassword) {
}
@Override
-public HttpServer createHttpServerInstance(int port) throws IOException {
-if (s_logger.isInfoEnabled())
-s_logger.info("create HTTP server instance at port: " + port);
-return HttpServer.create(new InetSocketAddress(port), 5);
+public Server createHttpServerInstance(int port) throws IOException {
+Server webServer = new Server(port);
+Handler[] handlerList = new Handler[6];
+
+// new context handler for get screen
+ContextHandler contextHandlerForScreen = new ContextHandler();
Review comment:
Move all the `ContextHandlers` to `startupHttpMain` function
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM
syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding
Support for NoVNC Console for KVM
URL: https://github.com/apache/cloudstack/pull/2204#discussion_r131290482
##
File path:
services/console-proxy/server/src/com/cloud/consoleproxy/WebSocketHandlerForNovnc.java
##
@@ -0,0 +1,505 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.consoleproxy;
+
+import com.cloud.consoleproxy.vnc.RfbConstants;
+import org.apache.log4j.Logger;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.websocket.api.Session;
+import org.eclipse.jetty.websocket.api.annotations.OnWebSocketConnect;
+import org.eclipse.jetty.websocket.api.annotations.OnWebSocketClose;
+import org.eclipse.jetty.websocket.api.annotations.OnWebSocketFrame;
+import org.eclipse.jetty.websocket.api.annotations.OnWebSocketError;
+import org.eclipse.jetty.websocket.api.annotations.WebSocket;
+import org.eclipse.jetty.websocket.api.extensions.Frame;
+import org.eclipse.jetty.websocket.common.WebSocketSession;
+import org.eclipse.jetty.websocket.server.WebSocketHandler;
+import org.eclipse.jetty.websocket.servlet.WebSocketServletFactory;
+
+import javax.crypto.Cipher;
+import javax.crypto.SecretKey;
+import javax.crypto.SecretKeyFactory;
+import javax.crypto.spec.DESKeySpec;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.net.Socket;
+import java.nio.ByteBuffer;
+import java.security.spec.KeySpec;
+import java.util.Map;
+
+/**
+ * Created by root on 18/6/17.
+ */
+
+@WebSocket
+public class WebSocketHandlerForNovnc extends WebSocketHandler {
+
+
+public static final Logger s_logger =
Logger.getLogger(WebSocketHandlerForNovnc.class.getSimpleName());
+private Socket vncSocket;
+private DataInputStream is;
+private DataOutputStream os;
+private Session session;
+private boolean isConnectionStart = false;
+private int frameCount;
+private String hostPassword;
+private int authType;
+
+private static enum VncState {
+SERVER_VERSION_SENT, AUTH_TYPES_SENT, AUTH_RESULT_SENT, UNKNOWN
+}
+private static final byte[] M_VNC_AUTH_OK = new byte[]{0, 0, 0, 0};
+private static final byte[] M_VNC_AUTH_TYE_NOAUTH = new byte[]{01, 01};
+private VncState clientState;
+
+/**
+ * Reverse bits in byte, so least significant bit will be most significant
+ * bit. E.g. 01001100 will become 00110010.
+ *
+ * See also: http://www.vidarholen.net/contents/junk/vnc.html ,
+ * http://bytecrafter
+ * .blogspot.com/2010/09/des-encryption-as-used-in-vnc.html
+ *
+ * @param b a byte
+ * @return byte in reverse order
+ */
+private static byte flipByte(byte b) {
+int b1_8 = (b & 0x1) << 7;
+int b2_7 = (b & 0x2) << 5;
+int b3_6 = (b & 0x4) << 3;
+int b4_5 = (b & 0x8) << 1;
+int b5_4 = (b & 0x10) >>> 1;
+int b6_3 = (b & 0x20) >>> 3;
+int b7_2 = (b & 0x40) >>> 5;
+int b8_1 = (b & 0x80) >>> 7;
+byte c = (byte) (b1_8 | b2_7 | b3_6 | b4_5 | b5_4 | b6_3 | b7_2 |
b8_1);
+return c;
+}
+
+@Override
+public void configure(WebSocketServletFactory webSocketServletFactory) {
+webSocketServletFactory.register(WebSocketHandlerForNovnc.class);
+}
+
+@Override
+public void handle(String target, Request baseRequest, HttpServletRequest
request, HttpServletResponse response) throws IOException, ServletException {
+if (this.getWebSocketFactory().isUpgradeRequest(request, response)) {
+response.addHeader("Sec-WebSocket-Protocol", "binary");
+if (this.getWebSocketFactory().acceptWebSocket(request, response))
{
+baseRequest.setHandled(true);
+return;
+}
+
+if (response.isCommitted()) {
+return;
+}
+}
+}
+
+@OnWebSocketConnect
+public void onConnect(final Session session) throws IOException,
InterruptedException {
+
+
[GitHub] syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM
syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM URL: https://github.com/apache/cloudstack/pull/2204#discussion_r131290300 ## File path: services/console-proxy/server/src/com/cloud/consoleproxy/WebSocketHandlerForNovnc.java ## @@ -0,0 +1,505 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.consoleproxy; + +import com.cloud.consoleproxy.vnc.RfbConstants; +import org.apache.log4j.Logger; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.websocket.api.Session; +import org.eclipse.jetty.websocket.api.annotations.OnWebSocketConnect; +import org.eclipse.jetty.websocket.api.annotations.OnWebSocketClose; +import org.eclipse.jetty.websocket.api.annotations.OnWebSocketFrame; +import org.eclipse.jetty.websocket.api.annotations.OnWebSocketError; +import org.eclipse.jetty.websocket.api.annotations.WebSocket; +import org.eclipse.jetty.websocket.api.extensions.Frame; +import org.eclipse.jetty.websocket.common.WebSocketSession; +import org.eclipse.jetty.websocket.server.WebSocketHandler; +import org.eclipse.jetty.websocket.servlet.WebSocketServletFactory; + +import javax.crypto.Cipher; +import javax.crypto.SecretKey; +import javax.crypto.SecretKeyFactory; +import javax.crypto.spec.DESKeySpec; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.DataInputStream; +import java.io.DataOutputStream; +import java.io.IOException; +import java.net.Socket; +import java.nio.ByteBuffer; +import java.security.spec.KeySpec; +import java.util.Map; + +/** Review comment: Remove all the comments that say "created by root" This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM
syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding
Support for NoVNC Console for KVM
URL: https://github.com/apache/cloudstack/pull/2204#discussion_r131290155
##
File path:
services/console-proxy/server/src/com/cloud/consoleproxy/ConsoleProxySecureServerFactoryImpl.java
##
@@ -76,34 +86,83 @@ public void init(byte[] ksBits, String ksPassword) {
}
@Override
-public HttpServer createHttpServerInstance(int port) throws IOException {
-try {
-HttpsServer server = HttpsServer.create(new
InetSocketAddress(port), 5);
-server.setHttpsConfigurator(new HttpsConfigurator(sslContext) {
-@Override
-public void configure(HttpsParameters params) {
-
-// get the remote address if needed
-InetSocketAddress remote = params.getClientAddress();
-SSLContext c = getSSLContext();
-
-// get the default parameters
-SSLParameters sslparams = c.getDefaultSSLParameters();
-
-params.setSSLParameters(sslparams);
-params.setProtocols(SSLUtils.getRecommendedProtocols());
-params.setCipherSuites(SSLUtils.getRecommendedCiphers());
-// statement above could throw IAE if any params invalid.
-// eg. if app has a UI and parameters supplied by a user.
-}
-});
-
-s_logger.info("create HTTPS server instance on port: " + port);
-return server;
-} catch (Exception ioe) {
-s_logger.error(ioe.toString(), ioe);
-}
-return null;
+public Server createHttpServerInstance(int port) throws IOException {
+Server server = getServerWithContext(port);
+
+/**
+ * add all context here
+ **/
+
+// HTTP Configuration
+HttpConfiguration http = new HttpConfiguration();
+http.addCustomizer(new SecureRequestCustomizer());
+
+// Configuration for HTTPS redirect
+http.setSecurePort(port);
+http.setSecureScheme("https");
+ServerConnector connector = new ServerConnector(server);
+connector.addConnectionFactory(new HttpConnectionFactory(http));
+// Setting HTTP port
+connector.setPort(80);
+
+// HTTPS configuration
+HttpConfiguration https = new HttpConfiguration();
+https.addCustomizer(new SecureRequestCustomizer());
+
+// Configuring the connector
+ServerConnector sslConnector = new ServerConnector(server,
+new SslConnectionFactory(sslContextFactory, "http/1.1"), new
HttpConnectionFactory(https));
+sslConnector.setPort(port);
+
+// Setting HTTP and HTTPS connectors
+server.setConnectors(new Connector[]{connector, sslConnector});
+return server;
+}
+
+private Server getServerWithContext(int port) {
Review comment:
Same here, the context will be set in `startHttpMain` for both HTTP and SSL
connections
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM
syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding
Support for NoVNC Console for KVM
URL: https://github.com/apache/cloudstack/pull/2204#discussion_r131290099
##
File path:
services/console-proxy/server/src/com/cloud/consoleproxy/ConsoleProxySecureServerFactoryImpl.java
##
@@ -76,34 +86,83 @@ public void init(byte[] ksBits, String ksPassword) {
}
@Override
-public HttpServer createHttpServerInstance(int port) throws IOException {
-try {
-HttpsServer server = HttpsServer.create(new
InetSocketAddress(port), 5);
-server.setHttpsConfigurator(new HttpsConfigurator(sslContext) {
-@Override
-public void configure(HttpsParameters params) {
-
-// get the remote address if needed
-InetSocketAddress remote = params.getClientAddress();
-SSLContext c = getSSLContext();
-
-// get the default parameters
-SSLParameters sslparams = c.getDefaultSSLParameters();
-
-params.setSSLParameters(sslparams);
-params.setProtocols(SSLUtils.getRecommendedProtocols());
-params.setCipherSuites(SSLUtils.getRecommendedCiphers());
-// statement above could throw IAE if any params invalid.
-// eg. if app has a UI and parameters supplied by a user.
-}
-});
-
-s_logger.info("create HTTPS server instance on port: " + port);
-return server;
-} catch (Exception ioe) {
-s_logger.error(ioe.toString(), ioe);
-}
-return null;
+public Server createHttpServerInstance(int port) throws IOException {
+Server server = getServerWithContext(port);
+
+/**
+ * add all context here
+ **/
+
+// HTTP Configuration
+HttpConfiguration http = new HttpConfiguration();
+http.addCustomizer(new SecureRequestCustomizer());
+
+// Configuration for HTTPS redirect
+http.setSecurePort(port);
+http.setSecureScheme("https");
+ServerConnector connector = new ServerConnector(server);
+connector.addConnectionFactory(new HttpConnectionFactory(http));
+// Setting HTTP port
+connector.setPort(80);
Review comment:
hardcoded value. Can you move this to a constant or get it some other way?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM
syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding
Support for NoVNC Console for KVM
URL: https://github.com/apache/cloudstack/pull/2204#discussion_r130695076
##
File path:
services/console-proxy/server/src/com/cloud/consoleproxy/vnc/VncClientPacketSender.java
##
@@ -99,7 +99,7 @@ public void closeConnection() {
connectionAlive = false;
}
-public void requestFullScreenUpdate() {
+public void requestFullScreenUpdate() {
Review comment:
can you revert the extra spaces here?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM
syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding
Support for NoVNC Console for KVM
URL: https://github.com/apache/cloudstack/pull/2204#discussion_r130692538
##
File path:
services/console-proxy/server/src/com/cloud/consoleproxy/ConsoleProxy.java
##
@@ -328,7 +329,7 @@ public static void start(Properties conf) {
}
if (httpListenPort != 0) {
-startupHttpMain();
+startServerForNoVNC();
Review comment:
Can you keep the `startupHttpMain` function and move the logic of
`startServerForNoVNC` into `startupHttpMain` since we are no longer just doing
VNC, we are also doing ajax through the same jetty service
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM
syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM URL: https://github.com/apache/cloudstack/pull/2204#discussion_r130695220 ## File path: systemvm/systemvm-descriptor.xml ## @@ -16,7 +16,7 @@ specific language governing permissions and limitations under the License. --> -http://maven.apache.org/plugins/maven-assembly-plugin/assembly/1.1.2; +http://maven.apache.org/lugins/maven-assembly-plugin/assembly/1.1.2; Review comment: can you fix the typo here? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM
syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding
Support for NoVNC Console for KVM
URL: https://github.com/apache/cloudstack/pull/2204#discussion_r130693648
##
File path:
services/console-proxy/server/src/com/cloud/consoleproxy/ConsoleProxyAjaxHandler.java
##
@@ -20,179 +20,26 @@
import java.io.BufferedReader;
import java.io.IOException;
-import java.io.InputStream;
-import java.io.InputStreamReader;
import java.io.OutputStream;
import java.net.URLDecoder;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
-
-import com.sun.net.httpserver.Headers;
-import com.sun.net.httpserver.HttpExchange;
-import com.sun.net.httpserver.HttpHandler;
-
import com.cloud.consoleproxy.util.Logger;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.handler.AbstractHandler;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
-public class ConsoleProxyAjaxHandler implements HttpHandler {
+public class ConsoleProxyAjaxHandler extends AbstractHandler {
private static final Logger s_logger =
Logger.getLogger(ConsoleProxyAjaxHandler.class);
public ConsoleProxyAjaxHandler() {
}
-@Override
-public void handle(HttpExchange t) throws IOException {
Review comment:
It seems that the functions `handle()` and `dohandle()` are removed here and
added down again, technically, not a lot changed, can you make sure that the
diff is more accurate?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM
syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM URL: https://github.com/apache/cloudstack/pull/2204#discussion_r130691942 ## File path: server/src/com/cloud/servlet/ConsoleProxyServlet.java ## @@ -16,35 +16,6 @@ // under the License. package com.cloud.servlet; -import java.io.IOException; Review comment: There seem to be a lot of unnecessary changes in this file. Can you please check this and make sure you only include the changes that are needed This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM
syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding
Support for NoVNC Console for KVM
URL: https://github.com/apache/cloudstack/pull/2204#discussion_r130694606
##
File path:
services/console-proxy/server/src/com/cloud/consoleproxy/ConsoleProxySecureServerFactoryImpl.java
##
@@ -105,6 +122,87 @@ public void configure(HttpsParameters params) {
}
@Override
+public Server createConsoleProxyServer(int port) throws IOException {
+
+Server server = getServerWithContext(port);
+
+/**
+ * add all context here
+ **/
+
+// HTTP Configuration
+HttpConfiguration http = new HttpConfiguration();
+http.addCustomizer(new SecureRequestCustomizer());
+
+// Configuration for HTTPS redirect
+http.setSecurePort(443);
Review comment:
Make sure you move any hard coded configuration to a static final config
variable
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM
syed commented on a change in pull request #2204: [CLOUDSTACK-10025] Adding
Support for NoVNC Console for KVM
URL: https://github.com/apache/cloudstack/pull/2204#discussion_r130693009
##
File path:
services/console-proxy/server/src/com/cloud/consoleproxy/ConsoleProxy.java
##
@@ -379,7 +359,23 @@ private static void startupHttpCmdPort() {
}
}
-public static void main(String[] argv) {
+private static void startServerForNoVNC() {
+try {
+ConsoleProxyServerFactory factory = getHttpServerFactory();
+if (factory == null) {
+s_logger.error("Unable to load HTTP server factory");
+System.exit(1);
+}
+Server webServer = factory.
+createConsoleProxyServer(443);
Review comment:
You don't need the function `createConsoleProxyServer` re-use the function
`factory.createHttpServerInstance`
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
