Skip to content

Commit 573f9bb

Browse files
committed
Auto merge of #6912 - alexcrichton:fix-parse, r=Eh2406
Fix skipping over invalid registry packages This accidentally regressed in the previous caching PR for Cargo. Invalid lines of JSON in the registry are intended to be skipped over, but when skipping we forgot to update some indices which meant that all future versions would fail to parse as well!
2 parents afd240e + 71c01fe commit 573f9bb

File tree

3 files changed

+74
-18
lines changed

3 files changed

+74
-18
lines changed

src/cargo/sources/registry/index.rs

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,7 @@ impl Summaries {
490490
if cfg!(debug_assertions) {
491491
cache_contents = Some(s.raw_data);
492492
} else {
493-
return Ok(Some(s))
493+
return Ok(Some(s));
494494
}
495495
}
496496
Err(e) => {
@@ -512,14 +512,12 @@ impl Summaries {
512512
ret.raw_data = contents.to_vec();
513513
let mut cache = SummariesCache::default();
514514
hit_closure = true;
515-
let mut start = 0;
516-
for end in memchr::Memchr::new(b'\n', contents) {
515+
for line in split(contents, b'\n') {
517516
// Attempt forwards-compatibility on the index by ignoring
518517
// everything that we ourselves don't understand, that should
519518
// allow future cargo implementations to break the
520519
// interpretation of each line here and older cargo will simply
521520
// ignore the new lines.
522-
let line = &contents[start..end];
523521
let summary = match IndexSummary::parse(line, source_id) {
524522
Ok(summary) => summary,
525523
Err(e) => {
@@ -530,7 +528,6 @@ impl Summaries {
530528
let version = summary.summary.package_id().version().clone();
531529
cache.versions.push((version.clone(), line));
532530
ret.versions.insert(version, summary.into());
533-
start = end + 1;
534531
}
535532
if let Some(index_version) = index_version {
536533
cache_bytes = Some(cache.serialize(index_version));
@@ -635,30 +632,24 @@ impl<'a> SummariesCache<'a> {
635632
if *first_byte != CURRENT_CACHE_VERSION {
636633
failure::bail!("looks like a different Cargo's cache, bailing out");
637634
}
638-
let mut iter = memchr::Memchr::new(0, rest);
639-
let mut start = 0;
640-
if let Some(end) = iter.next() {
641-
let update = &rest[start..end];
635+
let mut iter = split(rest, 0);
636+
if let Some(update) = iter.next() {
642637
if update != last_index_update.as_bytes() {
643638
failure::bail!(
644639
"cache out of date: current index ({}) != cache ({})",
645640
last_index_update,
646641
str::from_utf8(update)?,
647642
)
648643
}
649-
start = end + 1;
650644
} else {
651645
failure::bail!("malformed file");
652646
}
653647
let mut ret = SummariesCache::default();
654-
while let Some(version_end) = iter.next() {
655-
let version = &rest[start..version_end];
648+
while let Some(version) = iter.next() {
656649
let version = str::from_utf8(version)?;
657650
let version = Version::parse(version)?;
658-
let summary_end = iter.next().unwrap();
659-
let summary = &rest[version_end + 1..summary_end];
651+
let summary = iter.next().unwrap();
660652
ret.versions.push((version, summary));
661-
start = summary_end + 1;
662653
}
663654
Ok(ret)
664655
}
@@ -740,3 +731,28 @@ impl IndexSummary {
740731
})
741732
}
742733
}
734+
735+
fn split<'a>(haystack: &'a [u8], needle: u8) -> impl Iterator<Item = &'a [u8]> + 'a {
736+
struct Split<'a> {
737+
haystack: &'a [u8],
738+
needle: u8,
739+
}
740+
741+
impl<'a> Iterator for Split<'a> {
742+
type Item = &'a [u8];
743+
744+
fn next(&mut self) -> Option<&'a [u8]> {
745+
if self.haystack.is_empty() {
746+
return None;
747+
}
748+
let (ret, remaining) = match memchr::memchr(self.needle, self.haystack) {
749+
Some(pos) => (&self.haystack[..pos], &self.haystack[pos + 1..]),
750+
None => (self.haystack, &[][..]),
751+
};
752+
self.haystack = remaining;
753+
Some(ret)
754+
}
755+
}
756+
757+
Split { haystack, needle }
758+
}

tests/testsuite/registry.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1953,3 +1953,31 @@ fn rename_deps_and_features() {
19531953
p.cargo("build --features bar/foo01").run();
19541954
p.cargo("build --features bar/another").run();
19551955
}
1956+
1957+
#[test]
1958+
fn ignore_invalid_json_lines() {
1959+
Package::new("foo", "0.1.0").publish();
1960+
Package::new("foo", "0.1.1")
1961+
.invalid_json(true)
1962+
.publish();
1963+
Package::new("foo", "0.2.0").publish();
1964+
1965+
let p = project()
1966+
.file(
1967+
"Cargo.toml",
1968+
r#"
1969+
[project]
1970+
name = "a"
1971+
version = "0.5.0"
1972+
authors = []
1973+
1974+
[dependencies]
1975+
foo = '0.1.0'
1976+
foo02 = { version = '0.2.0', package = 'foo' }
1977+
"#,
1978+
)
1979+
.file("src/lib.rs", "")
1980+
.build();
1981+
1982+
p.cargo("build").run();
1983+
}

tests/testsuite/support/registry.rs

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ use cargo::sources::CRATES_IO_INDEX;
77
use cargo::util::Sha256;
88
use flate2::write::GzEncoder;
99
use flate2::Compression;
10-
use git2;
11-
use hex;
1210
use tar::{Builder, Header};
1311
use url::Url;
1412

@@ -137,6 +135,7 @@ pub struct Package {
137135
features: HashMap<String, Vec<String>>,
138136
local: bool,
139137
alternative: bool,
138+
invalid_json: bool,
140139
}
141140

142141
#[derive(Clone)]
@@ -232,6 +231,7 @@ impl Package {
232231
features: HashMap::new(),
233232
local: false,
234233
alternative: false,
234+
invalid_json: false,
235235
}
236236
}
237237

@@ -342,6 +342,13 @@ impl Package {
342342
self
343343
}
344344

345+
/// Causes the JSON line emitted in the index to be invalid, presumably
346+
/// causing Cargo to skip over this version.
347+
pub fn invalid_json(&mut self, invalid: bool) -> &mut Package {
348+
self.invalid_json = invalid;
349+
self
350+
}
351+
345352
/// Creates the package and place it in the registry.
346353
///
347354
/// This does not actually use Cargo's publishing system, but instead
@@ -384,8 +391,13 @@ impl Package {
384391
t!(t!(File::open(&self.archive_dst())).read_to_end(&mut c));
385392
cksum(&c)
386393
};
394+
let name = if self.invalid_json {
395+
serde_json::json!(1)
396+
} else {
397+
serde_json::json!(self.name)
398+
};
387399
let line = serde_json::json!({
388-
"name": self.name,
400+
"name": name,
389401
"vers": self.vers,
390402
"deps": deps,
391403
"cksum": cksum,

0 commit comments

Comments
 (0)