package-url / packageurl-go Goto Github PK

packageurl-go needs to be updated to the latest purl spec.

Hi,
I just found that the license of this package is not correctly detected by pkg.go.dev and GitHub.
It would be great to follow the convention(?) to rename the mit.LICENSE file to LICENSE.

Ref: https://pkg.go.dev/license-policy
Ref: https://docs.github.com/en/github/creating-cloning-and-archiving-repositories/licensing-a-repository#detecting-a-license

Currently, our README.md defines the following versioning scheme:

The versions will follow the spec. So if the spec is released at 1.0. 
Then all versions in the 1.x.y will follow the 1.x spec.

In #57 we realized how impractical this is, hence I thought about releasing a v1.0.0 and creating a v1.1.0 release after merging the aforementioned PR.

Also, I thought about dropping this version scheme in general and instead we could go with version directories: v1/ or v2/, like some other APIs do.

WDYT? @pombredanne @sschuberth

Right now we are not doing releases of packageurl-go. We should. Cut a release once the current set of issues have been run through and travis is passing.

Hi again!

I've started wondering about the correct escaping-rules for all the different components of the purl. As far as I can tell, the current implementation doesn't actually match the spec in a couple of edge-cases, as it uses PathEncode instead of QueryEncode.

(I'd argue that using PathEncode is the right thing for namespace, name and version, but it creates a couple of difficulties that I've written down on an issue on the spec).

Here's the escapes that are required:

• namespace: separate on / to get the individual segments, escape every segment with Path/QueryEscape. Currently this is implemented by the usage of JoinPath and then EscapedPath, I believe this is correct.
• name: simply escape with Path/QueryEscape: #55
• version: escape with Path/QueryEscape
• qualifier: use QueryEscape

The current implementation calls EscapedPathon namespace, name and version, so technically implements the above points, except for one difference: the name isn't escaped by itself, meaning that if it contains a /, it will not be escaped.

Additionally, it won't "fully" escape versions, which could create ambiguities if the version contains an @, e.g. pkg:deb/debian/[email protected]@alpha-1. However, this question I think needs to be answered by the spec, see my linked comment. As a workaround, I guess we could query-escape it...

The name issue can be solved by PathEscapeing (and unescaping) it, I think I'll raise a PR to fix this.

It would be good to know however what to do with the version as well :)

When qualifiers are included in a purl, the ToString method no longer provides deterministic results.

The following loop:

uri := "pkg:deb/debian/[email protected]?arch=i386&distro=jessie"
purl, _ := packageurl.FromString(uri)
for i := 0; i < 20; i++ {
    fmt.Printf("%s\n", purl.ToString())
}

yields results similar to:

pkg:deb/debian/[email protected]?distro=jessie&arch=i386
pkg:deb/debian/[email protected]?arch=i386&distro=jessie
pkg:deb/debian/[email protected]?arch=i386&distro=jessie
pkg:deb/debian/[email protected]?arch=i386&distro=jessie
pkg:deb/debian/[email protected]?arch=i386&distro=jessie
pkg:deb/debian/[email protected]?arch=i386&distro=jessie
pkg:deb/debian/[email protected]?arch=i386&distro=jessie
pkg:deb/debian/[email protected]?distro=jessie&arch=i386
pkg:deb/debian/[email protected]?arch=i386&distro=jessie
pkg:deb/debian/[email protected]?arch=i386&distro=jessie
pkg:deb/debian/[email protected]?arch=i386&distro=jessie
pkg:deb/debian/[email protected]?arch=i386&distro=jessie
pkg:deb/debian/[email protected]?distro=jessie&arch=i386
pkg:deb/debian/[email protected]?distro=jessie&arch=i386
pkg:deb/debian/[email protected]?arch=i386&distro=jessie
pkg:deb/debian/[email protected]?arch=i386&distro=jessie
pkg:deb/debian/[email protected]?arch=i386&distro=jessie
pkg:deb/debian/[email protected]?arch=i386&distro=jessie
pkg:deb/debian/[email protected]?arch=i386&distro=jessie
pkg:deb/debian/[email protected]?distro=jessie&arch=i386

