Skip to content

Commit

Permalink
apacheGH-275: Improve SFTP status message parsing
Browse files Browse the repository at this point in the history
Introduce a message record for SSH_FXP_STATUS to centralize parsing it
from a buffer. Treat the error message and the language tag as optional
because these fields exist only since SFTP v3, and apparently there are
SFTP v3 servers that don't send it.

Bug: apache#275
  • Loading branch information
tomaswolf committed Nov 25, 2022
1 parent 29ed50c commit 95526ce
Show file tree
Hide file tree
Showing 7 changed files with 207 additions and 71 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
## Bug fixes

* [GH-268](https://github.com/apache/mina-sshd/issues/268) (Regression in 2.9.0) Heartbeat should throw an exception if no reply arrives within the timeout.
* [GH-275](https://github.com/apache/mina-sshd/issues/275) SFTP: be more lenient when reading SSH_FXP_STATUS replies.

## Major code re-factoring

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.apache.sshd.sftp.client.SftpClient;
import org.apache.sshd.sftp.client.SftpClient.Handle;
import org.apache.sshd.sftp.client.extensions.SftpClientExtension;
import org.apache.sshd.sftp.client.impl.SftpStatus;
import org.apache.sshd.sftp.common.SftpConstants;
import org.apache.sshd.sftp.common.SftpException;

Expand Down Expand Up @@ -191,16 +192,13 @@ protected Buffer checkExtendedReplyBuffer(Buffer buffer) throws IOException {
validateIncomingResponse(SftpConstants.SSH_FXP_EXTENDED, id, type, length, buffer);

if (type == SftpConstants.SSH_FXP_STATUS) {
int substatus = buffer.getInt();
String msg = buffer.getString();
String lang = buffer.getString();
SftpStatus status = SftpStatus.parse(buffer);
if (log.isDebugEnabled()) {
log.debug("checkExtendedReplyBuffer({})[id={}] - status: {} [{}] {}",
getName(), id, substatus, lang, msg);
log.debug("checkExtendedReplyBuffer({})[id={}] - status: {}", getName(), id, status);
}

if (substatus != SftpConstants.SSH_FX_OK) {
throwStatusException(id, substatus, msg, lang);
if (!status.isOk()) {
throwStatusException(id, status);
}

return null;
Expand All @@ -222,7 +220,7 @@ protected void validateIncomingResponse(
}
}

protected void throwStatusException(int id, int substatus, String msg, String lang) throws IOException {
throw new SftpException(substatus, msg);
protected void throwStatusException(int id, SftpStatus status) throws IOException {
throw new SftpException(status.getStatusCode(), status.getMessage());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -180,10 +180,7 @@ protected void checkResponseStatus(int cmd, Buffer buffer) throws IOException {
validateIncomingResponse(cmd, id, type, length, buffer);

if (type == SftpConstants.SSH_FXP_STATUS) {
int substatus = buffer.getInt();
String msg = buffer.getString();
String lang = buffer.getString();
checkResponseStatus(cmd, id, substatus, msg, lang);
checkResponseStatus(cmd, id, SftpStatus.parse(buffer));
} else {
IOException err = handleUnexpectedPacket(cmd, SftpConstants.SSH_FXP_STATUS, id, type, length, buffer);
if (err != null) {
Expand All @@ -195,26 +192,24 @@ protected void checkResponseStatus(int cmd, Buffer buffer) throws IOException {
/**
* @param cmd The sent command opcode
* @param id The request id
* @param substatus The sub-status value
* @param msg The message
* @param lang The language
* @throws IOException if the sub-status is not {@code SSH_FX_OK}
* @see #throwStatusException(int, int, int, String, String)
* @param status The {@link SftpStatus}
* @throws IOException if {@code !status.isOk()}
* @see #throwStatusException(int, int, SftpStatus)
*/
protected void checkResponseStatus(int cmd, int id, int substatus, String msg, String lang) throws IOException {
protected void checkResponseStatus(int cmd, int id, SftpStatus status) throws IOException {
if (log.isTraceEnabled()) {
log.trace("checkResponseStatus({})[id={}] cmd={} status={} lang={} msg={}",
log.trace("checkResponseStatus({})[id={}] cmd={} status={}",
getClientChannel(), id, SftpConstants.getCommandMessageName(cmd),
SftpConstants.getStatusName(substatus), lang, msg);
status);
}

if (substatus != SftpConstants.SSH_FX_OK) {
throwStatusException(cmd, id, substatus, msg, lang);
if (!status.isOk()) {
throwStatusException(cmd, id, status);
}
}

protected void throwStatusException(int cmd, int id, int substatus, String msg, String lang) throws IOException {
throw new SftpException(substatus, msg);
protected void throwStatusException(int cmd, int id, SftpStatus status) throws IOException {
throw new SftpException(status.getStatusCode(), status.getMessage());
}

/**
Expand Down Expand Up @@ -244,15 +239,12 @@ protected byte[] checkHandleResponse(int cmd, Buffer buffer) throws IOException
}

if (type == SftpConstants.SSH_FXP_STATUS) {
int substatus = buffer.getInt();
String msg = buffer.getString();
String lang = buffer.getString();
SftpStatus status = SftpStatus.parse(buffer);
if (log.isTraceEnabled()) {
log.trace("checkHandleResponse({})[id={}] {} - status: {} [{}] {}",
getClientChannel(), id, SftpConstants.getCommandMessageName(cmd),
SftpConstants.getStatusName(substatus), lang, msg);
log.trace("checkHandleResponse({})[id={}] {} - status: {}", getClientChannel(), id,
SftpConstants.getCommandMessageName(cmd), status);
}
throwStatusException(cmd, id, substatus, msg, lang);
throwStatusException(cmd, id, status);
}

return handleUnexpectedHandlePacket(cmd, id, type, length, buffer);
Expand Down Expand Up @@ -294,15 +286,12 @@ protected Attributes checkAttributesResponse(int cmd, Buffer buffer) throws IOEx
}

if (type == SftpConstants.SSH_FXP_STATUS) {
int substatus = buffer.getInt();
String msg = buffer.getString();
String lang = buffer.getString();
SftpStatus status = SftpStatus.parse(buffer);
if (log.isTraceEnabled()) {
log.trace("checkAttributesResponse({})[id={}] {} - status: {} [{}] {}",
getClientChannel(), id, SftpConstants.getCommandMessageName(cmd),
SftpConstants.getStatusName(substatus), lang, msg);
log.trace("checkAttributesResponse({})[id={}] {} - status: {}", getClientChannel(), id,
SftpConstants.getCommandMessageName(cmd), status);
}
throwStatusException(cmd, id, substatus, msg, lang);
throwStatusException(cmd, id, status);
}

return handleUnexpectedAttributesPacket(cmd, id, type, length, buffer);
Expand Down Expand Up @@ -366,16 +355,13 @@ protected String checkOneNameResponse(int cmd, Buffer buffer) throws IOException
}

if (type == SftpConstants.SSH_FXP_STATUS) {
int substatus = buffer.getInt();
String msg = buffer.getString();
String lang = buffer.getString();
SftpStatus status = SftpStatus.parse(buffer);
if (log.isTraceEnabled()) {
log.trace("checkOneNameResponse({})[id={}] {} status: {} [{}] {}",
getClientChannel(), id, SftpConstants.getCommandMessageName(cmd),
SftpConstants.getStatusName(substatus), lang, msg);
log.trace("checkOneNameResponse({})[id={}] {} status: {}", getClientChannel(), id,
SftpConstants.getCommandMessageName(cmd), status);
}

throwStatusException(cmd, id, substatus, msg, lang);
throwStatusException(cmd, id, status);
}

return handleUnknownOneNamePacket(cmd, id, type, length, buffer);
Expand Down Expand Up @@ -696,20 +682,18 @@ protected int checkDataResponse(
}

if (type == SftpConstants.SSH_FXP_STATUS) {
int substatus = buffer.getInt();
String msg = buffer.getString();
String lang = buffer.getString();
SftpStatus status = SftpStatus.parse(buffer);
if (log.isTraceEnabled()) {
log.trace("checkDataResponse({})[id={}] {} status: {} [{}] {}",
log.trace("checkDataResponse({})[id={}] {} status: {}",
getClientChannel(), id, SftpConstants.getCommandMessageName(cmd),
SftpConstants.getStatusName(substatus), lang, msg);
status);
}

if (substatus == SftpConstants.SSH_FX_EOF) {
if (status.getStatusCode() == SftpConstants.SSH_FX_EOF) {
return -1;
}

throwStatusException(cmd, id, substatus, msg, lang);
throwStatusException(cmd, id, status);
}

return handleUnknownDataPacket(cmd, id, type, length, buffer);
Expand Down Expand Up @@ -910,19 +894,16 @@ protected List<DirEntry> checkDirResponse(int cmd, Buffer buffer, AtomicReferenc
}

if (type == SftpConstants.SSH_FXP_STATUS) {
int substatus = buffer.getInt();
String msg = buffer.getString();
String lang = buffer.getString();
SftpStatus status = SftpStatus.parse(buffer);
if (traceEnabled) {
log.trace("checkDirResponse({})[id={}] - status: {} [{}] {}",
getClientChannel(), id, SftpConstants.getStatusName(substatus), lang, msg);
log.trace("checkDirResponse({})[id={}] - status: {}", getClientChannel(), id, status);
}

if (substatus == SftpConstants.SSH_FX_EOF) {
if (status.getStatusCode() == SftpConstants.SSH_FX_EOF) {
return null;
}

throwStatusException(cmd, id, substatus, msg, lang);
throwStatusException(cmd, id, status);
}

return handleUnknownDirListingPacket(cmd, id, type, length, buffer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -416,15 +416,12 @@ protected void handleInitResponse(Buffer buffer) throws IOException {
extensions.put(name, data);
}
} else if (type == SftpConstants.SSH_FXP_STATUS) {
int substatus = buffer.getInt();
String msg = buffer.getString();
String lang = buffer.getString();
SftpStatus status = SftpStatus.parse(buffer);
if (traceEnabled) {
log.trace("handleInitResponse({})[id={}] - status: {} [{}] {}",
clientChannel, id, SftpConstants.getStatusName(substatus), lang, msg);
log.trace("handleInitResponse({})[id={}] - status: {}", clientChannel, id, status);
}

throwStatusException(SftpConstants.SSH_FXP_INIT, id, substatus, msg, lang);
throwStatusException(SftpConstants.SSH_FXP_INIT, id, status);
} else {
IOException err = handleUnexpectedPacket(
SftpConstants.SSH_FXP_INIT, SftpConstants.SSH_FXP_VERSION, id, type, length, buffer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -333,13 +333,11 @@ protected void pollBuffer(SftpAckData ack) throws IOException {
buf.wpos(rpos + dlen);
this.buffer = buf;
} else if (type == SftpConstants.SSH_FXP_STATUS) {
int substatus = buf.getInt();
String msg = buf.getString();
String lang = buf.getString();
if (substatus == SftpConstants.SSH_FX_EOF) {
SftpStatus status = SftpStatus.parse(buf);
if (status.getStatusCode() == SftpConstants.SSH_FX_EOF) {
eofIndicator = true;
} else {
client.checkResponseStatus(SshConstants.SSH_MSG_CHANNEL_DATA, id, substatus, msg, lang);
client.checkResponseStatus(SshConstants.SSH_MSG_CHANNEL_DATA, id, status);
}
} else {
IOException err = client.handleUnexpectedPacket(SshConstants.SSH_MSG_CHANNEL_DATA,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
* 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.sftp.client.impl;

import org.apache.sshd.common.util.buffer.Buffer;
import org.apache.sshd.sftp.common.SftpConstants;

/**
* A representation of a SSH_FXP_STATUS record.
*/
public final class SftpStatus {

private final int statusCode;

private final String language;

private final String message;

private SftpStatus(int statusCode, String message, String language) {
this.statusCode = statusCode;
this.message = message;
this.language = language;
}

public int getStatusCode() {
return statusCode;
}

public String getLanguage() {
return language;
}

public String getMessage() {
return message;
}

public boolean isOk() {
return statusCode == SftpConstants.SSH_FX_OK;
}

@Override
public String toString() {
return "SSH_FXP_STATUS[" + SftpConstants.getStatusName(statusCode) + " ,language=" + language + " ,message=" + message
+ ']';
}

public static SftpStatus parse(Buffer buffer) {
int code = buffer.getInt();
// Treat the message and language tag as optional. These fields did not exist in SFTP v0-2, and there are
// apparently SFTP v3 servers that sometimes send SSH_FXP_STATUS without them.
String message = buffer.available() > 0 ? buffer.getString() : null;
String language = buffer.available() > 0 ? buffer.getString() : null;
return new SftpStatus(code, message, language);
}
}
Loading

0 comments on commit 95526ce

Please sign in to comment.