Skip to content

Ensure URI creation is cross platform compatible #421

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 17 commits into from
Jul 28, 2021
Merged

Ensure URI creation is cross platform compatible #421

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
559a645
change URI creation, tests are broken
anamnavi Jul 23, 2021
8f2cf57
fix url creation for PSGalleryParameterSet
anamnavi Jul 23, 2021
84fe368
tests still broken but push changes
anamnavi Jul 26, 2021
63e1cd6
this works for all tests Linux and Windows and for relative paths
anamnavi Jul 27, 2021
343eb55
set and register tests work on Linux and Windows now
anamnavi Jul 27, 2021
f24e961
revert accidental changes
anamnavi Jul 27, 2021
325dd21
remove accidental change to InstallHelper
anamnavi Jul 27, 2021
afcfd66
remove uneccessary variable
anamnavi Jul 27, 2021
1d09ed6
remove comments
anamnavi Jul 27, 2021
96ace23
push some todo comments I need to resolve
anamnavi Jul 27, 2021
f1efe5a
refactor and clean up
anamnavi Jul 27, 2021
6281802
remove unneccessary directive
anamnavi Jul 27, 2021
de78219
resolve PR styling feedback
anamnavi Jul 27, 2021
8363f60
change error message for invalid url to be more insightful for user
anamnavi Jul 27, 2021
761c826
add tests with relative path
anamnavi Jul 28, 2021
5fa79c7
Merge branch 'master' of https://github.com/anamnavi/PowerShellGet in…
anamnavi Jul 28, 2021
9223c74
use Get-Location to get current location
anamnavi Jul 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 17 additions & 27 deletions src/code/RegisterPSResourceRepository.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class RegisterPSResourceRepository : PSCmdlet
private const string NameParameterSet = "NameParameterSet";
private const string PSGalleryParameterSet = "PSGalleryParameterSet";
private const string RepositoriesParameterSet = "RepositoriesParameterSet";
private Uri _url;

#endregion

Expand All @@ -51,25 +52,7 @@ class RegisterPSResourceRepository : PSCmdlet
/// </summary>
[Parameter(Mandatory = true, Position = 1, ParameterSetName = NameParameterSet)]
[ValidateNotNullOrEmpty]
public Uri URL
{
get
{ return _url; }

set
{
if (!Uri.TryCreate(value, string.Empty, out Uri url))
{
var message = string.Format(CultureInfo.InvariantCulture, "The URL provided is not valid: {0}", value);
var ex = new ArgumentException(message);
var moduleManifestNotFound = new ErrorRecord(ex, "InvalidUrl", ErrorCategory.InvalidArgument, null);
ThrowTerminatingError(moduleManifestNotFound);
}

_url = url;
}
}
private Uri _url;
public string URL { get; set; }

/// <summary>
/// When specified, registers PSGallery repository.
Expand Down Expand Up @@ -156,9 +139,17 @@ protected override void ProcessRecord()
switch (ParameterSetName)
{
case NameParameterSet:
if (!Utils.TryCreateValidUrl(urlString: URL,
cmdletPassedIn: this,
urlResult: out _url,
errorRecord: out ErrorRecord errorRecord))
{
ThrowTerminatingError(errorRecord);
}

try
{
items.Add(NameParameterSetHelper(Name, URL, Priority, Trusted));
items.Add(NameParameterSetHelper(Name, _url, Priority, Trusted));
}
catch (Exception e)
{
Expand Down Expand Up @@ -334,13 +325,12 @@ private PSRepositoryInfo RepoValidationHelper(Hashtable repo)
return null;
}

if (!Uri.TryCreate(repo["URL"].ToString(), UriKind.Absolute, out Uri repoURL))
if (!Utils.TryCreateValidUrl(urlString: repo["Url"].ToString(),
cmdletPassedIn: this,
urlResult: out Uri repoURL,
errorRecord: out ErrorRecord errorRecord))
{
WriteError(new ErrorRecord(
new PSInvalidOperationException("Invalid url, unable to create"),
"InvalidUrlScheme",
ErrorCategory.InvalidArgument,
this));
WriteError(errorRecord);
return null;
}

Expand Down Expand Up @@ -372,6 +362,6 @@ private PSRepositoryInfo RepoValidationHelper(Hashtable repo)
}
}