Note how the ToString method fails to produce a consistent result. Qualifiers are not ordered. This can be highly problematic for users of this package, in particular in testing scenarios.

The problem is the use of a map to store qualifiers. This does not capture the ordering of qualifier fields.

Rather than using

type Qualifiers map[string]string

I would suggest using:

// Qualifier represens a single key=value qualifier in the package url
type Qualifier struct {
       Key   string
       Value string
}

 // Qualifiers houses each key=value pair in the package url
type Qualifiers []Qualifier

Hi @sschuberth @stevespringett @pombredanne @iamwillbar is this project still alive?

If you are looking for a new co-maintainer, I would be willing to contribute. I think I am mentioned as one of the recent top contributors anyway and people start thinking that I am involved in the project 🤣

The packageurl.FromString() function does not properly parse subpaths. It strips the result of slashes. The following code:

package main

import (
	"fmt"

	packageurl "github.com/package-url/packageurl-go"
)

func main() {
	p, _ := packageurl.FromString("pkg:golang/google.golang.org/genproto#googleapis/api/annotations")
	fmt.Printf("subpath: %v\n", p.Subpath)
}

results in

subpath: googleapisapiannotations

Note that the example purl is taken from the purl-spec repo.

As the PURL spec is evolving, other purl-dependant projects might fail to detect whether a given purl type is a valid type or just a random string that looks like a purl.

For instance, if we want to check from "test:ok/name@version", test is a supported type or not, we would have to check if it is one of purl.TypeBitbucket, purl.TypeCocoapods, purl.TypeCargo... etc., as the FromString func does not seem to validate the type,

A simplistic approach I can think of is just having an exported SupportedTypes map[string]struct{} like:

var SupportedTypes = map[string]struct{}{
	purl.TypeBitbucket:   {},
	purl.TypeCargo:       {},
	purl.TypeCocoapods:   {},
	purl.TypeComposer:    {},
        ...
}

