Repository: mina-sshd
Updated Branches:
  refs/heads/master d07089cea -> 5342179f5


[SSHD-807] SFTP should accept empty owner/group value when setting attributes


Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/5342179f
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/5342179f
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/5342179f

Branch: refs/heads/master
Commit: 5342179f57645e1d2737c8f0f3ccd09bc22f55ed
Parents: d07089c
Author: Lyor Goldstein <lyor.goldst...@gmail.com>
Authored: Tue Mar 6 21:12:06 2018 +0200
Committer: Lyor Goldstein <lyor.goldst...@gmail.com>
Committed: Tue Mar 6 21:12:33 2018 +0200

----------------------------------------------------------------------
 .../sshd/client/subsystem/sftp/SftpClient.java  | 40 +++++++++++++++-----
 .../subsystem/sftp/impl/AbstractSftpClient.java | 22 ++++++++---
 2 files changed, 48 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/5342179f/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/SftpClient.java
----------------------------------------------------------------------
diff --git 
a/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/SftpClient.java 
b/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/SftpClient.java
index c860cb4..43cc619 100644
--- 
a/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/SftpClient.java
+++ 
b/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/SftpClient.java
@@ -46,7 +46,6 @@ import org.apache.sshd.client.subsystem.SubsystemClient;
 import org.apache.sshd.client.subsystem.sftp.extensions.SftpClientExtension;
 import org.apache.sshd.common.subsystem.sftp.SftpConstants;
 import org.apache.sshd.common.subsystem.sftp.SftpHelper;
-import org.apache.sshd.common.subsystem.sftp.SftpUniversalOwnerAndGroup;
 import org.apache.sshd.common.util.GenericUtils;
 import org.apache.sshd.common.util.ValidateUtils;
 import org.apache.sshd.common.util.buffer.BufferUtils;
@@ -245,6 +244,11 @@ public interface SftpClient extends SubsystemClient {
             return this;
         }
 
+        public Attributes removeFlag(Attribute flag) {
+            flags.remove(flag);
+            return this;
+        }
+
         public int getType() {
             return type;
         }
@@ -277,10 +281,19 @@ public interface SftpClient extends SubsystemClient {
         }
 
         public void setOwner(String owner) {
-            this.owner = ValidateUtils.checkNotNullAndNotEmpty(owner, "No 
owner");
-            addFlag(Attribute.OwnerGroup);
-            if (GenericUtils.isEmpty(getGroup())) {
-                setGroup(SftpUniversalOwnerAndGroup.Group.getName());
+            this.owner = owner;
+            /*
+             * According to 
https://tools.ietf.org/wg/secsh/draft-ietf-secsh-filexfer/draft-ietf-secsh-filexfer-13.txt
+             * section 7.5
+             *
+             *      If either the owner or group field is zero length, the 
field
+             *      should be considered absent, and no change should be made 
to
+             *      that specific field during a modification operation.
+             */
+            if (GenericUtils.isEmpty(owner)) {
+                removeFlag(Attribute.OwnerGroup);
+            } else {
+                addFlag(Attribute.OwnerGroup);
             }
         }
 
@@ -294,10 +307,19 @@ public interface SftpClient extends SubsystemClient {
         }
 
         public void setGroup(String group) {
-            this.group = ValidateUtils.checkNotNullAndNotEmpty(group, "No 
group");
-            addFlag(Attribute.OwnerGroup);
-            if (GenericUtils.isEmpty(getOwner())) {
-                setOwner(SftpUniversalOwnerAndGroup.Owner.getName());
+            this.group = group;
+            /*
+             * According to 
https://tools.ietf.org/wg/secsh/draft-ietf-secsh-filexfer/draft-ietf-secsh-filexfer-13.txt
+             * section 7.5
+             *
+             *      If either the owner or group field is zero length, the 
field
+             *      should be considered absent, and no change should be made 
to
+             *      that specific field during a modification operation.
+             */
+            if (GenericUtils.isEmpty(group)) {
+                removeFlag(Attribute.OwnerGroup);
+            } else {
+                addFlag(Attribute.OwnerGroup);
             }
         }
 

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/5342179f/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/impl/AbstractSftpClient.java
----------------------------------------------------------------------
diff --git 
a/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/impl/AbstractSftpClient.java
 
b/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/impl/AbstractSftpClient.java
index 681c07d..89c73b9 100644
--- 
a/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/impl/AbstractSftpClient.java
+++ 
b/sshd-core/src/main/java/org/apache/sshd/client/subsystem/sftp/impl/AbstractSftpClient.java
@@ -43,7 +43,6 @@ import org.apache.sshd.common.channel.Channel;
 import org.apache.sshd.common.subsystem.sftp.SftpConstants;
 import org.apache.sshd.common.subsystem.sftp.SftpException;
 import org.apache.sshd.common.subsystem.sftp.SftpHelper;
-import org.apache.sshd.common.subsystem.sftp.SftpUniversalOwnerAndGroup;
 import org.apache.sshd.common.subsystem.sftp.extensions.ParserUtils;
 import org.apache.sshd.common.util.GenericUtils;
 import org.apache.sshd.common.util.ValidateUtils;
@@ -528,9 +527,22 @@ public abstract class AbstractSftpClient extends 
AbstractSubsystemClient impleme
                     case Size:
                         flagsMask |= SftpConstants.SSH_FILEXFER_ATTR_SIZE;
                         break;
-                    case OwnerGroup:
-                        flagsMask |= 
SftpConstants.SSH_FILEXFER_ATTR_OWNERGROUP;
+                    case OwnerGroup: {
+                        /*
+                         * According to 
https://tools.ietf.org/wg/secsh/draft-ietf-secsh-filexfer/draft-ietf-secsh-filexfer-13.txt
+                         * section 7.5
+                         *
+                         *      If either the owner or group field is zero 
length, the field
+                         *      should be considered absent, and no change 
should be made to
+                         *      that specific field during a modification 
operation.
+                         */
+                        String owner = attributes.getOwner();
+                        String group = attributes.getGroup();
+                        if (GenericUtils.isNotEmpty(owner) && 
GenericUtils.isNotEmpty(group)) {
+                            flagsMask |= 
SftpConstants.SSH_FILEXFER_ATTR_OWNERGROUP;
+                        }
                         break;
+                    }
                     case Perms:
                         flagsMask |= 
SftpConstants.SSH_FILEXFER_ATTR_PERMISSIONS;
                         break;
@@ -559,10 +571,10 @@ public abstract class AbstractSftpClient extends 
AbstractSubsystemClient impleme
             }
             if ((flagsMask & SftpConstants.SSH_FILEXFER_ATTR_OWNERGROUP) != 0) 
{
                 String owner = attributes.getOwner();
-                buffer.putString(GenericUtils.isEmpty(owner) ? 
SftpUniversalOwnerAndGroup.Owner.getName() : owner);
+                buffer.putString(owner);
 
                 String group = attributes.getGroup();
-                buffer.putString(GenericUtils.isEmpty(group) ? 
SftpUniversalOwnerAndGroup.Group.getName() : group);
+                buffer.putString(group);
             }
             if ((flagsMask & SftpConstants.SSH_FILEXFER_ATTR_PERMISSIONS) != 
0) {
                 buffer.putInt(attributes.getPermissions());

Reply via email to