#endregion
#endregion
}
}
59 changes: 32 additions & 27 deletions src/code/SetPSResourceRepository.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class SetPSResourceRepository : PSCmdlet
private const string NameParameterSet = "NameParameterSet";
private const string RepositoriesParameterSet = "RepositoriesParameterSet";
private const int DefaultPriority = -1;
private Uri _url;
#endregion

#region Parameters
Expand All @@ -45,25 +46,7 @@ class SetPSResourceRepository : PSCmdlet
/// </sumamry>
[Parameter(ParameterSetName = NameParameterSet)]
[ValidateNotNullOrEmpty]
public Uri URL
{
get
{ return _url; }

set
{
if (!Uri.TryCreate(value, string.Empty, out Uri url))
{
var message = string.Format(CultureInfo.InvariantCulture, "The URL provided is not a valid url: {0}", value);
var ex = new ArgumentException(message);
var urlErrorRecord = new ErrorRecord(ex, "InvalidUrl", ErrorCategory.InvalidArgument, null);
ThrowTerminatingError(urlErrorRecord);
}

_url = url;
}
}
private Uri _url;
public string URL { get; set; }

/// <summary>
/// Specifies a hashtable of repositories and is used to register multiple repositories at once.
Expand Down Expand Up @@ -129,14 +112,23 @@ protected override void BeginProcessing()

