Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added charset support, centralized UTF-8 usage #305

Merged
merged 3 commits into from
Mar 9, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
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 {

Expand Down Expand Up @@ -48,7 +49,7 @@ private void connectHttpProxy(SocketAddress endpoint, int timeout) throws IOExce
}
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();
}

Expand All @@ -61,7 +62,7 @@ private void checkAndFlushProxyResponse()throws IOException {
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")) {
Expand Down
27 changes: 26 additions & 1 deletion src/main/java/net/schmizz/sshj/SSHClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
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.*;
Expand Down Expand Up @@ -128,6 +129,9 @@ public class SSHClient

private final List<LocalPortForwarder> forwarders = new ArrayList<LocalPortForwarder>();

/** character set of the remote machine */
protected Charset remoteCharset;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assign the default IOUtils.UTF_8 here.


/** Default constructor. Initializes this object using {@link DefaultConfig}. */
public SSHClient() {
this(new DefaultConfig());
Expand Down Expand Up @@ -440,6 +444,16 @@ public Connection getConnection() {
return conn;
}

/**
* Returns the character set used to communicate with the remote machine for certain strings (like paths).
*
* @return remote character set or {@code null} for default
*/
public Charset getRemoteCharset()
{
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the same formatting as the rest of the code, ie. open-curly on end-of-line

return remoteCharset;
}

/** @return a {@link RemotePortForwarder} that allows requesting remote forwarding over this connection. */
public RemotePortForwarder getRemotePortForwarder() {
synchronized (conn) {
Expand Down Expand Up @@ -708,12 +722,23 @@ public void rekey()
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;
}

@Override
public Session startSession()
throws ConnectionException, TransportException {
checkConnected();
checkAuthenticated();
final SessionChannel sess = new SessionChannel(conn);
final SessionChannel sess = (remoteCharset != null)? new SessionChannel(conn, remoteCharset) : new SessionChannel(conn);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to do the if in the case that we default to UTF_8 here already.

sess.open();
return sess;
}
Expand Down
8 changes: 1 addition & 7 deletions src/main/java/net/schmizz/sshj/common/Buffer.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
package net.schmizz.sshj.common;

import java.io.UnsupportedEncodingException;
import java.math.BigInteger;
import java.security.GeneralSecurityException;
import java.security.PublicKey;
Expand Down Expand Up @@ -369,12 +368,7 @@ public String readString()
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, IOUtils.UTF8);
rpos += len;
return s;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

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;
Expand All @@ -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;

Expand Down Expand Up @@ -78,12 +81,16 @@ public abstract class AbstractChannel
private volatile boolean autoExpand = false;

protected AbstractChannel(Connection conn, String type) {
this(conn, type, IOUtils.UTF8);
}
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;
id = conn.nextID();

lwin = new Window.Local(conn.getWindowSize(), conn.getMaxPacketSize(), loggerFactory);
Expand Down Expand Up @@ -135,6 +142,12 @@ public int getRecipient() {
return recipient;
}

@Override
public Charset getRemoteCharset()
{
return remoteCharset;
}

@Override
public int getRemoteMaxPacketSize() {
return rwin.getMaxPacketSize();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.io.Closeable;
import java.io.InputStream;
import java.io.OutputStream;
import java.nio.charset.Charset;
import java.util.concurrent.TimeUnit;

/**
Expand Down Expand Up @@ -123,6 +124,9 @@ interface Forwarded extends Channel {
*/
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.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
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. */
Expand All @@ -41,6 +42,15 @@ protected AbstractDirectChannel(Connection conn, String type) {
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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
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;
Expand All @@ -47,6 +48,10 @@ public SessionChannel(Connection conn) {
super(conn, "session");
}

public SessionChannel(Connection conn, Charset remoteCharset) {
super(conn, "session", remoteCharset);
}

@Override
public void allocateDefaultPTY()
throws ConnectionException, TransportException {
Expand Down Expand Up @@ -93,7 +98,7 @@ public Command exec(String command)
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.getBytes(getRemoteCharset())))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main question I have when reading this, should every string be written using the remote charset? If so, it is nicer to make it a property of the Buffer, rather than converting it everywhere we call putString.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or make a putString(String s, Charset cs) method, which makes the rest of the code at least cleaner and more concise.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The remote side seems to decode the command using its own charset instead of utf-8. Commands containing filenames with special characters won't work properly using utf-8 on non-utf-8 remote machines.
I'll add the putString(String s, Charset cs) method. There will still be a lot of getRemoteCharset() calls in SFTPEngine though which could also be replaced by a property.

.await(conn.getTimeoutMs(), TimeUnit.MILLISECONDS);
usedUp = true;
return this;
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/net/schmizz/sshj/sftp/RemoteDirectory.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public List<RemoteResourceInfo> scan(RemoteResourceFilter filter)
case NAME:
final int count = res.readUInt32AsInt();
for (int i = 0; i < count; i++) {
final String name = res.readString();
final String name = new String(res.readStringAsBytes(), 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);
Expand Down
44 changes: 29 additions & 15 deletions src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -25,6 +26,7 @@
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;
Expand Down Expand Up @@ -137,7 +139,7 @@ private Response doRequest(Request req)
public RemoteFile open(String path, Set<OpenMode> 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.getBytes(sub.getRemoteCharset())).putUInt32(OpenMode.toMask(modes)).putFileAttributes(fa)
).ensurePacketTypeIs(PacketType.HANDLE).readBytes();
return new RemoteFile(this, path, handle);
}
Expand All @@ -155,15 +157,15 @@ public RemoteFile open(String filename)
public RemoteDirectory openDir(String path)
throws IOException {
final byte[] handle = doRequest(
newRequest(PacketType.OPENDIR).putString(path)
newRequest(PacketType.OPENDIR).putString(path.getBytes(sub.getRemoteCharset()))
).ensurePacketTypeIs(PacketType.HANDLE).readBytes();
return new RemoteDirectory(this, path, handle);
}

public void setAttributes(String path, FileAttributes attrs)
throws IOException {
doRequest(
newRequest(PacketType.SETSTAT).putString(path).putFileAttributes(attrs)
newRequest(PacketType.SETSTAT).putString(path.getBytes(sub.getRemoteCharset())).putFileAttributes(attrs)
).ensureStatusPacketIsOK();
}

Expand All @@ -173,13 +175,13 @@ public String readLink(String path)
throw new SFTPException("READLINK is not supported in SFTPv" + operativeVersion);
return readSingleName(
doRequest(
newRequest(PacketType.READLINK).putString(path)
));
newRequest(PacketType.READLINK).putString(path.getBytes(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.getBytes(sub.getRemoteCharset())).putFileAttributes(attrs)).ensureStatusPacketIsOK();
}

public void makeDir(String path)
Expand All @@ -192,21 +194,21 @@ public void symlink(String linkpath, String targetpath)
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.getBytes(sub.getRemoteCharset())).putString(targetpath.getBytes(sub.getRemoteCharset()))
).ensureStatusPacketIsOK();
}

