Use FILE_OPEN_IF if no CreateDisposition provided (Fixes #786) (#798)

* Use FILE_OPEN_IF if no CreateDisposition provided (Fixes #786) Signed-off-by: Jeroen van Erp <[email protected]> * Added license header --------- Signed-off-by: Jeroen van Erp <[email protected]>
hierynomus · Sep 25, 2023 · d549dda · d549dda
1 parent a50bc41
commit d549dda
Show file tree

Hide file tree

Showing 7 changed files with 132 additions and 55 deletions.
diff --git a/README.adoc b/README.adoc
@@ -158,6 +158,12 @@ The implementation is based on the following specifications:
 
 == Changelog
 
+=== 0.13.0 <-- UPCOMING
+* Use FILE_OPEN_IF if no SMB2CreateDisposition provided (Fixes https://github.com/hierynomus/smbj/issues/786[#786])
+* Integration tests rewritten to JUnit Jupiter and TestContainers
+* Use BufferedInputStream to read Socket (Merged https://github.com/hierynomus/smbj/issues/795[#795])
+* Clear serverList when last connection closed (Merged https://github.com/hierynomus/smbj/issues/719[#719])
+
 === 0.12.2 (2023-08-07)
 * Added missing applicationKey getter to SessionContext (Merged https://github.com/hierynomus/smbj/pulls/776[#776])
 * Fix error with anonymous authentication (Fixes https://github.com/hierynomus/smbj/issues/779[#779])

diff --git a/src/main/java/com/hierynomus/mssmb2/messages/SMB2CreateRequest.java b/src/main/java/com/hierynomus/mssmb2/messages/SMB2CreateRequest.java
@@ -53,7 +53,7 @@ public SMB2CreateRequest(SMB2Dialect smbDialect,
         this.accessMask = accessMask;
         this.fileAttributes = ensureNotNull(fileAttributes, FileAttributes.class);
         this.shareAccess = ensureNotNull(shareAccess, SMB2ShareAccess.class);
-        this.createDisposition = ensureNotNull(createDisposition, SMB2CreateDisposition.FILE_SUPERSEDE);
+        this.createDisposition = ensureNotNull(createDisposition, SMB2CreateDisposition.FILE_OPEN_IF);
         this.createOptions = ensureNotNull(createOptions, SMB2CreateOptions.class);
         this.path = path;
     }
@@ -62,7 +62,7 @@ public SMB2CreateRequest(SMB2Dialect smbDialect,
     protected void writeTo(SMBBuffer buffer) {
         buffer.putUInt16(structureSize); // StructureSize (2 bytes)
         buffer.putByte((byte) 0); // SecurityFlags (1 byte) - Reserved
-        buffer.putByte((byte) 0);  // RequestedOpLockLevel (1 byte) - None
+        buffer.putByte((byte) 0); // RequestedOpLockLevel (1 byte) - None
         buffer.putUInt32(impersonationLevel.getValue()); // ImpersonationLevel (4 bytes) - Identification
         buffer.putReserved(8); // SmbCreateFlags (8 bytes)
         buffer.putReserved(8); // Reserved (8 bytes)
@@ -95,4 +95,8 @@ protected void writeTo(SMBBuffer buffer) {
 
         buffer.putRawBytes(nameBytes);
     }
+
+    public SMB2CreateDisposition getCreateDisposition() {
+        return createDisposition;
+    }
 }
diff --git a/src/main/java/com/hierynomus/mssmb2/messages/SMB2CreateResponse.java b/src/main/java/com/hierynomus/mssmb2/messages/SMB2CreateResponse.java
@@ -90,4 +90,12 @@ public Set<FileAttributes> getFileAttributes() {
     public SMB2FileId getFileId() {
         return fileId;
     }
+
+    public void setFileAttributes(Set<FileAttributes> fileAttributes) {
+        this.fileAttributes = fileAttributes;
+    }
+
+    public void setFileId(SMB2FileId fileId) {
+        this.fileId = fileId;
+    }
 }
diff --git a/src/test/groovy/com/hierynomus/smbj/session/SessionSpec.groovy b/src/test/groovy/com/hierynomus/smbj/session/SessionSpec.groovy
diff --git a/src/test/java/com/hierynomus/smbj/connection/ConnectionTest.java b/src/test/java/com/hierynomus/smbj/connection/ConnectionTest.java
@@ -18,6 +18,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static com.hierynomus.smbj.testing.Utils.*;
 
 import java.util.UUID;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -33,20 +34,10 @@
 import com.hierynomus.mssmb2.messages.SMB2NegotiateResponse;
 import com.hierynomus.smbj.SMBClient;
 import com.hierynomus.smbj.SmbConfig;
-import com.hierynomus.smbj.testing.PacketProcessor;
-import com.hierynomus.smbj.testing.StubAuthenticator;
-import com.hierynomus.smbj.testing.StubTransportLayerFactory;
-import com.hierynomus.smbj.testing.PacketProcessor.DefaultPacketProcessor;
 import com.hierynomus.smbj.testing.PacketProcessor.NoOpPacketProcessor;;
 
 public class ConnectionTest {
 
-    private static SmbConfig config(PacketProcessor processor) {
-        return SmbConfig.builder()
-                .withTransportLayerFactory(new StubTransportLayerFactory<>(new DefaultPacketProcessor().wrap(processor)))
-                .withAuthenticators(new StubAuthenticator.Factory()).build();
-    }
-
     @Test
     public void shouldUnregisterServerWhenConnectionClosed() throws Exception {
         SmbConfig config = config(new NoOpPacketProcessor());

diff --git a/src/test/java/com/hierynomus/smbj/session/SessionTest.java b/src/test/java/com/hierynomus/smbj/session/SessionTest.java
@@ -0,0 +1,84 @@
+/*
+ * Copyright (C)2016 - SMBJ Contributors
+ *
+ * Licensed 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 com.hierynomus.smbj.session;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.isA;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.EnumSet;
+import java.util.concurrent.Future;
+
+import org.junit.jupiter.api.Test;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+
+import com.hierynomus.msfscc.FileAttributes;
+import com.hierynomus.mssmb2.SMB2CreateDisposition;
+import com.hierynomus.mssmb2.SMB2Dialect;
+import com.hierynomus.mssmb2.SMB2ShareAccess;
+import com.hierynomus.mssmb2.messages.SMB2CreateRequest;
+import com.hierynomus.mssmb2.messages.SMB2CreateResponse;
+import com.hierynomus.protocol.commons.concurrent.Promise;
+import com.hierynomus.protocol.transport.TransportException;
+import com.hierynomus.smbj.SmbConfig;
+import com.hierynomus.smbj.common.SmbPath;
+import com.hierynomus.smbj.connection.Connection;
+import com.hierynomus.smbj.connection.NegotiatedProtocol;
+import com.hierynomus.smbj.event.SMBEventBus;
+import com.hierynomus.smbj.paths.PathResolver;
+import com.hierynomus.smbj.share.DiskShare;
+import com.hierynomus.smbj.share.File;
+import com.hierynomus.smbj.share.TreeConnect;
+
+public class SessionTest {
+    @Test
+    public void shareNameCannotContainBackslashes() {
+        SmbConfig cfg = SmbConfig.createDefaultConfig();
+        Connection c = mock(Connection.class);
+        Session s = new Session(c, cfg, null, mock(SMBEventBus.class), null, null, null);
+        Exception ex = assertThrows(IllegalArgumentException.class, () -> s.connectShare("foo\\bar"));
+        assertThat(ex.getMessage()).contains("foo\\bar");
+    }
+
+    @Test
+    public void shouldUseOpenIfIfNoCreateDispositionProvided() throws Exception {
+        SmbConfig cfg = SmbConfig.createDefaultConfig();
+        Session s = mock(Session.class);
+        TreeConnect tc = mock(TreeConnect.class);
+        when(tc.getSession()).thenReturn(s);
+        when(tc.getConfig()).thenReturn(cfg);
+        when(tc.getNegotiatedProtocol())
+                .thenReturn(new NegotiatedProtocol(SMB2Dialect.SMB_2_0_2, 1024, 1024, 1024, true));
+        DiskShare share = new DiskShare(new SmbPath("localhost", "public"), tc, PathResolver.LOCAL);
+        when(s.send(isA(SMB2CreateRequest.class))).thenAnswer(new Answer<Future<SMB2CreateResponse>>() {
+            @Override
+            public Future<SMB2CreateResponse> answer(InvocationOnMock invocation) throws Throwable {
+                SMB2CreateRequest req = (SMB2CreateRequest) invocation.getArguments()[0];
+                assertThat(req.getCreateDisposition()).isEqualTo(SMB2CreateDisposition.FILE_OPEN_IF);
+                Promise<SMB2CreateResponse, TransportException> p = new Promise<>("foo", TransportException.Wrapper);
+                SMB2CreateResponse resp = new SMB2CreateResponse();
+                resp.setFileAttributes(EnumSet.of(FileAttributes.FILE_ATTRIBUTE_NORMAL));
+                p.deliver(resp);
+                return p.future();
+            }
+        });
+        File f = share.openFile("foo", null, null, SMB2ShareAccess.ALL, null, null);
+
+    }
+}
diff --git a/src/test/java/com/hierynomus/smbj/testing/Utils.java b/src/test/java/com/hierynomus/smbj/testing/Utils.java
@@ -0,0 +1,27 @@
+/*
+ * Copyright (C)2016 - SMBJ Contributors
+ *
+ * Licensed 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 com.hierynomus.smbj.testing;
+
+import com.hierynomus.smbj.SmbConfig;
+import com.hierynomus.smbj.testing.PacketProcessor.DefaultPacketProcessor;
+
+public class Utils {
+    public static SmbConfig config(PacketProcessor processor) {
+        return SmbConfig.builder()
+                .withTransportLayerFactory(new StubTransportLayerFactory<>(new DefaultPacketProcessor().wrap(processor)))
+                .withAuthenticators(new StubAuthenticator.Factory()).build();
+    }
+}