cocoapods, cran, vsm and cpan packages should not be lowered

[gfs]

Like nuget, these other repos are case sensitive. Additionally the namespaces for some of them are case sensitive.

Cocoapods, for example stores the pod specs on their GitHub based on the first 3 bytes of an MD5 of the package name.

So RandomKit => 8278d1b7c80616d5bbcbe1261075786c => 827 yields the location of the podspec:

https://github.com/CocoaPods/Specs/tree/master/Specs/8/2/7/RandomKit

But currently the name is automatically lowered: randomkit => 8d6a963d5a8e769c65f0bf1f867f6d02 => 8d6 which is not the correct location.

https://github.com/CocoaPods/Specs/tree/master/Specs/8/d/6/randomkit => 404

[am11]

Thanks for your contribution!

A few suggestions; let me know what you think:

• Delete Validate* methods and inline exceptions, e.g.:
  Type = type ?? throw new MalformedPackageUrlException("The PackageURL type specified is invalid");
• Apply (replace, tolower) transformations only during the ToString().

[gfs]

Thanks for your contribution!

A few suggestions; let me know what you think:

• Delete Validate* methods and inline exceptions, e.g.:
  Type = type ?? throw new MalformedPackageUrlException("The PackageURL type specified is invalid");
• Apply (replace, tolower) transformations only during the ToString().

Those bits of code are actually what already existed [1 - the current version on main] - the only net change I made was to add more ifs to the nuget and the namespace method.

I now have pushed a new update that gets rid of the exception in the name but since I haven't modified the ValidateType at all I haven't updated that one which still throws an exception.

I don't think we should only lower names in the to string - callers likely want to fetch the name/namespace individually and not have to parse apart the ToString. We are currently using a fork of PackageUrl in https://github.com/Microsoft/OSSGadget and need the individual components to fetch from the repos.

[1]

packageurl-dotnet/src/PackageUrl.cs

Lines 267 to 282 in 1f7af04

	private string ValidateName(string name)
	{
	if (name == null)
	{
	throw new MalformedPackageUrlException("The PackageURL name specified is invalid");
	}
	if (Type == "pypi")
	{
	name = name.Replace('_', '-');
	}
	if (Type == "nuget")
	{
	return name;
	}
	return name.ToLower();
	}

[am11]

Similar to JavaScript impl. https://github.com/package-url/packageurl-js/blob/66f50318608e730070852beede255ce004e1f565/src/package-url.js#L66 (but we also curate package name for type npm)

Like nuget

Note that on protocol level, this is true. However, AFAIK, the NuGet/NuGet.Client repo changed their implementation to always create package with lower case name.

[gfs]

Do you want to remove the throw from the ValidateType as well?

[am11]

I don't think we should only lower names in the to string - callers likely want to fetch the name/namespace individually and not have to parse apart the ToString. We are currently using a fork of PackageUrl in https://github.com/Microsoft/OSSGadget and need the individual components to fetch from the repos

In which situation does the caller provide name in "wrong" casing and expects library to transform and fix it? pypi case came along the initial port of this library. Unless spec is requiring it, or you have a genuine use-case, I don't think we should overload the responsibility to fix user typos (i.e. just return originalName; // without any transformation).

[gfs]

I don't think we should only lower names in the to string - callers likely want to fetch the name/namespace individually and not have to parse apart the ToString. We are currently using a fork of PackageUrl in https://github.com/Microsoft/OSSGadget and need the individual components to fetch from the repos

In which situation does the caller provide name in "wrong" casing and expects library to transform and fix it? pypi case came along the initial port of this library. Unless spec is requiring it, or you have a genuine use-case, I don't think we should overload the responsibility to fix user typos (i.e. just return originalName; // without any transformation).

Doesn't this argue that we should remove the .ToLower call entirely? The user providing the wrong casing seems to be exactlyt the intent of the method being modified by this PR. The current implementation has made a choice to correct certain types of purls which means it will need to correct it in all cases that it is relevant. Either the Name property should just return what the user gave or it should correct it. Currently it is doing the wrong thing by always lowercasing it.

[am11]

I agree that it is wrong and we should fix it. The question was what would be the best way to fix it? If spec is not expecting us to handle such transformations or consumer, such as yourself, are not relying on this (so called) auto-correct feature, we should just delete .Replace and .ToLower calls.

[gfs]

As a consumer it is definitely convenient to be able to rely on the python mechanism but ultimately it’s not a huge change to adapt to not having it. I foresee the automatic lowering to be a recurring problem that detracts from how generically the library can be used and may require continual changes to support new schemes. Do you want all the modification removed or just the lowering? (Ie should Python _ Substitution remain)?

…

On Wed, Mar 2, 2022 at 3:00 PM, Adeel Mujahid ***@***.***> wrote: I agree that it is wrong and we should fix it. The question was what would be the best way to fix it? If spec is not expecting us to handle such transformations or consumer, such as yourself, are not relying on this (so called) auto-correct feature, we should just delete .Replace and .ToLower calls. — Reply to this email directly, [view it on GitHub](#18 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AAAYEVGFA5NNKSP22GQOZQ3U57XH3ANCNFSM5PYML7SQ). You are receiving this because you authored the thread.Message ID: ***@***.***>

[gfs]

Removing the tolower causes test failures. Let me know how you'd like to proceed.

[am11]

Ah, I see. Test data is reliable in a sense that it keeps the implementation in sync with packageurl-java and packageurl-js (we use almost the same test-suite-data.json file). Lets revert to 16de673.

Please add some test cases in test-suite-data.json for cocoapods, cran vsm etc.

[gfs]

Sounds good. I'll have a revision with some test cases shortly.

[am11]

LGTM, thanks a lot!

[jpinz]

@am11 Hey! Would you be able to publish a new version to NuGet with the changes from this PR? Thanks!

[am11]

Here we go: https://www.nuget.org/packages/packageurl-dotnet/1.1.0 📦