Skip to content

Commit 7ca9ed6

Browse files
committed
Mark Info::infohash as potentially lossy
`Info::infohash` will discard fields not present in the `Info` struct, and can thus produce an erronous infohash. To try to prevent this function from being called in those circumstances, rename it to `Info::infohash_lossy`, and add a comment explaining why it is dangerous. Embarassingly, `Info::infohash` was called in `TorrentSummary::from_input`, and thus transitively by the torrent show subcommand, where it is definitely not safe, since torrent show is intended to be used with arbitrary torrents. This is now fixed. There was also a build failure caused by a cache issue, so change the cache keys to invalidate the caches. type: fixed
1 parent dbb0eac commit 7ca9ed6

File tree

6 files changed

+29
-13
lines changed

6 files changed

+29
-13
lines changed

.github/workflows/build.yaml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,19 +49,19 @@ jobs:
4949
uses: actions/cache@v1
5050
with:
5151
path: ~/.cargo/registry
52-
key: ${{ runner.os }}-cargo-registry
52+
key: ${{ runner.os }}-cargo-registry-v0
5353

5454
- name: Cache cargo index
5555
uses: actions/cache@v1
5656
with:
5757
path: ~/.cargo/git
58-
key: ${{ runner.os }}-cargo-index
58+
key: ${{ runner.os }}-cargo-index-v0
5959

6060
- name: Cache cargo build
6161
uses: actions/cache@v1
6262
with:
6363
path: target
64-
key: ${{ runner.os }}-cargo-build-target
64+
key: ${{ runner.os }}-cargo-build-target-v0
6565

6666
- name: Install Stable
6767
uses: actions-rs/toolchain@v1

src/info.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,16 @@ impl Info {
3434
self.mode.content_size()
3535
}
3636

37-
pub(crate) fn infohash(&self) -> Result<Infohash> {
37+
/// This function is potentially lossy. If an arbitrary torrent info
38+
/// dictionary is deserialized into an `Info` struct, extra fields not present
39+
/// on the struct will be discarded. If `infohash_lossy` is then called on
40+
/// the resultant struct, those fields will not contribute to the infohash,
41+
/// which will thus be different from that of the original torrent.
42+
///
43+
/// It will not be lossy if no extra fields are present in the original
44+
/// torrent. So, it is safe to call on torrents that have just been created
45+
/// and are still in memory, and thus are known to have no extra fields.
46+
pub(crate) fn infohash_lossy(&self) -> Result<Infohash> {
3847
let encoded = bendy::serde::ser::to_bytes(self).context(error::InfoSerialize)?;
3948
Ok(Infohash::from_bencoded_info_dict(&encoded))
4049
}

src/magnet_link.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,9 @@ pub(crate) struct MagnetLink {
99
}
1010

1111
impl MagnetLink {
12-
pub(crate) fn from_metainfo(metainfo: &Metainfo) -> Result<MagnetLink> {
13-
let mut link = Self::with_infohash(metainfo.infohash()?);
12+
/// See `Info::infohash_lossy` for details on when this function is lossy.
13+
pub(crate) fn from_metainfo_lossy(metainfo: &Metainfo) -> Result<MagnetLink> {
14+
let mut link = Self::with_infohash(metainfo.infohash_lossy()?);
1415

1516
link.set_name(metainfo.info.name.clone());
1617

src/metainfo.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,9 @@ impl Metainfo {
114114
})
115115
}
116116

117-
pub(crate) fn infohash(&self) -> Result<Infohash> {
118-
self.info.infohash()
117+
/// See `Info::infohash_lossy` for details on when this function is lossy.
118+
pub(crate) fn infohash_lossy(&self) -> Result<Infohash> {
119+
self.info.infohash_lossy()
119120
}
120121

121122
#[cfg(test)]

src/subcommand/torrent/create.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -454,11 +454,13 @@ impl Create {
454454
}
455455

456456
if self.show {
457-
TorrentSummary::from_metainfo(metainfo.clone())?.write(env)?;
457+
// We just created this torrent, so no extra fields have been discarded.
458+
TorrentSummary::from_metainfo_lossy(metainfo.clone())?.write(env)?;
458459
}
459460

460461
if self.print_magnet_link {
461-
let mut link = MagnetLink::from_metainfo(&metainfo)?;
462+
// We just created this torrent, so no extra fields have been discarded.
463+
let mut link = MagnetLink::from_metainfo_lossy(&metainfo)?;
462464
for peer in self.peers {
463465
link.add_peer(peer);
464466
}

src/torrent_summary.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,20 @@ impl TorrentSummary {
1515
}
1616
}
1717

18-
pub(crate) fn from_metainfo(metainfo: Metainfo) -> Result<Self> {
18+
/// See `Info::infohash_lossy` for details on when this function is lossy.
19+
pub(crate) fn from_metainfo_lossy(metainfo: Metainfo) -> Result<Self> {
1920
let bytes = metainfo.serialize()?;
2021
let size = Bytes(bytes.len().into_u64());
21-
let infohash = metainfo.infohash()?;
22+
let infohash = metainfo.infohash_lossy()?;
2223
Ok(Self::new(metainfo, infohash, size))
2324
}
2425

2526
pub(crate) fn from_input(input: &Input) -> Result<Self> {
2627
let metainfo = Metainfo::from_input(input)?;
28+
let infohash = Infohash::from_input(input)?;
29+
let size = Bytes(input.data().len().into_u64());
2730

28-
Ok(Self::from_metainfo(metainfo)?)
31+
Ok(Self::new(metainfo, infohash, size))
2932
}
3033

3134
pub(crate) fn write(&self, env: &mut Env) -> Result<()> {

0 commit comments

Comments
 (0)