Skip to content

Commit

Permalink
Use FILE_OPEN_IF if no CreateDisposition provided (Fixes #786) (#798)
Browse files Browse the repository at this point in the history
* Use FILE_OPEN_IF if no CreateDisposition provided (Fixes #786)

Signed-off-by: Jeroen van Erp <jeroen@hierynomus.com>

* Added license header

---------

Signed-off-by: Jeroen van Erp <jeroen@hierynomus.com>
  • Loading branch information
hierynomus committed Sep 25, 2023
1 parent a50bc41 commit d549dda
Show file tree
Hide file tree
Showing 7 changed files with 132 additions and 55 deletions.
6 changes: 6 additions & 0 deletions README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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)
Expand Down Expand Up @@ -95,4 +95,8 @@ protected void writeTo(SMBBuffer buffer) {

buffer.putRawBytes(nameBytes);
}

public SMB2CreateDisposition getCreateDisposition() {
return createDisposition;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
43 changes: 0 additions & 43 deletions src/test/groovy/com/hierynomus/smbj/session/SessionSpec.groovy

This file was deleted.

11 changes: 1 addition & 10 deletions src/test/java/com/hierynomus/smbj/connection/ConnectionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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());
Expand Down
84 changes: 84 additions & 0 deletions src/test/java/com/hierynomus/smbj/session/SessionTest.java
Original file line number Diff line number Diff line change
@@ -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);

}
}
27 changes: 27 additions & 0 deletions src/test/java/com/hierynomus/smbj/testing/Utils.java
Original file line number Diff line number Diff line change
@@ -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();
}
}

0 comments on commit d549dda

Please sign in to comment.