Remove exposing windows.h header from our public headers and reduce risk of introducing it in non-public headers (#2188)

Fixes https://github.com/Azure/azure-sdk-for-cpp/issues/1853

sdk/core/azure-core/CMakeLists.txt

@@ -57,7 +57,6 @@ set(
     inc/azure/core/internal/client_options.hpp
     inc/azure/core/internal/contract.hpp
     inc/azure/core/internal/diagnostics/log.hpp
-    inc/azure/core/internal/hkey_holder.hpp
     inc/azure/core/internal/http/pipeline.hpp
     inc/azure/core/internal/io/null_body_stream.hpp
     inc/azure/core/internal/json/json_serializable.hpp

sdk/core/azure-core/inc/azure/core/http/win_http_transport.hpp

@@ -12,6 +12,18 @@
 #include "azure/core/http/http.hpp"
 #include "azure/core/http/transport.hpp"
 
+#include <azure/core/platform.hpp>
+
+#if defined(AZ_PLATFORM_WINDOWS)
+#if !defined(WIN32_LEAN_AND_MEAN)
+#define WIN32_LEAN_AND_MEAN
+#endif
+#if !defined(NOMINMAX)
+#define NOMINMAX
+#endif
+#include <windows.h>
+#endif
+
 #include <type_traits>
 #include <vector>
 #include <winhttp.h>

sdk/core/azure-core/inc/azure/core/internal/hkey_holder.hpp

@@ -1,65 +0,0 @@
-// Copyright (c) Microsoft Corporation. All rights reserved.
-// SPDX-License-Identifier: MIT
-
-/**
- * @file
- * @brief Internal HKEY holder.
- *
- */
-#pragma once
-
-#include "azure/core/platform.hpp"
-
-#if defined(AZ_PLATFORM_WINDOWS)
-#if !defined(WIN32_LEAN_AND_MEAN)
-#define WIN32_LEAN_AND_MEAN
-#endif
-#if !defined(NOMINMAX)
-#define NOMINMAX
-#endif
-
-#include <windows.h>
-
-#if !defined(WINAPI_PARTITION_DESKTOP) \
-    || WINAPI_PARTITION_DESKTOP // See azure/core/platform.hpp for explanation.
-
-namespace Azure { namespace Core { namespace _internal {
-
-  /**
-   * @brief HkeyHolder ensures native handle resource is released.
-   */
-  class HkeyHolder {
-  private:
-    HKEY m_value = nullptr;
-
-  public:
-    explicit HkeyHolder() noexcept : m_value(nullptr){};
-
-    ~HkeyHolder() noexcept
-    {
-      if (m_value != nullptr)
-      {
-        ::RegCloseKey(m_value);
-      }
-    }
-
-    void operator=(HKEY p) noexcept
-    {
-      if (p != nullptr)
-      {
-        m_value = p;
-      }
-    }
-
-    operator HKEY() noexcept { return m_value; }
-
-    operator HKEY*() noexcept { return &m_value; }
-
-    HKEY* operator&() noexcept { return &m_value; }
-  };
-
-}}} // namespace Azure::Core::_internal
-
-#endif
-
-#endif

sdk/core/azure-core/inc/azure/core/io/body_stream.hpp

@@ -12,14 +12,6 @@
 
 #if defined(AZ_PLATFORM_POSIX)
 #include <unistd.h>
-#elif defined(AZ_PLATFORM_WINDOWS)
-#if !defined(WIN32_LEAN_AND_MEAN)
-#define WIN32_LEAN_AND_MEAN
-#endif
-#if !defined(NOMINMAX)
-#define NOMINMAX
-#endif
-#include <windows.h>
 #endif
 
 #include "azure/core/context.hpp"
@@ -171,7 +163,7 @@ namespace Azure { namespace Core { namespace IO {
 #if defined(AZ_PLATFORM_POSIX)
       int m_fileDescriptor;
 #elif defined(AZ_PLATFORM_WINDOWS)
-      HANDLE m_filehandle;
+      void* m_filehandle;
 #endif
       int64_t m_baseOffset;
       int64_t m_length;
@@ -220,7 +212,7 @@ namespace Azure { namespace Core { namespace IO {
        * for the necessary duration. The caller is also responsible for closing it once they are
        * done.
        */
-      RandomAccessFileBodyStream(HANDLE fileHandle, int64_t offset, int64_t length)
+      RandomAccessFileBodyStream(void* fileHandle, int64_t offset, int64_t length)
           : m_filehandle(fileHandle), m_baseOffset(offset), m_length(length), m_offset(0)
       {
       }
@@ -246,7 +238,7 @@ namespace Azure { namespace Core { namespace IO {
   private:
     // immutable
 #if defined(AZ_PLATFORM_WINDOWS)
-    HANDLE m_filehandle;
+    void* m_filehandle;
 #elif defined(AZ_PLATFORM_POSIX)
     int m_fileDescriptor;
 #endif

sdk/core/azure-core/src/http/curl/curl.cpp

@@ -17,6 +17,12 @@
 #include <poll.h> // for poll()
 #include <sys/socket.h> // for socket shutdown
 #elif defined(AZ_PLATFORM_WINDOWS)
+#if !defined(WIN32_LEAN_AND_MEAN)
+#define WIN32_LEAN_AND_MEAN
+#endif
+#if !defined(NOMINMAX)
+#define NOMINMAX
+#endif
 #include <winsock2.h> // for WSAPoll();
 #endif
 
@@ -803,7 +809,7 @@ int64_t CurlSession::OnRead(uint8_t* buffer, int64_t count, Context const& conte
 
   auto totalRead = int64_t();
   auto readRequestLength = this->m_isChunkedResponseType
-      ? std::min(this->m_chunkSize - this->m_sessionTotalRead, count)
+      ? (std::min)(this->m_chunkSize - this->m_sessionTotalRead, count)
       : count;
 
   // For responses with content-length, avoid trying to read beyond Content-length or
@@ -812,7 +818,7 @@ int64_t CurlSession::OnRead(uint8_t* buffer, int64_t count, Context const& conte
   if (this->m_contentLength > 0)
   {
     auto remainingBodyContent = this->m_contentLength - this->m_sessionTotalRead;
-    readRequestLength = std::min(readRequestLength, remainingBodyContent);
+    readRequestLength = (std::min)(readRequestLength, remainingBodyContent);
   }
 
   // Take data from inner buffer if any

sdk/core/azure-core/src/http/telemetry_policy.cpp

@@ -17,7 +17,47 @@
 
 #include <windows.h>
 
-#include "azure/core/internal/hkey_holder.hpp"
+#if !defined(WINAPI_PARTITION_DESKTOP) \
+    || WINAPI_PARTITION_DESKTOP // See azure/core/platform.hpp for explanation.
+
+namespace Azure { namespace Core { namespace _internal {
+
+  /**
+   * @brief HkeyHolder ensures native handle resource is released.
+   */
+  class HkeyHolder {
+  private:
+    HKEY m_value = nullptr;
+
+  public:
+    explicit HkeyHolder() noexcept : m_value(nullptr){};
+
+    ~HkeyHolder() noexcept
+    {
+      if (m_value != nullptr)
+      {
+        ::RegCloseKey(m_value);
+      }
+    }
+
+    void operator=(HKEY p) noexcept
+    {
+      if (p != nullptr)
+      {
+        m_value = p;
+      }
+    }
+
+    operator HKEY() noexcept { return m_value; }
+
+    operator HKEY*() noexcept { return &m_value; }
+
+    HKEY* operator&() noexcept { return &m_value; }
+  };
+
+}}} // namespace Azure::Core::_internal
+
+#endif
 
 #elif defined(AZ_PLATFORM_POSIX)
 #include <sys/utsname.h>

sdk/core/azure-core/src/io/body_stream.cpp

@@ -33,6 +33,12 @@
 using Azure::Core::Context;
 using namespace Azure::Core::IO;
 
+#if defined(AZ_PLATFORM_WINDOWS)
+// For an abundance of caution, adding this as a compile time check since we are using static_cast
+// between windows HANDLE and void* to avoid having windows.h headers exposed in public headers.
+static_assert(sizeof(void*) >= sizeof(HANDLE), "We must be able to cast HANDLE to void* and back.");
+#endif
+
 // Keep reading until buffer is all fill out of the end of stream content is reached
 int64_t BodyStream::ReadToCount(uint8_t* buffer, int64_t count, Context const& context)
 {
@@ -84,12 +90,12 @@ int64_t MemoryBodyStream::OnRead(uint8_t* buffer, int64_t count, Context const&
 FileBodyStream::FileBodyStream(const std::string& filename)
 {
 #if defined(AZ_PLATFORM_WINDOWS)
-
+  HANDLE fileHandle;
   try
   {
 #if !defined(WINAPI_PARTITION_DESKTOP) \
     || WINAPI_PARTITION_DESKTOP // See azure/core/platform.hpp for explanation.
-    m_filehandle = CreateFile(
+    fileHandle = CreateFile(
         filename.data(),
         GENERIC_READ,
         FILE_SHARE_READ,
@@ -99,7 +105,7 @@ FileBodyStream::FileBodyStream(const std::string& filename)
                                    // intended to be sequential from beginning to end.
         NULL);
 #else
-    m_filehandle = CreateFile2(
+    fileHandle = CreateFile2(
         std::wstring_convert<std::codecvt_utf8_utf16<wchar_t>>().from_bytes(filename).c_str(),
         GENERIC_READ,
         FILE_SHARE_READ,
@@ -107,21 +113,22 @@ FileBodyStream::FileBodyStream(const std::string& filename)
         NULL);
 #endif
 
-    if (m_filehandle == INVALID_HANDLE_VALUE)
+    if (fileHandle == INVALID_HANDLE_VALUE)
     {
       throw std::runtime_error("Failed to open file for reading. File name: '" + filename + "'");
     }
     LARGE_INTEGER fileSize;
-    if (!GetFileSizeEx(m_filehandle, &fileSize))
+    if (!GetFileSizeEx(fileHandle, &fileSize))
     {
       throw std::runtime_error("Failed to get size of file. File name: '" + filename + "'");
     }
+    m_filehandle = static_cast<void*>(fileHandle);
     m_randomAccessFileBodyStream = std::make_unique<_internal::RandomAccessFileBodyStream>(
         _internal::RandomAccessFileBodyStream(m_filehandle, 0, fileSize.QuadPart));
   }
   catch (std::exception&)
   {
-    CloseHandle(m_filehandle);
+    CloseHandle(fileHandle);
     throw;
   }
 
@@ -156,7 +163,7 @@ FileBodyStream::~FileBodyStream()
 #if defined(AZ_PLATFORM_WINDOWS)
   if (m_filehandle)
   {
-    CloseHandle(m_filehandle);
+    CloseHandle(static_cast<HANDLE>(m_filehandle));
     m_filehandle = NULL;
   }
 #elif defined(AZ_PLATFORM_POSIX)

sdk/core/azure-core/src/io/random_access_file_body_stream.cpp

@@ -52,8 +52,10 @@ int64_t RandomAccessFileBodyStream::OnRead(
   o.Offset = static_cast<DWORD>(this->m_baseOffset + this->m_offset);
   o.OffsetHigh = static_cast<DWORD>((this->m_baseOffset + this->m_offset) >> 32);
 
+  HANDLE fileHandle = static_cast<HANDLE>(this->m_filehandle);
+
   auto result = ReadFile(
-      this->m_filehandle,
+      fileHandle,
       buffer,
       // at most 4Gb to be read
       static_cast<DWORD>(std::min(

sdk/core/azure-core/src/private/environment_log_level_listener.hpp

@@ -13,6 +13,7 @@
 #define NOMINMAX
 #endif
 
+// This use of windows.h within the header is OK because the header is private and in source only.
 #include <windows.h>
 #endif
 

sdk/storage/azure-storage-blobs/src/block_blob_client.cpp

@@ -3,6 +3,18 @@
 
 #include "azure/storage/blobs/block_blob_client.hpp"
 
+#include <azure/core/platform.hpp>
+
+#if defined(AZ_PLATFORM_WINDOWS)
+#if !defined(WIN32_LEAN_AND_MEAN)
+#define WIN32_LEAN_AND_MEAN
+#endif
+#if !defined(NOMINMAX)
+#define NOMINMAX
+#endif
+#include <windows.h>
+#endif
+
 #include <azure/core/io/body_stream.hpp>
 #include <azure/storage/common/concurrent_transfer.hpp>
 #include <azure/storage/common/constants.hpp>

sdk/storage/azure-storage-common/inc/azure/storage/common/file_io.hpp

@@ -21,7 +21,7 @@
 namespace Azure { namespace Storage { namespace _internal {
 
 #if defined(AZ_PLATFORM_WINDOWS)
-  using FileHandle = HANDLE;
+  using FileHandle = void*;
 #elif defined(AZ_PLATFORM_POSIX)
   using FileHandle = int;
 #endif

sdk/storage/azure-storage-common/src/file_io.cpp

@@ -12,6 +12,16 @@
 #include <unistd.h>
 #endif
 
+#if defined(AZ_PLATFORM_WINDOWS)
+#if !defined(WIN32_LEAN_AND_MEAN)
+#define WIN32_LEAN_AND_MEAN
+#endif
+#if !defined(NOMINMAX)
+#define NOMINMAX
+#endif
+#include <windows.h>
+#endif
+
 #include <codecvt>
 #include <limits>
 #include <locale>
@@ -22,9 +32,11 @@ namespace Azure { namespace Storage { namespace _internal {
 #if defined(AZ_PLATFORM_WINDOWS)
   FileReader::FileReader(const std::string& filename)
   {
+    HANDLE fileHandle;
+
 #if !defined(WINAPI_PARTITION_DESKTOP) \
     || WINAPI_PARTITION_DESKTOP // See azure/core/platform.hpp for explanation.
-    m_handle = CreateFile(
+    fileHandle = CreateFile(
         filename.data(),
         GENERIC_READ,
         FILE_SHARE_READ,
@@ -33,35 +45,38 @@ namespace Azure { namespace Storage { namespace _internal {
         FILE_ATTRIBUTE_NORMAL,
         NULL);
 #else
-    m_handle = CreateFile2(
+    fileHandle = CreateFile2(
         std::wstring_convert<std::codecvt_utf8_utf16<wchar_t>>().from_bytes(filename).c_str(),
         GENERIC_READ,
         FILE_SHARE_READ,
         OPEN_EXISTING,
         NULL);
 #endif
-    if (m_handle == INVALID_HANDLE_VALUE)
+    if (fileHandle == INVALID_HANDLE_VALUE)
     {
       throw std::runtime_error("failed to open file");
     }
 
     LARGE_INTEGER fileSize;
-    BOOL ret = GetFileSizeEx(m_handle, &fileSize);
+    BOOL ret = GetFileSizeEx(fileHandle, &fileSize);
     if (!ret)
     {
-      CloseHandle(m_handle);
+      CloseHandle(fileHandle);
       throw std::runtime_error("failed to get size of file");
     }
+    m_handle = static_cast<void*>(fileHandle);
     m_fileSize = fileSize.QuadPart;
   }
 
-  FileReader::~FileReader() { CloseHandle(m_handle); }
+  FileReader::~FileReader() { CloseHandle(static_cast<HANDLE>(m_handle)); }
 
   FileWriter::FileWriter(const std::string& filename)
   {
+    HANDLE fileHandle;
+
 #if !defined(WINAPI_PARTITION_DESKTOP) \
     || WINAPI_PARTITION_DESKTOP // See azure/core/platform.hpp for explanation.
-    m_handle = CreateFile(
+    fileHandle = CreateFile(
         filename.data(),
         GENERIC_WRITE,
         FILE_SHARE_READ | FILE_SHARE_WRITE,
@@ -70,20 +85,21 @@ namespace Azure { namespace Storage { namespace _internal {
         FILE_ATTRIBUTE_NORMAL,
         NULL);
 #else
-    m_handle = CreateFile2(
+    fileHandle = CreateFile2(
         std::wstring_convert<std::codecvt_utf8_utf16<wchar_t>>().from_bytes(filename).c_str(),
         GENERIC_WRITE,
         FILE_SHARE_READ | FILE_SHARE_WRITE,
         CREATE_ALWAYS,
         NULL);
 #endif
-    if (m_handle == INVALID_HANDLE_VALUE)
+    if (fileHandle == INVALID_HANDLE_VALUE)
     {
       throw std::runtime_error("failed to open file");
     }
+    m_handle = static_cast<void*>(fileHandle);
   }
 
-  FileWriter::~FileWriter() { CloseHandle(m_handle); }
+  FileWriter::~FileWriter() { CloseHandle(static_cast<HANDLE>(m_handle)); }
 
   void FileWriter::Write(const uint8_t* buffer, int64_t length, int64_t offset)
   {
@@ -98,7 +114,12 @@ namespace Azure { namespace Storage { namespace _internal {
     overlapped.OffsetHigh = static_cast<DWORD>(static_cast<uint64_t>(offset) >> 32);
 
     DWORD bytesWritten;
-    BOOL ret = WriteFile(m_handle, buffer, static_cast<DWORD>(length), &bytesWritten, &overlapped);
+    BOOL ret = WriteFile(
+        static_cast<HANDLE>(m_handle),
+        buffer,
+        static_cast<DWORD>(length),
+        &bytesWritten,
+        &overlapped);
     if (!ret)
     {
       throw std::runtime_error("failed to write file");

sdk/storage/azure-storage-common/test/test_base.hpp

@@ -3,6 +3,18 @@
 
 #pragma once
 
+#include <azure/core/platform.hpp>
+
+#if defined(AZ_PLATFORM_WINDOWS)
+#if !defined(WIN32_LEAN_AND_MEAN)
+#define WIN32_LEAN_AND_MEAN
+#endif
+#if !defined(NOMINMAX)
+#define NOMINMAX
+#endif
+#include <windows.h>
+#endif
+
 #include <chrono>
 #include <cstdint>
 #include <limits>