public void remove(String filename)
throws IOException {
doRequest(
newRequest(PacketType.REMOVE).putString(filename)
newRequest(PacketType.REMOVE).putString(filename.getBytes(sub.getRemoteCharset()))
).ensureStatusPacketIsOK();
}

public void removeDir(String path)
throws IOException {
doRequest(
newRequest(PacketType.RMDIR).putString(path)
newRequest(PacketType.RMDIR).putString(path.getBytes(sub.getRemoteCharset()))
).ensureStatusIs(Response.StatusCode.OK);
}

Expand All @@ -225,16 +227,16 @@ public void rename(String oldPath, String newPath)
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.getBytes(sub.getRemoteCharset())).putString(newPath.getBytes(sub.getRemoteCharset()))
).ensureStatusPacketIsOK();
}

public String canonicalize(String path)
throws IOException {
return readSingleName(
doRequest(
newRequest(PacketType.REALPATH).putString(path)
));
newRequest(PacketType.REALPATH).putString(path.getBytes(sub.getRemoteCharset()))
), sub.getRemoteCharset());
}

public void setTimeoutMs(int timeoutMs) {
Expand All @@ -258,20 +260,32 @@ protected LoggerFactory getLoggerFactory() {

protected FileAttributes stat(PacketType pt, String path)
throws IOException {
return doRequest(newRequest(pt).putString(path))
return doRequest(newRequest(pt).putString(path.getBytes(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<Request> payload)
throws IOException {
final int len = payload.available();
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/net/schmizz/sshj/xfer/scp/SCPEngine.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
import java.io.InputStream;
import java.io.OutputStream;

/** @see <a href="http://blogs.sun.com/janp/entry/how_the_scp_protocol_works">SCP Protocol</a> */
/** @see <a href="https://blogs.oracle.com/janp/entry/how_the_scp_protocol_works">SCP Protocol</a> */
class SCPEngine {


Expand Down Expand Up @@ -128,7 +128,7 @@ String readMessage()

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");
}
Expand Down
Loading