tomaswolf commented on code in PR #716:
URL: https://github.com/apache/mina-sshd/pull/716#discussion_r2022243574


##########
sshd-core/src/main/java/org/apache/sshd/client/channel/ChannelX11.java:
##########
@@ -0,0 +1,166 @@
+/*
+ * 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 org.apache.sshd.client.channel;
+
+import org.apache.sshd.client.future.DefaultOpenFuture;
+import org.apache.sshd.client.future.OpenFuture;
+import org.apache.sshd.client.session.ClientConnectionService;
+import org.apache.sshd.client.x11.X11IoHandler;
+import org.apache.sshd.common.Property;
+import org.apache.sshd.common.SshConstants;
+import org.apache.sshd.common.channel.ChannelOutputStream;
+import org.apache.sshd.common.io.IoConnectFuture;
+import org.apache.sshd.common.io.IoConnector;
+import org.apache.sshd.common.io.IoHandler;
+import org.apache.sshd.common.io.IoSession;
+import org.apache.sshd.common.util.buffer.Buffer;
+import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+/**
+ * @author <a href="mailto:dev@mina.apache.org";>Apache MINA SSHD Project</a>
+ */
+public class ChannelX11 extends AbstractClientChannel {
+
+    public static final Property<Object> X11_COOKIE = 
Property.object("x11-cookie");
+    public static final Property<Object> X11_COOKIE_HEX = 
Property.object("x11-cookie-hex");
+
+    private final AtomicBoolean isInitialized = new AtomicBoolean(false);
+    private final String host;
+    private final int port;
+
+    private IoSession x11;
+
+    public ChannelX11(String host, int port) {
+        super("x11");
+        this.host = host;
+        this.port = port;
+    }
+
+    @Override
+    public OpenFuture open(long recipient, long rwSize, long packetSize, 
Buffer buffer) {
+        final DefaultOpenFuture defaultOpenFuture = new 
DefaultOpenFuture(this, futureLock);
+        super.openFuture = defaultOpenFuture;
+
+        final IoConnector connector
+                = 
getSession().getFactoryManager().getIoServiceFactory().createConnector(createX11IoHandler());
+        addCloseFutureListener(future -> {
+            if (future.isClosed()) {
+                connector.close(true);
+            }
+        });
+
+        final IoConnectFuture connectFuture = connector.connect(new 
InetSocketAddress(host, port), this, null);
+        connectFuture.addListener(future -> {
+            if (future.isConnected()) {
+                x11 = future.getSession();
+                handleOpenSuccess(recipient, rwSize, packetSize, buffer);
+            } else {
+                if (future.getException() != null) {
+                    defaultOpenFuture.setException(future.getException());
+                } else {
+                    defaultOpenFuture.setValue(false);
+                }
+                unregisterSelf();
+            }
+        });
+
+        return defaultOpenFuture;
+
+    }
+
+    @Override
+    protected void doOpen() throws IOException {
+        setOut(new ChannelOutputStream(this, getRemoteWindow(), log, 
SshConstants.SSH_MSG_CHANNEL_DATA, true));
+    }
+
+    @Override
+    protected void doWriteData(byte[] data, int off, long len) throws 
IOException {
+        if (isInitialized.compareAndSet(false, true)) {
+            final byte[] xCookie = getXCookie();
+            if (xCookie == null) {
+                sendEof();
+                return;
+            }
+
+            int l = (int) len;
+            int s = 0;
+            byte[] foo = new byte[l];
+            System.arraycopy(data, off, foo, 0, l);
+
+            if (l < 9) {
+                return;
+            }
+
+            int plen = (foo[s + 6] & 0xff) * 256 + (foo[s + 7] & 0xff);
+            int dlen = (foo[s + 8] & 0xff) * 256 + (foo[s + 9] & 0xff);
+
+            if ((foo[s] & 0xff) == 0x6c) {

Review Comment:
   0x6c is a lower-case L (`'l'`). This comes from the [X11 
protocol](https://www.x.org/releases/X11R7.7/doc/xproto/x11protocol.html#Connection_Setup).
 De-obfuscate by actually writing `'l'` here.



##########
sshd-core/src/main/java/org/apache/sshd/client/channel/ChannelX11.java:
##########
@@ -0,0 +1,166 @@
+/*
+ * 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 org.apache.sshd.client.channel;
+
+import org.apache.sshd.client.future.DefaultOpenFuture;
+import org.apache.sshd.client.future.OpenFuture;
+import org.apache.sshd.client.session.ClientConnectionService;
+import org.apache.sshd.client.x11.X11IoHandler;
+import org.apache.sshd.common.Property;
+import org.apache.sshd.common.SshConstants;
+import org.apache.sshd.common.channel.ChannelOutputStream;
+import org.apache.sshd.common.io.IoConnectFuture;
+import org.apache.sshd.common.io.IoConnector;
+import org.apache.sshd.common.io.IoHandler;
+import org.apache.sshd.common.io.IoSession;
+import org.apache.sshd.common.util.buffer.Buffer;
+import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+/**
+ * @author <a href="mailto:dev@mina.apache.org";>Apache MINA SSHD Project</a>
+ */
+public class ChannelX11 extends AbstractClientChannel {
+
+    public static final Property<Object> X11_COOKIE = 
Property.object("x11-cookie");
+    public static final Property<Object> X11_COOKIE_HEX = 
Property.object("x11-cookie-hex");
+
+    private final AtomicBoolean isInitialized = new AtomicBoolean(false);
+    private final String host;
+    private final int port;
+
+    private IoSession x11;
+
+    public ChannelX11(String host, int port) {
+        super("x11");
+        this.host = host;
+        this.port = port;
+    }
+
+    @Override
+    public OpenFuture open(long recipient, long rwSize, long packetSize, 
Buffer buffer) {
+        final DefaultOpenFuture defaultOpenFuture = new 
DefaultOpenFuture(this, futureLock);
+        super.openFuture = defaultOpenFuture;
+
+        final IoConnector connector
+                = 
getSession().getFactoryManager().getIoServiceFactory().createConnector(createX11IoHandler());
+        addCloseFutureListener(future -> {
+            if (future.isClosed()) {
+                connector.close(true);
+            }
+        });
+
+        final IoConnectFuture connectFuture = connector.connect(new 
InetSocketAddress(host, port), this, null);
+        connectFuture.addListener(future -> {
+            if (future.isConnected()) {
+                x11 = future.getSession();
+                handleOpenSuccess(recipient, rwSize, packetSize, buffer);
+            } else {
+                if (future.getException() != null) {
+                    defaultOpenFuture.setException(future.getException());
+                } else {
+                    defaultOpenFuture.setValue(false);
+                }
+                unregisterSelf();
+            }
+        });
+
+        return defaultOpenFuture;
+
+    }
+
+    @Override
+    protected void doOpen() throws IOException {
+        setOut(new ChannelOutputStream(this, getRemoteWindow(), log, 
SshConstants.SSH_MSG_CHANNEL_DATA, true));
+    }
+
+    @Override
+    protected void doWriteData(byte[] data, int off, long len) throws 
IOException {
+        if (isInitialized.compareAndSet(false, true)) {
+            final byte[] xCookie = getXCookie();
+            if (xCookie == null) {
+                sendEof();
+                return;
+            }
+
+            int l = (int) len;
+            int s = 0;
+            byte[] foo = new byte[l];
+            System.arraycopy(data, off, foo, 0, l);
+
+            if (l < 9) {
+                return;
+            }
+
+            int plen = (foo[s + 6] & 0xff) * 256 + (foo[s + 7] & 0xff);
+            int dlen = (foo[s + 8] & 0xff) * 256 + (foo[s + 9] & 0xff);
+
+            if ((foo[s] & 0xff) == 0x6c) {
+                plen = ((plen >>> 8) & 0xff) | ((plen << 8) & 0xff00);
+                dlen = ((dlen >>> 8) & 0xff) | ((dlen << 8) & 0xff00);

Review Comment:
   Instead of doing
   ```
   // Compute 16bit MSB unsigned short
   if (/* is LSB first */) {
     // Swap bytes
   }
   ```
   let's do
   ```
   if (/* is LSB */) { // lower-case ell
     // Compute 16bit LSB shorts
   } else if (/* is MSB */) { // 'B'
     // Compute 16bit MSB shorts
   } else {
     // throw an exception; data is invalid
   }
   ```



##########
sshd-core/src/main/java/org/apache/sshd/client/channel/ChannelX11.java:
##########
@@ -0,0 +1,166 @@
+/*
+ * 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 org.apache.sshd.client.channel;
+
+import org.apache.sshd.client.future.DefaultOpenFuture;
+import org.apache.sshd.client.future.OpenFuture;
+import org.apache.sshd.client.session.ClientConnectionService;
+import org.apache.sshd.client.x11.X11IoHandler;
+import org.apache.sshd.common.Property;
+import org.apache.sshd.common.SshConstants;
+import org.apache.sshd.common.channel.ChannelOutputStream;
+import org.apache.sshd.common.io.IoConnectFuture;
+import org.apache.sshd.common.io.IoConnector;
+import org.apache.sshd.common.io.IoHandler;
+import org.apache.sshd.common.io.IoSession;
+import org.apache.sshd.common.util.buffer.Buffer;
+import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+/**
+ * @author <a href="mailto:dev@mina.apache.org";>Apache MINA SSHD Project</a>
+ */
+public class ChannelX11 extends AbstractClientChannel {
+
+    public static final Property<Object> X11_COOKIE = 
Property.object("x11-cookie");
+    public static final Property<Object> X11_COOKIE_HEX = 
Property.object("x11-cookie-hex");

Review Comment:
   X11_COOKIE_HEX is a String. Let's de-obfuscate this.



##########
sshd-core/src/main/java/org/apache/sshd/client/channel/PtyCapableChannelSession.java:
##########
@@ -271,4 +302,34 @@ protected void doOpenPty() throws IOException {
 
         sendEnvVariables(session);
     }
+
+    protected byte[] getXCookie() {
+        final Session session = getSession();
+        Object xCookie = ChannelX11.X11_COOKIE.getOrNull(session);
+        Object xCookieHex = ChannelX11.X11_COOKIE_HEX.getOrNull(session);
+        if (xCookie instanceof byte[] && xCookieHex instanceof byte[]) {
+            return (byte[]) xCookieHex;
+        }
+
+        synchronized (session) {
+            xCookieHex = ChannelX11.X11_COOKIE_HEX.getOrNull(session);
+            if (xCookieHex instanceof byte[]) {
+                return (byte[]) xCookieHex;
+            }
+
+            byte[] foo = new byte[16];
+            ThreadLocalRandom.current().nextBytes(foo);
+            ChannelX11.X11_COOKIE.set(session, foo);

Review Comment:
   So we generate a cookie here. This would be the "fake cookie" mentioned in 
RFC4254 then. But ChannelX11 will forward this to the X11 server, which will 
not know it.



##########
sshd-core/src/main/java/org/apache/sshd/client/channel/PtyCapableChannelSession.java:
##########
@@ -239,6 +255,21 @@ protected void doOpenPty() throws IOException {
             writePacket(buffer);
         }
 
+        if (xForwarding) {

Review Comment:
   Check that the X11 host & port properties are set. If not, it's futile to 
request X11 forwarding.
   
   Also, I think there will need to be a way to pass in the cookie (from the 
.Xauthority file).



##########
sshd-core/src/main/java/org/apache/sshd/client/channel/ChannelX11.java:
##########
@@ -0,0 +1,166 @@
+/*
+ * 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 org.apache.sshd.client.channel;
+
+import org.apache.sshd.client.future.DefaultOpenFuture;
+import org.apache.sshd.client.future.OpenFuture;
+import org.apache.sshd.client.session.ClientConnectionService;
+import org.apache.sshd.client.x11.X11IoHandler;
+import org.apache.sshd.common.Property;
+import org.apache.sshd.common.SshConstants;
+import org.apache.sshd.common.channel.ChannelOutputStream;
+import org.apache.sshd.common.io.IoConnectFuture;
+import org.apache.sshd.common.io.IoConnector;
+import org.apache.sshd.common.io.IoHandler;
+import org.apache.sshd.common.io.IoSession;
+import org.apache.sshd.common.util.buffer.Buffer;
+import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+/**
+ * @author <a href="mailto:dev@mina.apache.org";>Apache MINA SSHD Project</a>
+ */
+public class ChannelX11 extends AbstractClientChannel {
+
+    public static final Property<Object> X11_COOKIE = 
Property.object("x11-cookie");
+    public static final Property<Object> X11_COOKIE_HEX = 
Property.object("x11-cookie-hex");
+
+    private final AtomicBoolean isInitialized = new AtomicBoolean(false);
+    private final String host;
+    private final int port;
+
+    private IoSession x11;
+
+    public ChannelX11(String host, int port) {
+        super("x11");
+        this.host = host;
+        this.port = port;
+    }
+
+    @Override
+    public OpenFuture open(long recipient, long rwSize, long packetSize, 
Buffer buffer) {
+        final DefaultOpenFuture defaultOpenFuture = new 
DefaultOpenFuture(this, futureLock);
+        super.openFuture = defaultOpenFuture;
+
+        final IoConnector connector
+                = 
getSession().getFactoryManager().getIoServiceFactory().createConnector(createX11IoHandler());
+        addCloseFutureListener(future -> {
+            if (future.isClosed()) {
+                connector.close(true);
+            }
+        });
+
+        final IoConnectFuture connectFuture = connector.connect(new 
InetSocketAddress(host, port), this, null);
+        connectFuture.addListener(future -> {
+            if (future.isConnected()) {
+                x11 = future.getSession();
+                handleOpenSuccess(recipient, rwSize, packetSize, buffer);
+            } else {
+                if (future.getException() != null) {
+                    defaultOpenFuture.setException(future.getException());
+                } else {
+                    defaultOpenFuture.setValue(false);
+                }
+                unregisterSelf();
+            }
+        });
+
+        return defaultOpenFuture;
+
+    }
+
+    @Override
+    protected void doOpen() throws IOException {
+        setOut(new ChannelOutputStream(this, getRemoteWindow(), log, 
SshConstants.SSH_MSG_CHANNEL_DATA, true));
+    }
+
+    @Override
+    protected void doWriteData(byte[] data, int off, long len) throws 
IOException {
+        if (isInitialized.compareAndSet(false, true)) {
+            final byte[] xCookie = getXCookie();
+            if (xCookie == null) {
+                sendEof();
+                return;
+            }
+
+            int l = (int) len;
+            int s = 0;
+            byte[] foo = new byte[l];
+            System.arraycopy(data, off, foo, 0, l);
+
+            if (l < 9) {
+                return;
+            }
+
+            int plen = (foo[s + 6] & 0xff) * 256 + (foo[s + 7] & 0xff);
+            int dlen = (foo[s + 8] & 0xff) * 256 + (foo[s + 9] & 0xff);
+
+            if ((foo[s] & 0xff) == 0x6c) {
+                plen = ((plen >>> 8) & 0xff) | ((plen << 8) & 0xff00);
+                dlen = ((dlen >>> 8) & 0xff) | ((dlen << 8) & 0xff00);
+            }
+
+            if (l < 12 + plen + ((-plen) & 3) + dlen) {
+                return;
+            }
+
+            byte[] bar = new byte[dlen];
+            System.arraycopy(foo, s + 12 + plen + ((-plen) & 3), bar, 0, dlen);
+
+            if (Objects.deepEquals(xCookie, bar)) {
+                x11.writeBuffer(new ByteArrayBuffer(foo, s, l));

Review Comment:
   If the cookie is the one that was random-generated below, then you would 
have to replace it here with the real cookie. So there must be a way to set 
that real cookie on the channel. Client code could read it from the local 
.Xauthority file.



##########
sshd-core/src/main/java/org/apache/sshd/client/channel/ChannelX11.java:
##########
@@ -0,0 +1,166 @@
+/*
+ * 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 org.apache.sshd.client.channel;
+
+import org.apache.sshd.client.future.DefaultOpenFuture;
+import org.apache.sshd.client.future.OpenFuture;
+import org.apache.sshd.client.session.ClientConnectionService;
+import org.apache.sshd.client.x11.X11IoHandler;
+import org.apache.sshd.common.Property;
+import org.apache.sshd.common.SshConstants;
+import org.apache.sshd.common.channel.ChannelOutputStream;
+import org.apache.sshd.common.io.IoConnectFuture;
+import org.apache.sshd.common.io.IoConnector;
+import org.apache.sshd.common.io.IoHandler;
+import org.apache.sshd.common.io.IoSession;
+import org.apache.sshd.common.util.buffer.Buffer;
+import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+/**
+ * @author <a href="mailto:dev@mina.apache.org";>Apache MINA SSHD Project</a>
+ */
+public class ChannelX11 extends AbstractClientChannel {
+
+    public static final Property<Object> X11_COOKIE = 
Property.object("x11-cookie");

Review Comment:
   Don't use Property for internally computed things. Use `Attribute`.



##########
sshd-core/src/test/java/org/apache/sshd/client/channel/ChannelX11Test.java:
##########
@@ -0,0 +1,166 @@
+/*
+ * 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 org.apache.sshd.client.channel;
+
+import org.apache.sshd.client.SshClient;
+import org.apache.sshd.client.session.ClientSession;
+import org.apache.sshd.client.x11.X11ChannelFactory;
+import org.apache.sshd.common.util.GenericUtils;
+import org.apache.sshd.server.SshServer;
+import org.apache.sshd.server.channel.ChannelSession;
+import org.apache.sshd.server.forward.AcceptAllForwardingFilter;
+import org.apache.sshd.server.session.ServerConnectionService;
+import org.apache.sshd.server.x11.X11ForwardSupport;
+import org.apache.sshd.util.test.BaseTestSupport;
+import org.apache.sshd.util.test.CoreTestSupportUtils;
+import org.junit.jupiter.api.*;
+import org.testcontainers.containers.GenericContainer;
+import org.testcontainers.images.builder.ImageFromDockerfile;
+import org.testcontainers.junit.jupiter.Testcontainers;
+
+import java.io.*;

Review Comment:
   Don't use wildcard imports, please.



##########
sshd-core/src/main/java/org/apache/sshd/client/channel/PtyCapableChannelSession.java:
##########
@@ -271,4 +302,34 @@ protected void doOpenPty() throws IOException {
 
         sendEnvVariables(session);
     }
+
+    protected byte[] getXCookie() {
+        final Session session = getSession();
+        Object xCookie = ChannelX11.X11_COOKIE.getOrNull(session);
+        Object xCookieHex = ChannelX11.X11_COOKIE_HEX.getOrNull(session);
+        if (xCookie instanceof byte[] && xCookieHex instanceof byte[]) {
+            return (byte[]) xCookieHex;
+        }
+
+        synchronized (session) {
+            xCookieHex = ChannelX11.X11_COOKIE_HEX.getOrNull(session);
+            if (xCookieHex instanceof byte[]) {
+                return (byte[]) xCookieHex;
+            }
+
+            byte[] foo = new byte[16];

Review Comment:
   Give variables meaningful names. "foo" and "bar" just won't do. Here "foo" 
could be named "fakeCookie".



##########
sshd-core/src/main/java/org/apache/sshd/client/channel/PtyCapableChannelSession.java:
##########
@@ -34,6 +30,11 @@
 import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
 import org.apache.sshd.core.CoreModuleProperties;
 
+import java.io.IOException;
+import java.util.Collections;
+import java.util.Map;
+import java.util.concurrent.ThreadLocalRandom;
+

Review Comment:
   Keep the order of imports.



##########
sshd-core/src/main/java/org/apache/sshd/client/channel/ChannelX11.java:
##########
@@ -0,0 +1,166 @@
+/*
+ * 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 org.apache.sshd.client.channel;
+
+import org.apache.sshd.client.future.DefaultOpenFuture;
+import org.apache.sshd.client.future.OpenFuture;
+import org.apache.sshd.client.session.ClientConnectionService;
+import org.apache.sshd.client.x11.X11IoHandler;
+import org.apache.sshd.common.Property;
+import org.apache.sshd.common.SshConstants;
+import org.apache.sshd.common.channel.ChannelOutputStream;
+import org.apache.sshd.common.io.IoConnectFuture;
+import org.apache.sshd.common.io.IoConnector;
+import org.apache.sshd.common.io.IoHandler;
+import org.apache.sshd.common.io.IoSession;
+import org.apache.sshd.common.util.buffer.Buffer;
+import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+/**
+ * @author <a href="mailto:dev@mina.apache.org";>Apache MINA SSHD Project</a>
+ */
+public class ChannelX11 extends AbstractClientChannel {
+
+    public static final Property<Object> X11_COOKIE = 
Property.object("x11-cookie");
+    public static final Property<Object> X11_COOKIE_HEX = 
Property.object("x11-cookie-hex");
+
+    private final AtomicBoolean isInitialized = new AtomicBoolean(false);
+    private final String host;
+    private final int port;
+
+    private IoSession x11;
+
+    public ChannelX11(String host, int port) {
+        super("x11");
+        this.host = host;
+        this.port = port;
+    }
+
+    @Override
+    public OpenFuture open(long recipient, long rwSize, long packetSize, 
Buffer buffer) {
+        final DefaultOpenFuture defaultOpenFuture = new 
DefaultOpenFuture(this, futureLock);
+        super.openFuture = defaultOpenFuture;
+
+        final IoConnector connector
+                = 
getSession().getFactoryManager().getIoServiceFactory().createConnector(createX11IoHandler());
+        addCloseFutureListener(future -> {
+            if (future.isClosed()) {
+                connector.close(true);
+            }
+        });
+
+        final IoConnectFuture connectFuture = connector.connect(new 
InetSocketAddress(host, port), this, null);
+        connectFuture.addListener(future -> {
+            if (future.isConnected()) {
+                x11 = future.getSession();
+                handleOpenSuccess(recipient, rwSize, packetSize, buffer);
+            } else {
+                if (future.getException() != null) {
+                    defaultOpenFuture.setException(future.getException());
+                } else {
+                    defaultOpenFuture.setValue(false);
+                }
+                unregisterSelf();
+            }
+        });
+
+        return defaultOpenFuture;
+
+    }
+
+    @Override
+    protected void doOpen() throws IOException {
+        setOut(new ChannelOutputStream(this, getRemoteWindow(), log, 
SshConstants.SSH_MSG_CHANNEL_DATA, true));
+    }
+
+    @Override
+    protected void doWriteData(byte[] data, int off, long len) throws 
IOException {
+        if (isInitialized.compareAndSet(false, true)) {
+            final byte[] xCookie = getXCookie();
+            if (xCookie == null) {
+                sendEof();
+                return;
+            }
+
+            int l = (int) len;
+            int s = 0;
+            byte[] foo = new byte[l];
+            System.arraycopy(data, off, foo, 0, l);
+
+            if (l < 9) {
+                return;
+            }
+
+            int plen = (foo[s + 6] & 0xff) * 256 + (foo[s + 7] & 0xff);
+            int dlen = (foo[s + 8] & 0xff) * 256 + (foo[s + 9] & 0xff);
+
+            if ((foo[s] & 0xff) == 0x6c) {
+                plen = ((plen >>> 8) & 0xff) | ((plen << 8) & 0xff00);
+                dlen = ((dlen >>> 8) & 0xff) | ((dlen << 8) & 0xff00);
+            }
+
+            if (l < 12 + plen + ((-plen) & 3) + dlen) {

Review Comment:
   `plen + ((-plen) & 3)` rounds up to the next multiple of 4. I notice that 
openssh also rounds up `dlen` in this check.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
For additional commands, e-mail: dev-h...@mina.apache.org

Reply via email to