Skip to content

Commit

Permalink
Fixup sources urls (#511)
Browse files Browse the repository at this point in the history 
* Fix issue with .git suffix

Adds tests of non-crates.io registries

* Update

* Fix trailing slash

* Warn and strip when using scheme modifiers
  • Loading branch information
@Jake-Shadle
Jake-Shadle committed Apr 12, 2023
1 parent d485903 commit 568c350
Show file tree
Hide file tree
Showing 18 changed files with 414 additions and 35 deletions.
2 changes: 1 addition & 1 deletion src/advisories/helpers.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ pub(crate) fn url_to_local_dir(url: &str) -> Result<(String, String), Error> {
canonical.pop();
}

if canonical.ends_with(".git") {
if host == "github.com" && canonical.ends_with(".git") {
canonical.truncate(canonical.len() - 4);
}

Expand Down
4 changes: 4 additions & 0 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,10 @@ pub(crate) fn normalize_git_url(url: &mut Url) -> GitSpec {
url.path_segments_mut().unwrap().pop().push(&last);
}

if url.path().ends_with('/') {
url.path_segments_mut().unwrap().pop_if_empty();
}

let mut spec = GitSpec::Any;

for (k, _v) in url.query_pairs() {
Expand Down
92 changes: 61 additions & 31 deletions src/sources/cfg.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,16 +112,37 @@ impl cfg::UnvalidatedConfig for Config {
self.allow_registry.len() + self.allow_git.len() + self.private.len(),
);

for (aurl, exact) in self
for (aurl, exact, is_git) in self
.allow_registry
.into_iter()
.map(|u| (u, true))
.chain(self.allow_git.into_iter().map(|u| (u, true)))
.chain(self.private.into_iter().map(|u| (u, false)))
.map(|u| (u, true, false))
.chain(self.allow_git.into_iter().map(|u| (u, true, true)))
.chain(self.private.into_iter().map(|u| (u, false, false)))
{
match url::Url::parse(aurl.as_ref()) {
let astr = aurl.as_ref();
let mut skip = 0;

if let Some(start_scheme) = astr.find("://") {
if let Some(i) = astr[..start_scheme].find('+') {
diags.push(
Diagnostic::warning()
.with_message("scheme modifiers are unnecessary")
.with_labels(vec![Label::primary(
cfg_file,
aurl.span.start..aurl.span.start + start_scheme,
)]),
);

skip = i + 1;
}
}

match url::Url::parse(&astr[skip..]) {
Ok(mut url) => {
crate::normalize_git_url(&mut url);
if is_git {
crate::normalize_git_url(&mut url);
}

allowed_sources.push(UrlSource {
url: UrlSpan {
value: url,
Expand Down Expand Up @@ -208,38 +229,47 @@ mod test {

let mut diags = Vec::new();
let validated = cd.config.sources.validate(cd.id, &mut diags);
assert!(diags.is_empty());

assert!(diags.is_empty());
let diags: Vec<_> = diags
.into_iter()
.map(|d| crate::diag::diag_to_json(d.into(), &cd._files, None))
.collect();

insta::assert_json_snapshot!(diags);

assert_eq!(validated.file_id, cd.id);
assert_eq!(validated.unknown_registry, LintLevel::Allow);
assert_eq!(validated.unknown_git, LintLevel::Deny);

let expected = [
UrlSource {
url: url::Url::parse("https://sekretz.com/registry/index")
.unwrap()
.fake(),
exact: true,
},
UrlSource {
url: url::Url::parse("https://fake.sparse.com").unwrap().fake(),
exact: true,
},
UrlSource {
url: url::Url::parse("https://notgithub.com/orgname/reponame")
.unwrap()
.fake(),
exact: true,
},
UrlSource {
url: url::Url::parse("https://internal-host/repos")
.unwrap()
.fake(),
exact: false,
},
];

assert_eq!(
validated.allowed_sources,
vec![
UrlSource {
url: url::Url::parse("https://sekretz.com/registry/index")
.unwrap()
.fake(),
exact: true,
},
UrlSource {
url: url::Url::parse("https://notgithub.com/orgname/reponame")
.unwrap()
.fake(),
exact: true,
},
UrlSource {
url: url::Url::parse("https://internal-host/repos")
.unwrap()
.fake(),
exact: false,
},
],
"{:#?}",
validated.allowed_sources
validated.allowed_sources, expected,
"{:#?} != {:#?}",
validated.allowed_sources, expected
);

// Obviously order could change here, but for now just hardcode it
Expand Down
21 changes: 21 additions & 0 deletions src/sources/snapshots/cargo_deny__sources__cfg__test__deserializes_sources_cfg.snap
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
---
source: src/sources/cfg.rs
expression: diags
---
[
{
"fields": {
"labels": [
{
"column": 5,
"line": 7,
"message": "",
"span": "sparse+http"
}
],
"message": "scheme modifiers are unnecessary",
"severity": "warning"
},
"type": "diagnostic"
}
]
1 change: 1 addition & 0 deletions tests/cfg/sources.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ unknown-git = "deny"
required-git-spec = "tag"
allow-registry = [
"https://sekretz.com/registry/index",
"sparse+https://fake.sparse.com",
]
allow-git = [
"https://notgithub.com/orgname/reponame.git",
Expand Down
4 changes: 2 additions & 2 deletions tests/snapshots/sources__allows_git.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ expression: diags
"column": 9,
"line": 3,
"message": "source allowance",
"span": "'https://gitlab.com/amethyst-engine/amethyst'"
"span": "'https://gitlab.com/amethyst-engine/amethyst/'"
}
],
"message": "'git' source explicitly allowed",
Expand Down Expand Up @@ -81,7 +81,7 @@ expression: diags
"column": 9,
"line": 3,
"message": "source allowance",
"span": "'https://gitlab.com/amethyst-engine/amethyst'"
"span": "'https://gitlab.com/amethyst-engine/amethyst/'"
}
],
"message": "'git' source explicitly allowed",
Expand Down
76 changes: 76 additions & 0 deletions tests/snapshots/sources__allows_registry_index_git.snap
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
---
source: tests/sources.rs
expression: diags
---
[
{
"fields": {
"code": "allowed-source",
"graphs": [
{
"Krate": {
"name": "crate-one",
"version": "0.1.0"
},
"parents": [
{
"Krate": {
"name": "non-crates-io-registry",
"version": "0.1.0"
}
}
]
}
],
"labels": [
{
"column": 17,
"line": 1,
"message": "source",
"span": "registry+https://dl.cloudsmith.io/public/embark/deny/cargo/index.git"
},
{
"column": 9,
"line": 3,
"message": "source allowance",
"span": "'https://dl.cloudsmith.io/public/embark/deny/cargo/index.git'"
}
],
"message": "'registry' source explicitly allowed",
"severity": "note"
},
"type": "diagnostic"
},
{
"fields": {
"code": "source-not-allowed",
"graphs": [
{
"Krate": {
"name": "crate-two",
"version": "0.2.0"
},
"parents": [
{
"Krate": {
"name": "non-crates-io-registry",
"version": "0.1.0"
}
}
]
}
],
"labels": [
{
"column": 17,
"line": 2,
"message": "source",
"span": "sparse+https://cargo.cloudsmith.io/embark/deny/"
}
],
"message": "detected 'registry' source not explicitly allowed",
"severity": "error"
},
"type": "diagnostic"
}
]
76 changes: 76 additions & 0 deletions tests/snapshots/sources__allows_registry_index_sparse.snap
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
---
source: tests/sources.rs
expression: diags
---
[
{
"fields": {
"code": "source-not-allowed",
"graphs": [
{
"Krate": {
"name": "crate-one",
"version": "0.1.0"
},
"parents": [
{
"Krate": {
"name": "non-crates-io-registry",
"version": "0.1.0"
}
}
]
}
],
"labels": [
{
"column": 17,
"line": 1,
"message": "source",
"span": "registry+https://dl.cloudsmith.io/public/embark/deny/cargo/index.git"
}
],
"message": "detected 'registry' source not explicitly allowed",
"severity": "error"
},
"type": "diagnostic"
},
{
"fields": {
"code": "allowed-source",
"graphs": [
{
"Krate": {
"name": "crate-two",
"version": "0.2.0"
},
"parents": [
{
"Krate": {
"name": "non-crates-io-registry",
"version": "0.1.0"
}
}
]
}
],
"labels": [
{
"column": 17,
"line": 2,
"message": "source",
"span": "sparse+https://cargo.cloudsmith.io/embark/deny/"
},
{
"column": 9,
"line": 3,
"message": "source allowance",
"span": "'https://cargo.cloudsmith.io/embark/deny/'"
}
],
"message": "'registry' source explicitly allowed",
"severity": "note"
},
"type": "diagnostic"
}
]
Loading

0 comments on commit 568c350

Please sign in to comment.