Skip to content

Commit 9e157a5

Browse files
benrr101benrr101
committed
Improving open file stream code
1 parent 22601b8 commit 9e157a5

File tree

1 file changed

+81
-75
lines changed

1 file changed

+81
-75
lines changed

‎src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlTypes/SqlFileStream.Windows.cs‎

Lines changed: 81 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,11 @@ public sealed class SqlFileStream : Stream
4343
/// </remarks>
4444
private const int DefaultBufferSize = 1;
4545

46+
private const FileOptions AllowedOptions = FileOptions.WriteThrough |
47+
FileOptions.Asynchronous |
48+
FileOptions.RandomAccess |
49+
FileOptions.SequentialScan;
50+
4651
private const ushort IoControlCodeFunctionCode = 2392;
4752

4853
/// <summary>
@@ -719,12 +724,14 @@ private static FileStream OpenFileStream(SafeFileHandle fileHandle, FileAccess a
719724
sp.Assert();
720725
bRevertAssert = true;
721726

722-
return new FileStream(fileHandle, access, DefaultBufferSize, (options & FileOptions.Asynchronous) != 0);
727+
return new FileStream(fileHandle, access, DefaultBufferSize,(options & FileOptions.Asynchronous) != 0);
723728
}
724729
finally
725730
{
726731
if (bRevertAssert)
727-
SecurityPermission.RevertAssert();
732+
{
733+
CodeAccessPermission.RevertAssert();
734+
}
728735
}
729736
#else
730737
return new FileStream(fileHandle, access, DefaultBufferSize, (options & FileOptions.Asynchronous) != 0);
@@ -738,132 +745,129 @@ private void OpenSqlFileStream(
738745
FileOptions options,
739746
long allocationSize)
740747
{
741-
//-----------------------------------------------------------------
742-
// precondition validation
743-
744-
// these should be checked by any caller of this method
745-
// ensure we have validated and normalized the path before
746-
Debug.Assert(path != null);
747-
Debug.Assert(transactionContext != null);
748-
749-
if (access != FileAccess.Read && access != FileAccess.Write && access != FileAccess.ReadWrite)
748+
if (access is not (FileAccess.Read or FileAccess.Write or FileAccess.ReadWrite))
749+
{
750750
throw ADP.ArgumentOutOfRange("access");
751+
}
751752

752-
// FileOptions is a set of flags, so AND the given value against the set of values we do not support
753-
if ((options & ~(FileOptions.WriteThrough | FileOptions.Asynchronous | FileOptions.RandomAccess | FileOptions.SequentialScan)) != 0)
753+
if ((options & ~AllowedOptions) != 0)
754+
{
754755
throw ADP.ArgumentOutOfRange("options");
755-
756-
// normalize the provided path
757-
path = GetFullPathInternal(path);
756+
}
758757

759758
#if NETFRAMEWORK
760759
// Ensure the running code has permission to read/write the file
761760
DemandAccessPermission(path, access);
762761
#endif
763762

764-
SafeFileHandle hFile = null;
765-
Interop.NtDll.DesiredAccess nDesiredAccess = Interop.NtDll.DesiredAccess.FILE_READ_ATTRIBUTES | Interop.NtDll.DesiredAccess.SYNCHRONIZE;
766-
Interop.NtDll.CreateOptions dwCreateOptions = 0;
767-
Interop.NtDll.CreateDisposition dwCreateDisposition = 0;
768-
FileShare shareAccess = FileShare.None;
763+
Interop.NtDll.CreateOptions createOptions = 0;
764+
Interop.NtDll.CreateDisposition createDisposition = 0;
765+
Interop.NtDll.DesiredAccess desiredAccess = Interop.NtDll.DesiredAccess.FILE_READ_ATTRIBUTES |
766+
Interop.NtDll.DesiredAccess.SYNCHRONIZE;
767+
FileShare shareAccess = 0;
769768

770769
switch (access)
771770
{
772771
case FileAccess.Read:
773-
nDesiredAccess |= Interop.NtDll.DesiredAccess.FILE_READ_DATA;
772+
desiredAccess |= Interop.NtDll.DesiredAccess.FILE_READ_DATA;
774773
shareAccess = FileShare.Delete | FileShare.ReadWrite;
775-
dwCreateDisposition = Interop.NtDll.CreateDisposition.FILE_OPEN;
774+
createDisposition = Interop.NtDll.CreateDisposition.FILE_OPEN;
776775
break;
777776

778777
case FileAccess.Write:
779-
nDesiredAccess |= Interop.NtDll.DesiredAccess.FILE_WRITE_DATA;
778+
desiredAccess |= Interop.NtDll.DesiredAccess.FILE_WRITE_DATA;
780779
shareAccess = FileShare.Delete | FileShare.Read;
781-
dwCreateDisposition = Interop.NtDll.CreateDisposition.FILE_OVERWRITE;
780+
createDisposition = Interop.NtDll.CreateDisposition.FILE_OVERWRITE;
782781
break;
783782

784783
case FileAccess.ReadWrite:
785-
default:
786-
// we validate the value of 'access' parameter in the beginning of this method
787-
Debug.Assert(access == FileAccess.ReadWrite);
788-
789-
nDesiredAccess |= Interop.NtDll.DesiredAccess.FILE_READ_DATA | Interop.NtDll.DesiredAccess.FILE_WRITE_DATA;
784+
desiredAccess |= Interop.NtDll.DesiredAccess.FILE_READ_DATA |
785+
Interop.NtDll.DesiredAccess.FILE_WRITE_DATA;
790786
shareAccess = FileShare.Delete | FileShare.Read;
791-
dwCreateDisposition = Interop.NtDll.CreateDisposition.FILE_OVERWRITE;
787+
createDisposition = Interop.NtDll.CreateDisposition.FILE_OVERWRITE;
792788
break;
789+
790+
// Note: default case is heuristically unreachable due to check above.
793791
}
794792

795793
if ((options & FileOptions.WriteThrough) != 0)
796794
{
797-
dwCreateOptions |= Interop.NtDll.CreateOptions.FILE_WRITE_THROUGH;
795+
createOptions |= Interop.NtDll.CreateOptions.FILE_WRITE_THROUGH;
798796
}
799797

800798
if ((options & FileOptions.Asynchronous) == 0)
801799
{
802-
dwCreateOptions |= Interop.NtDll.CreateOptions.FILE_SYNCHRONOUS_IO_NONALERT;
800+
createOptions |= Interop.NtDll.CreateOptions.FILE_SYNCHRONOUS_IO_NONALERT;
803801
}
804802

805803
if ((options & FileOptions.SequentialScan) != 0)
806804
{
807-
dwCreateOptions |= Interop.NtDll.CreateOptions.FILE_SEQUENTIAL_ONLY;
805+
createOptions |= Interop.NtDll.CreateOptions.FILE_SEQUENTIAL_ONLY;
808806
}
809807

810808
if ((options & FileOptions.RandomAccess) != 0)
811809
{
812-
dwCreateOptions |= Interop.NtDll.CreateOptions.FILE_RANDOM_ACCESS;
810+
createOptions |= Interop.NtDll.CreateOptions.FILE_RANDOM_ACCESS;
813811
}
814812

813+
SafeFileHandle fileHandle = null;
815814
try
816815
{
817816
// NOTE: the Name property is intended to reveal the publicly available moniker for the
818817
// FILESTREAM attributed column data. We will not surface the internal processing that
819818
// takes place to create the mappedPath.
820819
string mappedPath = InitializeNtPath(path);
821-
int retval = 0;
822-
IntPtr handle;
823820

824821
Interop.Kernel32.SetThreadErrorMode(Interop.Kernel32.SEM_FAILCRITICALERRORS, out uint oldMode);
825822

823+
// Make the interop call to open the file
824+
int retval;
826825
try
827826
{
828827
if (transactionContext.Length >= ushort.MaxValue)
828+
{
829829
throw ADP.ArgumentOutOfRange("transactionContext");
830+
}
831+
832+
IntPtr handle;
830833

831834
#if NETFRAMEWORK
832-
string traceEventMessage = "<sc.SqlFileStream.OpenSqlFileStream|ADV> {0}, desiredAccess=0x{1}, allocationSize={2}, fileAttributes=0x00, shareAccess=0x{3}, dwCreateDisposition=0x{4}, createOptions=0x{5}";
835+
const string traceEventMessage = "<sc.SqlFileStream.OpenSqlFileStream|ADV> {0}, desiredAccess=0x{1}, allocationSize={2}, fileAttributes=0x00, shareAccess=0x{3}, dwCreateDisposition=0x{4}, createOptions=0x{5}";
833836
(retval, handle) = Interop.NtDll.CreateFile(
834837
path: mappedPath,
835838
eaName: EaNameString,
836839
eaValue: transactionContext,
837-
desiredAccess: nDesiredAccess,
840+
desiredAccess: desiredAccess,
838841
fileAttributes: 0,
839842
shareAccess: FileShare.None,
840-
createDisposition: dwCreateDisposition,
841-
createOptions: dwCreateOptions,
843+
createDisposition: createDisposition,
844+
createOptions: createOptions,
842845
impersonationLevel: Interop.ImpersonationLevel.SecurityAnonymous,
843846
isDynamicTracking: false,
844847
isEffectiveOnly: false);
845848
#else
846-
string traceEventMessage = "SqlFileStream.OpenSqlFileStream | ADV | Object Id {0}, Desired Access 0x{1}, Allocation Size {2}, File Attributes 0, Share Access 0x{3}, Create Disposition 0x{4}, Create Options 0x{5}";
849+
const string traceEventMessage = "SqlFileStream.OpenSqlFileStream | ADV | Object Id {0}, Desired Access 0x{1}, Allocation Size {2}, File Attributes 0, Share Access 0x{3}, Create Disposition 0x{4}, Create Options 0x{5}";
847850
(retval, handle) = Interop.NtDll.CreateFile(
848851
path: mappedPath,
849852
eaName: EaNameString,
850853
eaValue: transactionContext,
851-
desiredAccess: nDesiredAccess,
854+
desiredAccess: desiredAccess,
852855
fileAttributes: 0,
853856
shareAccess: FileShare.None,
854-
createDisposition: dwCreateDisposition,
855-
createOptions: dwCreateOptions);
857+
createDisposition: createDisposition,
858+
createOptions: createOptions);
856859
#endif
857860

858-
SqlClientEventSource.Log.TryAdvancedTraceEvent(traceEventMessage, _objectId, (int)nDesiredAccess, allocationSize, (int)shareAccess, dwCreateDisposition, dwCreateOptions);
861+
SqlClientEventSource.Log.TryAdvancedTraceEvent(traceEventMessage, _objectId, (int)desiredAccess, allocationSize, (int)shareAccess, createDisposition, createOptions);
859862

860-
hFile = new SafeFileHandle(handle, true);
863+
fileHandle = new SafeFileHandle(handle, true);
861864
}
862865
finally
863866
{
864867
Interop.Kernel32.SetThreadErrorMode(oldMode, out oldMode);
865868
}
866869

870+
// Handle error codes from the interop call
867871
switch (retval)
868872
{
869873
case 0:
@@ -876,65 +880,67 @@ private void OpenSqlFileStream(
876880
throw ADP.Argument(StringsHelper.GetString(Strings.SqlFileStream_InvalidParameter));
877881

878882
case Interop.Errors.ERROR_FILE_NOT_FOUND:
879-
{
880-
DirectoryNotFoundException e = new DirectoryNotFoundException();
881-
ADP.TraceExceptionAsReturnValue(e);
882-
throw e;
883-
}
883+
DirectoryNotFoundException dirNotFoundException = new DirectoryNotFoundException();
884+
ADP.TraceExceptionAsReturnValue(dirNotFoundException);
885+
throw dirNotFoundException;
886+
884887
default:
888+
uint error = Interop.NtDll.RtlNtStatusToDosError(retval);
889+
if (error == Interop.NtDll.ERROR_MR_MID_NOT_FOUND)
885890
{
886-
uint error = Interop.NtDll.RtlNtStatusToDosError(retval);
887-
if (error == Interop.NtDll.ERROR_MR_MID_NOT_FOUND)
888-
{
889-
// status code could not be mapped to a Win32 error code
890-
error = (uint)retval;
891-
}
892-
893-
Win32Exception e = new Win32Exception(unchecked((int)error));
894-
ADP.TraceExceptionAsReturnValue(e);
895-
throw e;
891+
// status code could not be mapped to a Win32 error code
892+
error = (uint)retval;
896893
}
894+
895+
Win32Exception win32Exception = new Win32Exception(unchecked((int)error));
896+
ADP.TraceExceptionAsReturnValue(win32Exception);
897+
throw win32Exception;
897898
}
898899

899-
if (hFile.IsInvalid)
900+
// Make sure the file handle is usable for us
901+
if (fileHandle.IsInvalid)
900902
{
901903
Win32Exception e = new Win32Exception(Interop.Errors.ERROR_INVALID_HANDLE);
902904
ADP.TraceExceptionAsReturnValue(e);
903905
throw e;
904906
}
905907

906-
if (Interop.Kernel32.GetFileType(hFile) != Interop.Kernel32.FileTypes.FILE_TYPE_DISK)
908+
if (Interop.Kernel32.GetFileType(fileHandle) != Interop.Kernel32.FileTypes.FILE_TYPE_DISK)
907909
{
908-
hFile.Dispose();
910+
fileHandle.Dispose();
909911
throw ADP.Argument(StringsHelper.GetString(Strings.SqlFileStream_PathNotValidDiskResource));
910912
}
911913

912914
// If the user is opening the SQL FileStream in read/write mode, we assume that
913915
// they want to scan through current data and then append new data to the end, so
914916
// we need to tell SQL Server to preserve the existing file contents.
915-
if (access == FileAccess.ReadWrite)
917+
if (access is FileAccess.ReadWrite)
916918
{
917-
uint ioControlCode = Interop.Kernel32.CTL_CODE(Interop.Kernel32.FILE_DEVICE_FILE_SYSTEM,
918-
IoControlCodeFunctionCode, (byte)Interop.Kernel32.IoControlTransferType.METHOD_BUFFERED,
919+
uint ioControlCode = Interop.Kernel32.CTL_CODE(
920+
Interop.Kernel32.FILE_DEVICE_FILE_SYSTEM,
921+
IoControlCodeFunctionCode,
922+
(byte)Interop.Kernel32.IoControlTransferType.METHOD_BUFFERED,
919923
(byte)Interop.Kernel32.IoControlCodeAccess.FILE_ANY_ACCESS);
920924

921-
if (!Interop.Kernel32.DeviceIoControl(hFile, ioControlCode, IntPtr.Zero, 0, IntPtr.Zero, 0, out uint cbBytesReturned, IntPtr.Zero))
925+
if (!Interop.Kernel32.DeviceIoControl(fileHandle, ioControlCode, IntPtr.Zero, 0, IntPtr.Zero, 0, out _, IntPtr.Zero))
922926
{
923927
Win32Exception e = new Win32Exception(Marshal.GetLastWin32Error());
924928
ADP.TraceExceptionAsReturnValue(e);
925929
throw e;
926930
}
927931
}
928932

929-
// now that we've successfully opened a handle on the path and verified that it is a file,
930-
// use the SafeFileHandle to initialize our internal FileStream instance
931-
Debug.Assert(_fileStream == null);
932-
_fileStream = OpenFileStream(hFile, access, options);
933+
// Now that we've successfully opened a handle on the path and verified that it is
934+
// a file, use the SafeFileHandle to initialize our internal FileStream instance.
935+
Debug.Assert(_fileStream is null);
936+
_fileStream = OpenFileStream(fileHandle, access, options);
933937
}
934938
catch
935939
{
936-
if (hFile != null && !hFile.IsInvalid)
937-
hFile.Dispose();
940+
if (fileHandle is not null && !fileHandle.IsInvalid)
941+
{
942+
fileHandle.Dispose();
943+
}
938944

939945
throw;
940946
}

0 commit comments

Comments
 (0)