protected override void ProcessRecord()
{
if (MyInvocation.BoundParameters.ContainsKey(nameof(URL)))
{
bool isUrlValid = Utils.TryCreateValidUrl(URL, this, out _url, out ErrorRecord errorRecord);
if (!isUrlValid)
{
ThrowTerminatingError(errorRecord);
}
}

List<PSRepositoryInfo> items = new List<PSRepositoryInfo>();

switch(ParameterSetName)
{
case NameParameterSet:
try
{
items.Add(UpdateRepositoryStoreHelper(Name, URL, Priority, Trusted));
items.Add(UpdateRepositoryStoreHelper(Name, _url, Priority, Trusted));
}
catch (Exception e)
{
Expand Down Expand Up @@ -243,15 +235,28 @@ private List<PSRepositoryInfo> RepositoriesParameterSetHelper()
private PSRepositoryInfo RepoValidationHelper(Hashtable repo)
{
WriteDebug(String.Format("Parsing through repository: {0}", repo["Name"]));

Uri repoURL = null;
if (repo.ContainsKey("Url") && !Uri.TryCreate(repo["URL"].ToString(), UriKind.Absolute, out repoURL))
if (repo.ContainsKey("Url"))
{
WriteError(new ErrorRecord(
new PSInvalidOperationException("Invalid Url, unable to parse and create Uri"),
"InvalidUrl",
ErrorCategory.InvalidArgument,
this));
return null;
if (String.IsNullOrEmpty(repo["Url"].ToString()))
{
WriteError(new ErrorRecord(
new PSInvalidOperationException("Repository url cannot be null if provided"),
"NullURLForRepositoriesParameterSetUpdate",
ErrorCategory.InvalidArgument,
this));
return null;
}

if (!Utils.TryCreateValidUrl(urlString: repo["Url"].ToString(),
cmdletPassedIn: this,
urlResult: out repoURL,
errorRecord: out ErrorRecord errorRecord))
{
WriteError(errorRecord);
return null;
}
}

bool repoTrusted = false;
Expand Down
50 changes: 50 additions & 0 deletions src/code/Utils.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using System.Management.Automation.Language;
using System.Runtime.InteropServices;
using NuGet.Versioning;
using System.Globalization;

namespace Microsoft.PowerShell.PowerShellGet.UtilClasses
{
Expand Down Expand Up @@ -171,6 +172,55 @@ public static bool TryParseVersionOrVersionRange(
return VersionRange.TryParse(version, out versionRange);
}

#endregion

#region Url methods

public static bool TryCreateValidUrl(
string urlString,
PSCmdlet cmdletPassedIn,
out Uri urlResult,
out ErrorRecord errorRecord
)
{
errorRecord = null;

if (!urlString.StartsWith(Uri.UriSchemeHttps) &&
!urlString.StartsWith(Uri.UriSchemeHttp) &&
!urlString.StartsWith(Uri.UriSchemeFtp))
{
// url string could be of type (potentially) UriSchemeFile or invalid type
// can't check for UriSchemeFile because relative paths don't qualify as UriSchemeFile
try
{
// this is needed for a relative path urlstring. Does not throw error for an absolute path
urlString = cmdletPassedIn.SessionState.Path.GetResolvedPSPathFromPSPath(urlString)[0].Path;

}
catch (Exception)
{
// this should only be reached if the url string is invalid
// i.e www.google.com
var message = string.Format(CultureInfo.InvariantCulture, "The URL provided is not valid: {0} and must be of Uri Scheme: HTTP, HTTPS, FTP or File", urlString);
var ex = new ArgumentException(message);
errorRecord = new ErrorRecord(ex, "InvalidUrl", ErrorCategory.InvalidArgument, null);
urlResult = null;
return false;
}
}

bool tryCreateResult = Uri.TryCreate(urlString, UriKind.Absolute, out urlResult);
if (!tryCreateResult)
{
var message = string.Format(CultureInfo.InvariantCulture, "The URL provided is not valid: {0}", urlString);
var ex = new ArgumentException(message);
errorRecord = new ErrorRecord(ex, "InvalidUrl", ErrorCategory.InvalidArgument, null);
urlResult = null;
}

return tryCreateResult;
}

#endregion

#region Path methods
Expand Down
34 changes: 23 additions & 11 deletions test/RegisterPSResourceRepository.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ Describe "Test Register-PSResourceRepository" {
$tmpDir3Path = Join-Path -Path $TestDrive -ChildPath "tmpDir3"
$tmpDirPaths = @($tmpDir1Path, $tmpDir2Path, $tmpDir3Path)
Get-NewTestDirs($tmpDirPaths)

$relativeCurrentPath = Get-Location
}
AfterEach {
Get-RevertPSResourceRepositoryFile
Expand All @@ -26,23 +28,23 @@ Describe "Test Register-PSResourceRepository" {
It "register repository given Name, URL (bare minimum for NameParmaterSet)" {
$res = Register-PSResourceRepository -Name "testRepository" -URL $tmpDir1Path -PassThru
$res.Name | Should -Be "testRepository"
$res.URL | Should -Contain $tmpDir1Path
$res.URL.LocalPath | Should -Contain $tmpDir1Path
$res.Trusted | Should -Be False
$res.Priority | Should -Be 50
}

It "register repository with Name, URL, Trusted (NameParameterSet)" {
$res = Register-PSResourceRepository -Name "testRepository" -URL $tmpDir1Path -Trusted -PassThru
$res.Name | Should -Be "testRepository"
$res.URL | Should -Contain $tmpDir1Path
$res.URL.LocalPath | Should -Contain $tmpDir1Path
$res.Trusted | Should -Be True
$res.Priority | Should -Be 50
}

It "register repository given Name, URL, Trusted, Priority (NameParameterSet)" {
$res = Register-PSResourceRepository -Name "testRepository" -URL $tmpDir1Path -Trusted -Priority 20 -PassThru
$res.Name | Should -Be "testRepository"
$res.URL | Should -Contain $tmpDir1Path
$res.URL.LocalPath | Should -Contain $tmpDir1Path
$res.Trusted | Should -Be True
$res.Priority | Should -Be 20
}
Expand Down Expand Up @@ -82,17 +84,17 @@ Describe "Test Register-PSResourceRepository" {

Register-PSResourceRepository -Repositories $arrayOfHashtables
$res = Get-PSResourceRepository -Name "testRepository"
$res.URL | Should -Contain $tmpDir1Path
$res.URL.LocalPath | Should -Contain $tmpDir1Path
$res.Trusted | Should -Be False
$res.Priority | Should -Be 50

$res2 = Get-PSResourceRepository -Name "testRepository2"
$res2.URL | Should -Contain $tmpDir2Path
$res2.URL.LocalPath | Should -Contain $tmpDir2Path
$res2.Trusted | Should -Be True
$res2.Priority | Should -Be 50

$res3 = Get-PSResourceRepository -Name "testRepository3"
$res3.URL | Should -Contain $tmpDir3Path
$res3.URL.LocalPath | Should -Contain $tmpDir3Path
$res3.Trusted | Should -Be True
$res3.Priority | Should -Be 20
}
Expand Down Expand Up @@ -123,23 +125,23 @@ Describe "Test Register-PSResourceRepository" {
$res1.Priority | Should -Be 50

$res2 = Get-PSResourceRepository -Name "testRepository"
$res2.URL | Should -Contain $tmpDir1Path
$res2.URL.LocalPath | Should -Contain $tmpDir1Path
$res2.Trusted | Should -Be False
$res2.Priority | Should -Be 50

$res3 = Get-PSResourceRepository -Name "testRepository2"
$res3.URL | Should -Contain $tmpDir2Path
$res3.URL.LocalPath | Should -Contain $tmpDir2Path
$res3.Trusted | Should -Be True
$res3.Priority | Should -Be 50

$res4 = Get-PSResourceRepository -Name "testRepository3"
$res4.URL | Should -Contain $tmpDir3Path
$res4.URL.LocalPath | Should -Contain $tmpDir3Path
$res4.Trusted | Should -Be True
$res4.Priority | Should -Be 20
}

It "not register repository when Name is provided but URL is not" {
{Register-PSResourceRepository -Name "testRepository" -URL "" -ErrorAction Stop} | Should -Throw -ErrorId "InvalidUrl,Microsoft.PowerShell.PowerShellGet.Cmdlets.RegisterPSResourceRepository"
{Register-PSResourceRepository -Name "testRepository" -URL "" -ErrorAction Stop} | Should -Throw -ErrorId "ParameterArgumentValidationError,Microsoft.PowerShell.PowerShellGet.Cmdlets.RegisterPSResourceRepository"
}

It "not register repository when Name is empty but URL is provided" {
Expand Down Expand Up @@ -193,7 +195,7 @@ Describe "Test Register-PSResourceRepository" {
$testCases2 = @{Type = "-Name is not specified"; IncorrectHashTable = @{URL = $tmpDir1Path}; ErrorId = "NullNameForRepositoriesParameterSetRegistration,Microsoft.PowerShell.PowerShellGet.Cmdlets.RegisterPSResourceRepository"},
@{Type = "-Name is PSGallery"; IncorrectHashTable = @{Name = "PSGallery"; URL = $tmpDir1Path}; ErrorId = "PSGalleryProvidedAsNameRepoPSet,Microsoft.PowerShell.PowerShellGet.Cmdlets.RegisterPSResourceRepository"},
@{Type = "-URL not specified"; IncorrectHashTable = @{Name = "testRepository"}; ErrorId = "NullURLForRepositoriesParameterSetRegistration,Microsoft.PowerShell.PowerShellGet.Cmdlets.RegisterPSResourceRepository"},
@{Type = "-URL is not valid scheme"; IncorrectHashTable = @{Name = "testRepository"; URL="www.google.com"}; ErrorId = "InvalidUrlScheme,Microsoft.PowerShell.PowerShellGet.Cmdlets.RegisterPSResourceRepository"}
@{Type = "-URL is not valid scheme"; IncorrectHashTable = @{Name = "testRepository"; URL="www.google.com"}; ErrorId = "InvalidUrl,Microsoft.PowerShell.PowerShellGet.Cmdlets.RegisterPSResourceRepository"}

It "not register incorrectly formatted Name type repo among correct ones when incorrect type is <Type>" -TestCases $testCases2 {
param($Type, $IncorrectHashTable, $ErrorId)
Expand All @@ -218,4 +220,14 @@ Describe "Test Register-PSResourceRepository" {
$res3.Name | Should -Be "PSGallery"
$res3.Priority | Should -Be 30
}

It "should register repository with relative location provided as URL" {
Register-PSResourceRepository -Name "testRepository" -URL "./"
$res = Get-PSResourceRepository -Name "testRepository"

$res.Name | Should -Be "testRepository"
$res.URL.LocalPath | Should -Contain $relativeCurrentPath
$res.Trusted | Should -Be False
$res.Priority | Should -Be 50
}
}
Loading