Skip to content

Commit

Permalink
Prevent download hang if filename exceeds fs limit
Browse files Browse the repository at this point in the history
If the name template ends up generating a path where the filename itself
exceeds the limits of the file system (e.g. 255 characters for NTFS on
Windows, 255 bytes for most Linux filesystems), the try..catch block to
handle race-condition file moves in MoveToSaveFolder unhelpfully ends up
causing an infinite loop.

Add a test for the correct behaviour of an IOException being thrown and
adjust MoveToSaveFolder to check for the existence of a file in the
destination path if an IOException has been caught, re-throwing if it
has been thrown for a different reason.

Resolves issue #259.
  • Loading branch information
ribbons committed Feb 3, 2024
1 parent 7c04ad8 commit 4051804
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 9 deletions.
4 changes: 2 additions & 2 deletions Classes/DownloadHandler.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright © 2007-2020 Matt Robinson
* Copyright © 2007-2024 Matt Robinson
*
* SPDX-License-Identifier: GPL-3.0-or-later
*/
Expand Down Expand Up @@ -227,7 +227,7 @@ private void DownloadProgThread()
}
catch (IOException ioExp)
{
this.DownloadError(Provider.ErrorType.LocalProblem, "Encountered an error saving the downloaded file. " + ioExp.Message + " You may need to select a new location for saving downloaded programmes under Options -> Main Options.");
this.DownloadError(Provider.ErrorType.LocalProblem, "Encountered an error saving the downloaded file. " + ioExp.Message.TrimEnd('.') + ". You may need to select a new location for saving downloaded programmes or adjust the file name format under Options -> Main Options.");
return;
}
catch (UnauthorizedAccessException unAuthExp)
Expand Down
9 changes: 3 additions & 6 deletions Classes/Model/Download.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright © 2008-2020 Matt Robinson
* Copyright © 2008-2024 Matt Robinson
* Copyright © 2017-2018 Neil Blanchard
*
* SPDX-License-Identifier: GPL-3.0-or-later
Expand Down Expand Up @@ -428,12 +428,9 @@ public static string MoveToSaveFolder(string formatString, Programme progInfo, E
{
OsUtils.MoveFile(sourceFile, savePath);
}
catch (IOException e)
catch (IOException)
{
// We only want to handle IOException itself as a
// number of IOException subclasses are thrown for
// other cases we don't want to handle
if (e.GetType() == typeof(IOException))
if (File.Exists(savePath))
{
// Destination file created since File.Exists check
continue;
Expand Down
25 changes: 24 additions & 1 deletion Test/Classes/Model/TestDownload.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright © 2020 Matt Robinson
* Copyright © 2020-2024 Matt Robinson
*
* SPDX-License-Identifier: GPL-3.0-or-later
*/
Expand Down Expand Up @@ -154,5 +154,28 @@ public void MoveToSaveFolderLongPath()
destFile,
Download.MoveToSaveFolder(@"%progname%", prog, episode, tempLong, "mp3", sourceFile));
}

/// <summary>
/// Test that MoveToSaveFolder will throw an IOException if the destination
/// file ends up with a name longer than the filesystem supports.
/// </summary>
[Fact]
public void MoveToSaveFolderLongFileName()
{
string tempBase = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
Directory.CreateDirectory(tempBase);
string sourceFile = Path.GetTempFileName();

var prog = new Programme();
prog.Name = new string('P', 500);

var episode = new Episode();
episode.Name = "EN";

string destFile = Path.Combine(tempBase, prog.Name + ".mp3");

Assert.Throws<IOException>(() =>
Download.MoveToSaveFolder(@"%progname%", prog, episode, tempBase, "mp3", sourceFile));
}
}
}

0 comments on commit 4051804

Please sign in to comment.