Skip to content
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.

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

Updates test suite re-enables encode : in version and / in qualifier values and assert Maven requirement of namespace #5

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 6 additions & 2 deletions src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ pub fn parse_name<'a>(input: &str) -> Result<(&str, String)> {
}
}

pub fn parse_namespace<'a>(input: &str) -> Result<(&str, Option<String>)> {
pub fn parse_namespace<'a>(input: &'a str, pkg_type: &str) -> Result<(&'a str, Option<String>)> {
if !input.is_empty() {
let mut namespace = String::with_capacity(input.len());
let mut components = input
Expand All @@ -133,6 +133,10 @@ pub fn parse_namespace<'a>(input: &str) -> Result<(&str, Option<String>)> {
}
Ok(("", Some(namespace)))
} else {
Ok(("", None))
//Some ecosystems require non-empty namespaces
match pkg_type {
"maven" => Err(Error::InvalidNamespaceComponent(input.to_string())),
_ => Ok(("", None))
}
}
}
6 changes: 3 additions & 3 deletions src/purl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ const ENCODE_SET: &AsciiSet = &percent_encoding::CONTROLS
.add(b'?')
.add(b'{')
.add(b'}')
// .add(b'/')
// .add(b':')
.add(b'/')
.add(b':')
.add(b';')
.add(b'=')
.add(b'@')
Expand Down Expand Up @@ -232,7 +232,7 @@ impl FromStr for PackageUrl<'static> {
let (s, version) = parser::parse_version(s)?;
let (s, ty) = parser::parse_type(s)?;
let (s, mut name) = parser::parse_name(s)?;
let (_, mut namespace) = parser::parse_namespace(s)?;
let (_, mut namespace) = parser::parse_namespace(s, &ty)?;