This way, one could just do sth like: if _, ok := purl.SupportedTypes[myTypeToCheck]; !ok { ... to handle unsupported types.

Do you think it makes sense? If so, I can raise a PR.

Running make test from a fresh clone of this repo results in a test failure

go test -v -cover ./...
=== RUN   TestFromStringExamples
--- PASS: TestFromStringExamples (0.00s)
=== RUN   TestToStringExamples
    packageurl_test.go:213: MLflow model with unique identifiers failed: pkg:mlflow/trafficsigns@10?model_uuid=36233173b22f4c89b451f1228d700d49&run_id=410a3121-2709-4f88-98dd-dba0ef056b0a&repository_url=https:%2F%2Fadb-5245952564735461.0.azuredatabricks.net%2Fapi%2F2.0%2Fmlflow != pkg:mlflow/trafficsigns@10?model_uuid=36233173b22f4c89b451f1228d700d49&repository_url=https://adb-5245952564735461.0.azuredatabricks.net/api/2.0/mlflow&run_id=410a3121-2709-4f88-98dd-dba0ef056b0a
--- FAIL: TestToStringExamples (0.00s)
=== RUN   TestStringer
--- PASS: TestStringer (0.00s)
=== RUN   TestQualifiersMapConversion
--- PASS: TestQualifiersMapConversion (0.00s)
FAIL
coverage: 58.0% of statements
FAIL	github.com/package-url/packageurl-go	0.139s
FAIL
make: *** [test] Error 1

I would like to do some improvements regarding release management. This involves several software supply chain security improvements:

1. Creating a changelog with every release
2. Creating a SBOM for every release (Software Bill of Materials)
3. Create signed releases with the chainguard stack

An example project for such a project can be found here: https://github.com/shibumi/secure-supply-chain-example

Would be nice if we manage to be SLSA compliant with our releases.

Hello,

@another-rex and I are attempting to package packageurl-go for Debian as we need it for a build dependency.

The Debian packaging tooling is grabbing the v0.1.0 tagged release, which does not include the subsequent addition of the testdata (added most recently in ab67608) so the package build fails.

Could you please make a new release that includes ab67608?

Thanks

Andrew

For types such as github, the specification states that name should be lowercased, but not the version.
https://github.com/package-url/purl-spec/blob/master/PURL-TYPES.rst#github

Git tags and branches however can be case sensitive.

Looking at the code the functiontypeAdjustName not only passes the name, but is passing name and version, eg. purl-spec@244fd47e07d1004 and as such the version gets lowercased along with the name.

https://github.com/package-url/packageurl-go/blob/master/packageurl.go#L267

Trying to view Go package doc in https://pkg.go.dev/github.com/package-url/packageurl-go shows the message:

Documentation not displayed due to license restrictions.
See our license policy.

Does anybody know what license issue they might be referring to? The repo is licensed under MIT, but maybe the mit.LICENSE symlink is causing pkg.go.dev to get confused?

At the moment, the TestFromStringExamples [1] parsing unit test does little else than verify that the parsing completes without an error. For each parsed purl, the purl's struct fields should be compared against the expected field values of the TestFixture.

[1]

I have code that solves this but it relies on PR #13 being merged. Let me know if you need help on this.

Hi again!

After #52, there seems to be a "bug" about the ordering of the qualifiers.
The code says:

// Qualifiers is a slice of key=value pairs, with order preserved as it appears
// in the package URL.

However, with the switch to the url.URL type, when encoding the qualifiers they actually get ordered lexicographically.

Now I've started to wonder what the "correct" behaviour for this is. The PURL Spec even says:

If the qualifiers are not empty and not composed only of key/value pairs where the value is empty:
...

• sort this list of qualifier strings lexicographically
  ...

So admittedly, according to the spec, the "new" behaviour is correct.
Should I submit a PR to remove the comment that the order is being preserved? I'm wondering if this should be considered a breaking change.

Or do you have an opinion / better idea on what to do here?

Thanks!

The latest release is v0.1.1, released 2 months ago, and is missing several types in the spec (added in #43). Can this not block on #47, and get a new release

At the moment, the PackageURL type provides a ToString method to output a textual representation of the package URL.

A more idiomatic approach would be to implement the Stringer interface.

To not break backwards compatability, String should be implemented as a forwarding call to ToString.

Rather than maintain diverging sources of truth, the local test-suite-data.json should be removed with all libraries referring to the purl-spec test-suite-data.json as the source of truth. An example of this is seen in #44 introducing a Julia test not mirrored in the purl-spec test-suite. #32 introduced the local test-suite-data.json copied from the packageurl-js repo instead of the purl-spec reflecting a problem with maintaining multiple divergent copies. While #46 updated the local copy from the purl-spec repo, it disables the github action workflow pulling the latest copy from the purl-spec. If the purl-spec test-suite is updated and the library is non-compliant, it won't be obvious until the someone updates the local copy and possibly resolves merge conflicts between the revisions.

There are inconsistencies with how namespace, subpath and name components are URL encoded by FromString and how they are decoded by ToString. See #22 for more details and proposed fixes.

Finally Go has a prescribed standard for managing project dependencies. With Go 1.11, support for modules was introduced. Starting in Go 1.13, module mode will be the default for all development.

For any project that wishes to be a part of the Go community, it makes a lot of sense to make use of modules. packageurl-go should be no exception.

specifically, case sensitivity depending on whether it's Azure ML or Databricks

https://github.com/package-url/purl-spec/blob/master/PURL-TYPES.rst#mlflow

A bug was discovered after upgrading to v0.1.2 where ./ and ../ are not valid prefixes as seen here: guacsec/guac#1545. It appears to be due to this logic:

if s == "." || s == ".." {
	return fmt.Errorf("invalid Package URL subpath: %q", p.Subpath)
}

As per the SPDX spec, file paths are generally prefixed with ./. If a purl was to be constructed using the files read in from an SPDX SBOM, it would throw an error.