FIx downloading of files from absolute or relative paths

[Daniel-Svensson]

Long story
I saw that SimpleWebSource seems to support specifying absolute url's in the "Filename" path of releases.json so I did try a scenaro with "releases.json" from one website and the .nupkg files from another location only to get download problems because the UpdateManager tried to create a file named "https://my.domain/client/mypackage." which contains invalid filename characters.

I suppose you can get the similar problems if you make the path relative (I assume "../../filename" fill put the downloaded file in the completely wrong folder .

I've made the quickest possible fix and only uses the filename part when choosing what the temporary downloaded file should be called (and verified that the update works as expected).

[caesay]

Thanks for this contribution!

This is kind of / not officially supported, although there is some code that does work with this.

I don't know if it makes sense for the modification to exist within UpdateManager the way you've suggested here. Would you expect any source to be able to specify an absolute path? What does that mean if I was to provide an absolute HTTP URL for a local package source eg. SimpleFileSource? Or, once a package has been downloaded from an absolute URL (eg. via SimpleWebSource) does it make sense for the local releases.json to preserve the URL or just store it with FileName?

Since there is a lot of code which expects there to only be a FileName and not a more complex absolute location, my gut feeling is that we may want to parse and transform URL's (in SimpleWebSource) or absolute file paths (in SimpleFileSource) before returning from GetReleaseFeed.

What do you think?

[Daniel-Svensson]

I don't know if it makes sense for the modification to exist within UpdateManager the way you've suggested here. Would you expect any source to be able to specify an absolute path? What does that mean if I was to provide an absolute HTTP URL for a local package source eg. SimpleFileSource?

My initial thought was to do something similar and deriving from velopackasset in a custom update source but I did reconsider it.

1. I would expect that as long the the VelopackAsset.FileName can contain relative or absolute file paths (and just not file names) then the UpdateManager should handle it gracefully without any risk of putting files in the wrong place.
   • I initially had planned to just replace invalid filename characters with "_" or similar, but getting the filename path seemed easier and I think one want to do that part anyway (and maybe replace invalid characters). This prevents the updatemanager from placing packages in the wrong directory.
   • The alternative would be to add validation to VelopackAsset.FileName and throw if filename contains any invalid characters Path.GetInvalidFileNameChars() but it might break currently working code
2. It becomes much more complicated and the feed it populated in VelopackAssetFeed.FromJson which would need to be replicated for webfeed (and maybe local path feed)

Or, once a package has been downloaded from an absolute URL (eg. via SimpleWebSource) does it make sense for the local releases.json to preserve the URL or just store it with FileName?

I did not know there is a local releases.json file, can you give a hint of where to locate it (I can have a look at the source code if you don't know straight away)

Since there is a lot of code which expects there to only be a FileName and not a more complex absolute location, my gut feeling is that we may want to parse and transform URL's (in SimpleWebSource) or absolute file paths (in SimpleFileSource) before returning from GetReleaseFeed.

• That is probably true but at least normal downloads works very fine with the minimal change, but some logs will say filename and not filepath.
• One idea would be to add an additional FilePath or OriginalFileName property to the asset and setting the FileName property would set both FilePaht (to full path) and filename (but call Path.GetFileName)

[Daniel-Svensson]

I did add some additional safeguards against other invalid characters in filename, but I can revert that if you think it makes it unnecessary complex.

Tests

GetSafeFilename("C:\\sad.sd").Dump();
GetSafeFilename("..\\sad.sd").Dump();
GetSafeFilename("..\\sadz<>.sd").Dump();

Output

sad.sd
sad.sd
sadz__.sd