// Special rules for some types
match ty.as_ref() {
Expand Down
4 changes: 4 additions & 0 deletions tests/spec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ spec_tests!(github, "github namespace and name should be lowercased");
spec_tests!(docker, "docker uses qualifiers and hash image id as versions");
spec_tests!(maven, "valid maven purl");
spec_tests!(maven_basic, "basic valid maven purl without version");
spec_tests!(maven_requires_namespace, "maven requires a namespace");
spec_tests!(maven_requires_nonempty_namespace, "maven requires a non-empty namespace");
spec_tests!(maven_case_sensitive, "valid maven purl with case sensitive namespace and name");
spec_tests!(maven_space, "valid maven purl containing a space in the version and qualifier");
spec_tests!(go_subpath, "valid go purl without version and with subpath");
Expand All @@ -29,3 +31,5 @@ spec_tests!(maven_type, "maven can come with a type qualifier");
spec_tests!(simple_slash, "slash / after scheme is not significant");
spec_tests!(double_slash, "double slash // after scheme is not significant");
spec_tests!(triple_slash, "slash /// after type is not significant");
spec_tests!(hackage, "valid hackage purl");
spec_tests!(hackage_name_and_version_required, "hackage purl requires name and version");
60 changes: 54 additions & 6 deletions tests/spec/test-suite-data.json
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@
{
"description": "docker uses qualifiers and hash image id as versions",
"purl": "pkg:docker/customer/dockerimage@sha256:244fd47e07d1004f0aed9c?repository_url=gcr.io",
"canonical_purl": "pkg:docker/customer/dockerimage@sha256:244fd47e07d1004f0aed9c?repository_url=gcr.io",
"canonical_purl": "pkg:docker/customer/dockerimage@sha256%3A244fd47e07d1004f0aed9c?repository_url=gcr.io",
"type": "docker",
"namespace": "customer",
"name": "dockerimage",
Expand All @@ -109,8 +109,8 @@
},
{
"description": "maven often uses qualifiers",
"purl": "pkg:Maven/org.apache.xmlgraphics/[email protected]?classifier=sources&repositorY_url=repo.spring.io/release",
"canonical_purl": "pkg:maven/org.apache.xmlgraphics/[email protected]?classifier=sources&repository_url=repo.spring.io/release",
"purl": "pkg:Maven/org.apache.xmlgraphics/[email protected]?repositorY_url=repo.spring.io/release&classifier=sources",
"canonical_purl": "pkg:maven/org.apache.xmlgraphics/[email protected]?classifier=sources&repository_url=repo.spring.io%2Frelease",
"type": "maven",
"namespace": "org.apache.xmlgraphics",
"name": "batik-anim",
Expand All @@ -121,8 +121,8 @@
},
{
"description": "maven pom reference",
"purl": "pkg:Maven/org.apache.xmlgraphics/[email protected]?extension=pom&repositorY_url=repo.spring.io/release",
"canonical_purl": "pkg:maven/org.apache.xmlgraphics/[email protected]?extension=pom&repository_url=repo.spring.io/release",
"purl": "pkg:Maven/org.apache.xmlgraphics/[email protected]?repositorY_url=repo.spring.io/release&extension=pom",
"canonical_purl": "pkg:maven/org.apache.xmlgraphics/[email protected]?extension=pom&repository_url=repo.spring.io%2Frelease",
"type": "maven",
"namespace": "org.apache.xmlgraphics",
"name": "batik-anim",
Expand All @@ -133,7 +133,7 @@
},
{
"description": "maven can come with a type qualifier",
"purl": "pkg:Maven/net.sf.jacob-project/[email protected]?classifier=x86&type=dll",
"purl": "pkg:Maven/net.sf.jacob-project/[email protected]?type=dll&classifier=x86",
"canonical_purl": "pkg:maven/net.sf.jacob-project/[email protected]?classifier=x86&type=dll",
"type": "maven",
"namespace": "net.sf.jacob-project",
Expand Down Expand Up @@ -227,6 +227,30 @@
"subpath": null,
"is_invalid": true
},
{
"description": "maven requires a namespace",
"purl": "pkg:maven/[email protected]",
"canonical_purl": "pkg:maven/[email protected]",
"type": "maven",
"namespace": null,
"name": null,
"version": null,
"qualifiers": null,
"subpath": null,
"is_invalid": true
},
{
"description": "maven requires a non-empty namespace",
"purl": "pkg:maven//[email protected]",
"canonical_purl": "pkg:maven//[email protected]",
"type": "maven",
"namespace": null,
"name": null,
"version": null,
"qualifiers": null,
"subpath": null,
"is_invalid": true
},
{
"description": "slash / after scheme is not significant",
"purl": "pkg:/maven/org.apache.commons/io",
Expand Down Expand Up @@ -298,5 +322,29 @@
"qualifiers": {"in production": "true"},
"subpath": null,
"is_invalid": true
},
{
"description": "valid hackage purl",
"purl": "pkg:hackage/[email protected]",
"canonical_purl": "pkg:hackage/[email protected]",
"type": "hackage",
"namespace": null,
"name": "AC-HalfInteger",
"version": "1.2.1",
"qualifiers": null,
"subpath": null,
"is_invalid": false
},
{
"description": "hackage purl requires name and version",
"purl": "pkg:hackage",
"canonical_purl": "pkg:hackage",
"type": "hackage",
"namespace": null,
"name": null,
"version": null,
"qualifiers": null,
"subpath": null,
"is_invalid": true
}
]
2 changes: 1 addition & 1 deletion tests/spec/testcase.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ impl<'a> SpecTestCase<'a> {
let json = v
.into_iter()
.find(|x| x["description"].as_str().unwrap().eq(desc))
.unwrap();
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This kind of helped grok how to make changes to the test suite, so figured I'd offer it up to give any future new contributors some more context than just a generic error message.

.expect(&format!("Failed to find test with expected description of {desc}"));
::serde_json::from_value(json).unwrap()
} else {
unreachable!("invalid json file")
Expand Down