From a03fa9ac6386c4474bf79cc2403d2678fe0e967e Mon Sep 17 00:00:00 2001 From: ISQ-GTT Date: Thu, 9 Mar 2017 13:58:51 +0100 Subject: [PATCH] Added charset support, centralized UTF-8 usage (#305) * Added charset support, centralized UTF-8 usage * Code style, buffer methods with charsets * assure remote charset isn't null --- .../sshj/backport/Jdk7HttpProxySocket.java | 7 +-- src/main/java/net/schmizz/sshj/SSHClient.java | 26 ++++++++++- .../java/net/schmizz/sshj/common/Buffer.java | 29 ++++++++---- .../connection/channel/AbstractChannel.java | 12 +++++ .../sshj/connection/channel/Channel.java | 4 ++ .../channel/direct/AbstractDirectChannel.java | 10 +++++ .../channel/direct/SessionChannel.java | 7 ++- .../schmizz/sshj/sftp/RemoteDirectory.java | 2 +- .../net/schmizz/sshj/sftp/SFTPEngine.java | 44 ++++++++++++------- .../net/schmizz/sshj/xfer/scp/SCPEngine.java | 4 +- .../hierynomus/sshj/test/util/FileUtil.java | 2 +- .../net/schmizz/sshj/util/BufferTest.java | 4 +- .../sshj/util/gss/BogusGSSContext.java | 5 ++- 13 files changed, 120 insertions(+), 36 deletions(-) diff --git a/src/main/java/com/hierynomus/sshj/backport/Jdk7HttpProxySocket.java b/src/main/java/com/hierynomus/sshj/backport/Jdk7HttpProxySocket.java index 2eba7695..a0d61725 100644 --- a/src/main/java/com/hierynomus/sshj/backport/Jdk7HttpProxySocket.java +++ b/src/main/java/com/hierynomus/sshj/backport/Jdk7HttpProxySocket.java @@ -18,7 +18,8 @@ package com.hierynomus.sshj.backport; import java.io.IOException; import java.io.InputStream; import java.net.*; -import java.nio.charset.Charset; + +import net.schmizz.sshj.common.IOUtils; public class Jdk7HttpProxySocket extends Socket { @@ -48,7 +49,7 @@ public class Jdk7HttpProxySocket extends Socket { } InetSocketAddress isa = (InetSocketAddress) endpoint; String httpConnect = "CONNECT " + isa.getHostName() + ":" + isa.getPort() + " HTTP/1.0\n\n"; - getOutputStream().write(httpConnect.getBytes(Charset.forName("UTF-8"))); + getOutputStream().write(httpConnect.getBytes(IOUtils.UTF8)); checkAndFlushProxyResponse(); } @@ -61,7 +62,7 @@ public class Jdk7HttpProxySocket extends Socket { throw new SocketException("Empty response from proxy"); } - String proxyResponse = new String(tmpBuffer, 0, len, "UTF-8"); + String proxyResponse = new String(tmpBuffer, 0, len, IOUtils.UTF8); // Expecting HTTP/1.x 200 OK if (proxyResponse.contains("200")) { diff --git a/src/main/java/net/schmizz/sshj/SSHClient.java b/src/main/java/net/schmizz/sshj/SSHClient.java index 0a12698f..f210cd8e 100644 --- a/src/main/java/net/schmizz/sshj/SSHClient.java +++ b/src/main/java/net/schmizz/sshj/SSHClient.java @@ -16,6 +16,7 @@ package net.schmizz.sshj; import net.schmizz.sshj.common.Factory; +import net.schmizz.sshj.common.IOUtils; import net.schmizz.sshj.common.LoggerFactory; import net.schmizz.sshj.common.SSHException; import net.schmizz.sshj.common.SecurityUtils; @@ -61,6 +62,7 @@ import java.io.Closeable; import java.io.File; import java.io.IOException; import java.net.ServerSocket; +import java.nio.charset.Charset; import java.security.KeyPair; import java.security.PublicKey; import java.util.*; @@ -128,6 +130,9 @@ public class SSHClient private final List forwarders = new ArrayList(); + /** character set of the remote machine */ + protected Charset remoteCharset = IOUtils.UTF8; + /** Default constructor. Initializes this object using {@link DefaultConfig}. */ public SSHClient() { this(new DefaultConfig()); @@ -440,6 +445,15 @@ public class SSHClient return conn; } + /** + * Returns the character set used to communicate with the remote machine for certain strings (like paths). + * + * @return remote character set + */ + public Charset getRemoteCharset() { + return remoteCharset; + } + /** @return a {@link RemotePortForwarder} that allows requesting remote forwarding over this connection. */ public RemotePortForwarder getRemotePortForwarder() { synchronized (conn) { @@ -708,12 +722,22 @@ public class SSHClient doKex(); } + /** + * Sets the character set used to communicate with the remote machine for certain strings (like paths) + * + * @param remoteCharset + * remote character set or {@code null} for default + */ + public void setRemoteCharset(Charset remoteCharset) { + this.remoteCharset = remoteCharset != null ? remoteCharset : IOUtils.UTF8; + } + @Override public Session startSession() throws ConnectionException, TransportException { checkConnected(); checkAuthenticated(); - final SessionChannel sess = new SessionChannel(conn); + final SessionChannel sess = new SessionChannel(conn, remoteCharset); sess.open(); return sess; } diff --git a/src/main/java/net/schmizz/sshj/common/Buffer.java b/src/main/java/net/schmizz/sshj/common/Buffer.java index d5887f2c..41bc9c1c 100644 --- a/src/main/java/net/schmizz/sshj/common/Buffer.java +++ b/src/main/java/net/schmizz/sshj/common/Buffer.java @@ -15,8 +15,8 @@ */ package net.schmizz.sshj.common; -import java.io.UnsupportedEncodingException; import java.math.BigInteger; +import java.nio.charset.Charset; import java.security.GeneralSecurityException; import java.security.PublicKey; import java.util.Arrays; @@ -361,24 +361,31 @@ public class Buffer> { /** * Reads an SSH string * + * @param cs the charset to use for decoding + * * @return the string as a Java {@code String} */ - public String readString() + public String readString(Charset cs) throws BufferException { int len = readUInt32AsInt(); if (len < 0 || len > 32768) throw new BufferException("Bad item length: " + len); ensureAvailable(len); - String s; - try { - s = new String(data, rpos, len, "UTF-8"); - } catch (UnsupportedEncodingException e) { - throw new SSHRuntimeException(e); - } + String s = new String(data, rpos, len, cs); rpos += len; return s; } + /** + * Reads an SSH string using {@code UTF8} + * + * @return the string as a Java {@code String} + */ + public String readString() + throws BufferException { + return readString(IOUtils.UTF8); + } + /** * Reads an SSH string * @@ -397,8 +404,12 @@ public class Buffer> { return putBytes(str, offset, len); } + public T putString(String string, Charset cs) { + return putString(string.getBytes(cs)); + } + public T putString(String string) { - return putString(string.getBytes(IOUtils.UTF8)); + return putString(string, IOUtils.UTF8); } /** diff --git a/src/main/java/net/schmizz/sshj/connection/channel/AbstractChannel.java b/src/main/java/net/schmizz/sshj/connection/channel/AbstractChannel.java index 779932fe..ff318cd7 100644 --- a/src/main/java/net/schmizz/sshj/connection/channel/AbstractChannel.java +++ b/src/main/java/net/schmizz/sshj/connection/channel/AbstractChannel.java @@ -26,6 +26,7 @@ import org.slf4j.Logger; import java.io.InputStream; import java.io.OutputStream; +import java.nio.charset.Charset; import java.util.LinkedList; import java.util.Queue; import java.util.concurrent.TimeUnit; @@ -51,6 +52,8 @@ public abstract class AbstractChannel private final int id; /** Remote recipient ID */ private int recipient; + /** Remote character set */ + private final Charset remoteCharset; private boolean eof = false; @@ -78,12 +81,16 @@ public abstract class AbstractChannel private volatile boolean autoExpand = false; protected AbstractChannel(Connection conn, String type) { + this(conn, type, null); + } + protected AbstractChannel(Connection conn, String type, Charset remoteCharset) { this.conn = conn; this.loggerFactory = conn.getTransport().getConfig().getLoggerFactory(); this.type = type; this.log = loggerFactory.getLogger(getClass()); this.trans = conn.getTransport(); + this.remoteCharset = remoteCharset != null ? remoteCharset : IOUtils.UTF8; id = conn.nextID(); lwin = new Window.Local(conn.getWindowSize(), conn.getMaxPacketSize(), loggerFactory); @@ -135,6 +142,11 @@ public abstract class AbstractChannel return recipient; } + @Override + public Charset getRemoteCharset() { + return remoteCharset; + } + @Override public int getRemoteMaxPacketSize() { return rwin.getMaxPacketSize(); diff --git a/src/main/java/net/schmizz/sshj/connection/channel/Channel.java b/src/main/java/net/schmizz/sshj/connection/channel/Channel.java index 17793822..747f2a73 100644 --- a/src/main/java/net/schmizz/sshj/connection/channel/Channel.java +++ b/src/main/java/net/schmizz/sshj/connection/channel/Channel.java @@ -24,6 +24,7 @@ import net.schmizz.sshj.transport.TransportException; import java.io.Closeable; import java.io.InputStream; import java.io.OutputStream; +import java.nio.charset.Charset; import java.util.concurrent.TimeUnit; /** @@ -123,6 +124,9 @@ public interface Channel extends Closeable, SSHPacketHandler, ErrorNotifiable { */ int getRecipient(); + /** @return the character set used to communicate with the remote machine for certain strings (like paths). */ + Charset getRemoteCharset(); + /** * @return the maximum packet size as specified by the remote end. */ diff --git a/src/main/java/net/schmizz/sshj/connection/channel/direct/AbstractDirectChannel.java b/src/main/java/net/schmizz/sshj/connection/channel/direct/AbstractDirectChannel.java index aab0794c..2c432acc 100644 --- a/src/main/java/net/schmizz/sshj/connection/channel/direct/AbstractDirectChannel.java +++ b/src/main/java/net/schmizz/sshj/connection/channel/direct/AbstractDirectChannel.java @@ -25,6 +25,7 @@ import net.schmizz.sshj.connection.channel.Channel; import net.schmizz.sshj.connection.channel.OpenFailException; import net.schmizz.sshj.transport.TransportException; +import java.nio.charset.Charset; import java.util.concurrent.TimeUnit; /** Base class for direct channels whose open is initated by the client. */ @@ -41,6 +42,15 @@ public abstract class AbstractDirectChannel conn.attach(this); } + protected AbstractDirectChannel(Connection conn, String type, Charset remoteCharset) { + super(conn, type, remoteCharset); + + /* + * We expect to receive channel open confirmation/rejection and want to be able to next this packet. + */ + conn.attach(this); + } + @Override public void open() throws ConnectionException, TransportException { diff --git a/src/main/java/net/schmizz/sshj/connection/channel/direct/SessionChannel.java b/src/main/java/net/schmizz/sshj/connection/channel/direct/SessionChannel.java index c28da4be..dfdfa55e 100644 --- a/src/main/java/net/schmizz/sshj/connection/channel/direct/SessionChannel.java +++ b/src/main/java/net/schmizz/sshj/connection/channel/direct/SessionChannel.java @@ -22,6 +22,7 @@ import net.schmizz.sshj.connection.channel.ChannelInputStream; import net.schmizz.sshj.transport.TransportException; import java.io.InputStream; +import java.nio.charset.Charset; import java.util.Collections; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -47,6 +48,10 @@ public class SessionChannel super(conn, "session"); } + public SessionChannel(Connection conn, Charset remoteCharset) { + super(conn, "session", remoteCharset); + } + @Override public void allocateDefaultPTY() throws ConnectionException, TransportException { @@ -93,7 +98,7 @@ public class SessionChannel throws ConnectionException, TransportException { checkReuse(); log.debug("Will request to exec `{}`", command); - sendChannelRequest("exec", true, new Buffer.PlainBuffer().putString(command)) + sendChannelRequest("exec", true, new Buffer.PlainBuffer().putString(command, getRemoteCharset())) .await(conn.getTimeoutMs(), TimeUnit.MILLISECONDS); usedUp = true; return this; diff --git a/src/main/java/net/schmizz/sshj/sftp/RemoteDirectory.java b/src/main/java/net/schmizz/sshj/sftp/RemoteDirectory.java index 58507b5a..4f9718ed 100644 --- a/src/main/java/net/schmizz/sshj/sftp/RemoteDirectory.java +++ b/src/main/java/net/schmizz/sshj/sftp/RemoteDirectory.java @@ -42,7 +42,7 @@ public class RemoteDirectory case NAME: final int count = res.readUInt32AsInt(); for (int i = 0; i < count; i++) { - final String name = res.readString(); + final String name = res.readString(requester.sub.getRemoteCharset()); res.readString(); // long name - IGNORED - shdve never been in the protocol final FileAttributes attrs = res.readFileAttributes(); final PathComponents comps = requester.getPathHelper().getComponents(path, name); diff --git a/src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java b/src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java index 40a18c7d..75e475e0 100644 --- a/src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java +++ b/src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java @@ -16,6 +16,7 @@ package net.schmizz.sshj.sftp; import net.schmizz.concurrent.Promise; +import net.schmizz.sshj.common.IOUtils; import net.schmizz.sshj.common.LoggerFactory; import net.schmizz.sshj.common.SSHException; import net.schmizz.sshj.connection.channel.direct.Session; @@ -25,6 +26,7 @@ import org.slf4j.Logger; import java.io.Closeable; import java.io.IOException; import java.io.OutputStream; +import java.nio.charset.Charset; import java.util.EnumSet; import java.util.HashMap; import java.util.Map; @@ -137,7 +139,7 @@ public class SFTPEngine public RemoteFile open(String path, Set modes, FileAttributes fa) throws IOException { final byte[] handle = doRequest( - newRequest(PacketType.OPEN).putString(path).putUInt32(OpenMode.toMask(modes)).putFileAttributes(fa) + newRequest(PacketType.OPEN).putString(path, sub.getRemoteCharset()).putUInt32(OpenMode.toMask(modes)).putFileAttributes(fa) ).ensurePacketTypeIs(PacketType.HANDLE).readBytes(); return new RemoteFile(this, path, handle); } @@ -155,7 +157,7 @@ public class SFTPEngine public RemoteDirectory openDir(String path) throws IOException { final byte[] handle = doRequest( - newRequest(PacketType.OPENDIR).putString(path) + newRequest(PacketType.OPENDIR).putString(path, sub.getRemoteCharset()) ).ensurePacketTypeIs(PacketType.HANDLE).readBytes(); return new RemoteDirectory(this, path, handle); } @@ -163,7 +165,7 @@ public class SFTPEngine public void setAttributes(String path, FileAttributes attrs) throws IOException { doRequest( - newRequest(PacketType.SETSTAT).putString(path).putFileAttributes(attrs) + newRequest(PacketType.SETSTAT).putString(path, sub.getRemoteCharset()).putFileAttributes(attrs) ).ensureStatusPacketIsOK(); } @@ -173,13 +175,13 @@ public class SFTPEngine throw new SFTPException("READLINK is not supported in SFTPv" + operativeVersion); return readSingleName( doRequest( - newRequest(PacketType.READLINK).putString(path) - )); + newRequest(PacketType.READLINK).putString(path, sub.getRemoteCharset()) + ), sub.getRemoteCharset()); } public void makeDir(String path, FileAttributes attrs) throws IOException { - doRequest(newRequest(PacketType.MKDIR).putString(path).putFileAttributes(attrs)).ensureStatusPacketIsOK(); + doRequest(newRequest(PacketType.MKDIR).putString(path, sub.getRemoteCharset()).putFileAttributes(attrs)).ensureStatusPacketIsOK(); } public void makeDir(String path) @@ -192,21 +194,21 @@ public class SFTPEngine if (operativeVersion < 3) throw new SFTPException("SYMLINK is not supported in SFTPv" + operativeVersion); doRequest( - newRequest(PacketType.SYMLINK).putString(linkpath).putString(targetpath) + newRequest(PacketType.SYMLINK).putString(linkpath, sub.getRemoteCharset()).putString(targetpath, sub.getRemoteCharset()) ).ensureStatusPacketIsOK(); } public void remove(String filename) throws IOException { doRequest( - newRequest(PacketType.REMOVE).putString(filename) + newRequest(PacketType.REMOVE).putString(filename, sub.getRemoteCharset()) ).ensureStatusPacketIsOK(); } public void removeDir(String path) throws IOException { doRequest( - newRequest(PacketType.RMDIR).putString(path) + newRequest(PacketType.RMDIR).putString(path, sub.getRemoteCharset()) ).ensureStatusIs(Response.StatusCode.OK); } @@ -225,7 +227,7 @@ public class SFTPEngine if (operativeVersion < 1) throw new SFTPException("RENAME is not supported in SFTPv" + operativeVersion); doRequest( - newRequest(PacketType.RENAME).putString(oldPath).putString(newPath) + newRequest(PacketType.RENAME).putString(oldPath, sub.getRemoteCharset()).putString(newPath, sub.getRemoteCharset()) ).ensureStatusPacketIsOK(); } @@ -233,8 +235,8 @@ public class SFTPEngine throws IOException { return readSingleName( doRequest( - newRequest(PacketType.REALPATH).putString(path) - )); + newRequest(PacketType.REALPATH).putString(path, sub.getRemoteCharset()) + ), sub.getRemoteCharset()); } public void setTimeoutMs(int timeoutMs) { @@ -258,20 +260,32 @@ public class SFTPEngine protected FileAttributes stat(PacketType pt, String path) throws IOException { - return doRequest(newRequest(pt).putString(path)) + return doRequest(newRequest(pt).putString(path, sub.getRemoteCharset())) .ensurePacketTypeIs(PacketType.ATTRS) .readFileAttributes(); } - protected static String readSingleName(Response res) + private static byte[] readSingleNameAsBytes(Response res) throws IOException { res.ensurePacketTypeIs(PacketType.NAME); if (res.readUInt32AsInt() == 1) - return res.readString(); + return res.readStringAsBytes(); else throw new SFTPException("Unexpected data in " + res.getType() + " packet"); } + /** Using UTF-8 */ + protected static String readSingleName(Response res) + throws IOException { + return readSingleName(res, IOUtils.UTF8); + } + + /** Using any character set */ + protected static String readSingleName(Response res, Charset charset) + throws IOException { + return new String(readSingleNameAsBytes(res), charset); + } + protected synchronized void transmit(SFTPPacket payload) throws IOException { final int len = payload.available(); diff --git a/src/main/java/net/schmizz/sshj/xfer/scp/SCPEngine.java b/src/main/java/net/schmizz/sshj/xfer/scp/SCPEngine.java index 3869e110..821a1c86 100644 --- a/src/main/java/net/schmizz/sshj/xfer/scp/SCPEngine.java +++ b/src/main/java/net/schmizz/sshj/xfer/scp/SCPEngine.java @@ -29,7 +29,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -/** @see SCP Protocol */ +/** @see SCP Protocol */ class SCPEngine { @@ -128,7 +128,7 @@ class SCPEngine { void sendMessage(String msg) throws IOException { log.debug("Sending message: {}", msg); - scp.getOutputStream().write((msg + LF).getBytes(IOUtils.UTF8)); + scp.getOutputStream().write((msg + LF).getBytes(scp.getRemoteCharset())); scp.getOutputStream().flush(); check("Message ACK received"); } diff --git a/src/test/java/com/hierynomus/sshj/test/util/FileUtil.java b/src/test/java/com/hierynomus/sshj/test/util/FileUtil.java index 19ccff1d..10662bc1 100644 --- a/src/test/java/com/hierynomus/sshj/test/util/FileUtil.java +++ b/src/test/java/com/hierynomus/sshj/test/util/FileUtil.java @@ -34,7 +34,7 @@ public class FileUtil { FileInputStream fileInputStream = new FileInputStream(f); try { ByteArrayOutputStream byteArrayOutputStream = IOUtils.readFully(fileInputStream); - return byteArrayOutputStream.toString("UTF-8"); + return byteArrayOutputStream.toString(IOUtils.UTF8.displayName()); } finally { IOUtils.closeQuietly(fileInputStream); } diff --git a/src/test/java/net/schmizz/sshj/util/BufferTest.java b/src/test/java/net/schmizz/sshj/util/BufferTest.java index 49d1c4c8..0d3316d5 100644 --- a/src/test/java/net/schmizz/sshj/util/BufferTest.java +++ b/src/test/java/net/schmizz/sshj/util/BufferTest.java @@ -31,6 +31,8 @@ package net.schmizz.sshj.util; import net.schmizz.sshj.common.Buffer; +import net.schmizz.sshj.common.IOUtils; + import org.junit.Before; import org.junit.Test; @@ -51,7 +53,7 @@ public class BufferTest { public void setUp() throws UnsupportedEncodingException, GeneralSecurityException { // for position test - byte[] data = "Hello".getBytes("UTF-8"); + byte[] data = "Hello".getBytes(IOUtils.UTF8); posBuf = new Buffer.PlainBuffer(data); handyBuf = new Buffer.PlainBuffer(); } diff --git a/src/test/java/net/schmizz/sshj/util/gss/BogusGSSContext.java b/src/test/java/net/schmizz/sshj/util/gss/BogusGSSContext.java index 892faac2..69000fad 100644 --- a/src/test/java/net/schmizz/sshj/util/gss/BogusGSSContext.java +++ b/src/test/java/net/schmizz/sshj/util/gss/BogusGSSContext.java @@ -17,11 +17,12 @@ package net.schmizz.sshj.util.gss; import org.ietf.jgss.*; +import net.schmizz.sshj.common.IOUtils; + import java.io.InputStream; import java.io.OutputStream; import java.net.InetAddress; import java.net.UnknownHostException; -import java.nio.charset.Charset; import java.util.Arrays; import static net.schmizz.sshj.util.gss.BogusGSSManager.unavailable; @@ -34,7 +35,7 @@ public class BogusGSSContext private static final byte[] MIC = fromString("LGTM"); private static byte[] fromString(String s) { - return s.getBytes(Charset.forName("UTF-8")); + return s.getBytes(IOUtils.UTF8); } private boolean initialized = false;