Skip to content

Commit f3aa9b6

Browse files
More error handling in HandleShadowCopy when creating the shadow copy directory fails
Add ConfigurationLoadException to exception helper logging
1 parent 8f77e2a commit f3aa9b6

File tree

3 files changed

+49
-26
lines changed

3 files changed

+49
-26
lines changed

src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -331,14 +331,23 @@ APPLICATION_INFO::HandleShadowCopy(const ShimOptions& options, IHttpContext& pHt
331331
auto shadowCopyBaseDirectory = std::filesystem::directory_entry(shadowCopyPath);
332332
if (!shadowCopyBaseDirectory.exists())
333333
{
334-
LOG_INFOF(L"Attempting to Create Directory");
335-
336334
auto ret = CreateDirectory(shadowCopyBaseDirectory.path().wstring().c_str(), nullptr);
337335
if (!ret)
338336
{
339-
LOG_ERRORF(L"Failed to create shadow copy base directory %ls. Error: %d",
340-
shadowCopyBaseDirectory.path().c_str(),
341-
GetLastError());
337+
auto pathString = to_multi_byte_string(shadowCopyBaseDirectory.path(), CP_UTF8);
338+
auto errorCode = std::error_code(GetLastError(), std::system_category());
339+
std::string errorMessage = format("Failed to create shadow copy base directory %s. Error: %s",
340+
pathString.c_str(),
341+
errorCode.message().c_str());
342+
343+
// TODO: Better substatus code
344+
error.statusCode = 500i16;
345+
error.subStatusCode = 30i16;
346+
error.generalErrorType = format("ASP.NET Core app failed to start - Failed to copy to shadow copy directory");
347+
error.errorReason = format("Ensure the application pool process model has write permissions for the shadow copy base directory %s",
348+
pathString.c_str());
349+
error.detailedErrorContent = errorMessage;
350+
return std::wstring();
342351
}
343352
}
344353

@@ -373,7 +382,7 @@ APPLICATION_INFO::HandleShadowCopy(const ShimOptions& options, IHttpContext& pHt
373382
// It could expand to a network drive, or an expanded link folder path
374383
// We already made it an absolute path relative to the physicalPath above
375384
try {
376-
// CopyToDirectory throws exception on failure, therefore don't need to check return value
385+
// CopyToDirectory will succeed or throw exception, so return value can be ignored
377386
Environment::CopyToDirectory(physicalPath, shadowCopyPath, options.QueryCleanShadowCopyDirectory(), shadowCopyBaseDirectory.path(), copiedFileCount);
378387
}
379388
catch (const std::system_error& ex)

src/Servers/IIS/AspNetCoreModuleV2/CommonLib/exceptions.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "debugutil.h"
1010
#include "StringHelpers.h"
1111
#include "InvalidOperationException.h"
12+
#include "ConfigurationLoadException.h"
1213
#include "ntassert.h"
1314
#include "NonCopyable.h"
1415
#include "EventTracing.h"
@@ -197,6 +198,11 @@ __declspec(noinline) inline HRESULT CaughtExceptionHResult(LOCATION_ARGUMENTS_ON
197198
ReportException(LOCATION_CALL exception);
198199
return exception.GetResult();
199200
}
201+
catch (const ConfigurationLoadException& exception)
202+
{
203+
ReportException(LOCATION_CALL exception);
204+
return HRESULT_FROM_WIN32(ERROR_UNHANDLED_EXCEPTION);
205+
}
200206
catch (const InvalidOperationException& exception)
201207
{
202208
ReportException(LOCATION_CALL exception);
@@ -224,6 +230,10 @@ __declspec(noinline) inline std::wstring CaughtExceptionToString()
224230
{
225231
return exception.as_wstring();
226232
}
233+
catch (const ConfigurationLoadException& exception)
234+
{
235+
return exception.get_message();
236+
}
227237
catch (const std::system_error& exception)
228238
{
229239
return to_wide_string(exception.what(), CP_ACP);

src/Servers/IIS/IIS/test/IIS.ShadowCopy.Tests/ShadowCopyTests.cs

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ public async Task ShadowCopy_FailsWithUsefulExceptionMessage_WhenNoPermissionsTo
4343
Assert.Contains(shadowCopyDirectory.DirectoryPath, content, StringComparison.InvariantCultureIgnoreCase);
4444

4545
shadowCopyDirectory.RestoreAllPermissions();
46-
Assert.True(IsDirectoryEmpty(shadowCopyDirectory.DirectoryPath), "Expected shadow copy shadowCopyDirectory to be empty");
46+
// If failed to copy then the shadowCopyDirectory should be empty
47+
Assert.True(IsDirectoryEmpty(shadowCopyDirectory.DirectoryPath), "Expected shadow copy directory to be empty");
4748
}
4849

4950
[ConditionalFact]
@@ -73,7 +74,7 @@ public async Task ShadowCopy_FailsWithUsefulExceptionMessage_WhenNoWritePermissi
7374

7475
shadowCopyDirectory.RestoreAllPermissions();
7576
// If failed to copy then the shadowCopyDirectory should be empty
76-
Assert.True(IsDirectoryEmpty(shadowCopyDirectory.DirectoryPath), "Expected shadow copy shadowCopyDirectory to be empty");
77+
Assert.True(IsDirectoryEmpty(shadowCopyDirectory.DirectoryPath), "Expected shadow copy directory to be empty");
7778
}
7879

7980
[ConditionalFact]
@@ -241,7 +242,7 @@ public async Task ShadowCopyDeleteFolderDuringShutdownWorks()
241242
// Act
242243

243244
// Delete folder + file after app is shut down
244-
// Testing specific path on startup where we compare the app shadowCopyDirectory contents with the shadow copy shadowCopyDirectory
245+
// Testing specific path on startup where we compare the app directory contents with the shadow copy directory
245246
Directory.Delete(deleteDirPath, recursive: true);
246247

247248
RemoveAppOffline(deploymentResult.ContentRoot);
@@ -313,18 +314,18 @@ public async Task ShadowCopyE2EWorksWithOldFoldersPresent()
313314

314315
// Act & Assert
315316
response = await deploymentResult.HttpClient.GetAsync("Wow!");
316-
Assert.False(Directory.Exists(Path.Combine(directory.DirectoryPath, "0")), "Expected 0 shadow copy shadowCopyDirectory to be skipped");
317+
Assert.False(Directory.Exists(Path.Combine(directory.DirectoryPath, "0")), "Expected 0 shadow copy directory to be skipped");
317318

318319
// Depending on timing, this could result in a shutdown failure, but sometimes it succeeds, handle both situations
319320
if (!response.IsSuccessStatusCode)
320321
{
321322
Assert.True(response.ReasonPhrase == "Application Shutting Down" || response.ReasonPhrase == "Server has been shutdown");
322323
}
323324

324-
// This shutdown should trigger a copy to the next highest shadowCopyDirectory, which will be 2
325+
// This shutdown should trigger a copy to the next highest directory, which will be 2
325326
await deploymentResult.AssertRecycledAsync();
326327

327-
Assert.True(Directory.Exists(Path.Combine(directory.DirectoryPath, "2")), "Expected 2 shadow copy shadowCopyDirectory");
328+
Assert.True(Directory.Exists(Path.Combine(directory.DirectoryPath, "2")), "Expected 2 shadow copy directory to exist");
328329

329330
// Act & Assert
330331
response = await deploymentResult.HttpClient.GetAsync("Wow!");
@@ -360,26 +361,26 @@ public async Task ShadowCopyCleansUpOlderFolders()
360361

361362
// Act & Assert
362363
response = await deploymentResult.HttpClient.GetAsync("Wow!");
363-
Assert.False(Directory.Exists(Path.Combine(shadowCopyDirectory.DirectoryPath, "0")), "Expected 0 shadow copy shadowCopyDirectory to be skipped");
364+
Assert.False(Directory.Exists(Path.Combine(shadowCopyDirectory.DirectoryPath, "0")), "Expected 0 shadow copy directory to be skipped");
364365

365366
// Depending on timing, this could result in a shutdown failure, but sometimes it succeeds, handle both situations
366367
if (!response.IsSuccessStatusCode)
367368
{
368369
Assert.True(response.ReasonPhrase == "Application Shutting Down" || response.ReasonPhrase == "Server has been shutdown");
369370
}
370371

371-
// This shutdown should trigger a copy to the next highest shadowCopyDirectory, which will be 11
372+
// This shutdown should trigger a copy to the next highest directory, which will be 11
372373
await deploymentResult.AssertRecycledAsync();
373374

374-
Assert.True(Directory.Exists(Path.Combine(shadowCopyDirectory.DirectoryPath, "11")), "Expected 11 shadow copy shadowCopyDirectory");
375+
Assert.True(Directory.Exists(Path.Combine(shadowCopyDirectory.DirectoryPath, "11")), "Expected 11 shadow copy directory");
375376

376377
// Act & Assert
377378
response = await deploymentResult.HttpClient.GetAsync("Wow!");
378379
Assert.True(response.IsSuccessStatusCode);
379380

380381
// Verify old directories were cleaned up
381-
Assert.False(Directory.Exists(Path.Combine(shadowCopyDirectory.DirectoryPath, "1")), "Expected 1 shadow copy shadowCopyDirectory to be deleted");
382-
Assert.False(Directory.Exists(Path.Combine(shadowCopyDirectory.DirectoryPath, "3")), "Expected 3 shadow copy shadowCopyDirectory to be deleted");
382+
Assert.False(Directory.Exists(Path.Combine(shadowCopyDirectory.DirectoryPath, "1")), "Expected 1 shadow copy directory to be deleted");
383+
Assert.False(Directory.Exists(Path.Combine(shadowCopyDirectory.DirectoryPath, "3")), "Expected 3 shadow copy directory to be deleted");
383384
}
384385

385386
[ConditionalFact]
@@ -469,7 +470,7 @@ public static int RunCommand(string command, string arguments, ILogger logger, s
469470
return process.ExitCode;
470471
}
471472

472-
public static void RemoveAllPermissions(DirectoryInfo directoryInfo)
473+
internal static void RemoveAllPermissions(DirectoryInfo directoryInfo)
473474
{
474475
var directorySecurity = directoryInfo.GetAccessControl();
475476

@@ -485,7 +486,7 @@ public static void RemoveAllPermissions(DirectoryInfo directoryInfo)
485486
directoryInfo.SetAccessControl(emptyPermissions);
486487
}
487488

488-
public static void RemoveWritePermissions(DirectoryInfo directoryInfo)
489+
internal static void RemoveWritePermissions(DirectoryInfo directoryInfo)
489490
{
490491
var directorySecurity = directoryInfo.GetAccessControl();
491492
// Deny Write access for Everyone
@@ -523,7 +524,10 @@ public TempDirectoryRestrictedPermissions(DirectoryInfo directoryInfo, ILogger l
523524

524525
public void RestoreAllPermissions()
525526
{
526-
if (_hasPermissions) return;
527+
if (_hasPermissions)
528+
{
529+
return;
530+
}
527531

528532
RestoreAllPermissionsInner();
529533

@@ -539,7 +543,7 @@ private void RestoreAllPermissionsInner()
539543

540544
if (res != 0)
541545
{
542-
Logger.LogError("Failed to restore permissions for shadowCopyDirectory {DirectoryPath}. Takeown result: {TakeownResult}", DirectoryPath, res);
546+
Logger.LogError("Failed to restore permissions for directory {DirectoryPath}. Takeown result: {TakeownResult}", DirectoryPath, res);
543547
}
544548
}
545549

@@ -608,30 +612,30 @@ private static void DeleteDirectory(string directoryPath)
608612
}
609613
catch (Exception e)
610614
{
611-
Console.WriteLine($@"Failed to delete shadowCopyDirectory {directoryPath}: {e.Message}");
615+
Console.WriteLine($@"Failed to delete directory {directoryPath}: {e.Message}");
612616
}
613617
}
614618
}
615619

616620
// copied from https://learn.microsoft.com/dotnet/standard/io/how-to-copy-directories
617621
private static void DirectoryCopy(string sourceDirName, string destDirName, bool copySubDirs, string ignoreDirectory = "")
618622
{
619-
// Get the subdirectories for the specified shadowCopyDirectory.
623+
// Get the subdirectories for the specified directory.
620624
DirectoryInfo dir = new DirectoryInfo(sourceDirName);
621625

622626
if (!dir.Exists)
623627
{
624628
throw new DirectoryNotFoundException(
625-
"Source shadowCopyDirectory does not exist or could not be found: "
629+
"Source directory does not exist or could not be found: "
626630
+ sourceDirName);
627631
}
628632

629633
DirectoryInfo[] dirs = dir.GetDirectories();
630634

631-
// If the destination shadowCopyDirectory doesn't exist, create it.
635+
// If the destination directory doesn't exist, create it.
632636
Directory.CreateDirectory(destDirName);
633637

634-
// Get the files in the shadowCopyDirectory and copy them to the new location.
638+
// Get the files in the directory and copy them to the new location.
635639
FileInfo[] files = dir.GetFiles();
636640
foreach (FileInfo file in files)
637641
{

0 commit comments

Comments
 (0)