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
This commit is contained in:
Casey Rodarmor 2020-09-06 20:01:55 -07:00
parent dbb0eac96d
commit 7ca9ed62d5
No known key found for this signature in database
GPG Key ID: 556186B153EC6FE0
6 changed files with 29 additions and 13 deletions

View File

@ -49,19 +49,19 @@ jobs:
uses: actions/cache@v1 uses: actions/cache@v1
with: with:
path: ~/.cargo/registry path: ~/.cargo/registry
key: ${{ runner.os }}-cargo-registry key: ${{ runner.os }}-cargo-registry-v0
- name: Cache cargo index - name: Cache cargo index
uses: actions/cache@v1 uses: actions/cache@v1
with: with:
path: ~/.cargo/git path: ~/.cargo/git
key: ${{ runner.os }}-cargo-index key: ${{ runner.os }}-cargo-index-v0
- name: Cache cargo build - name: Cache cargo build
uses: actions/cache@v1 uses: actions/cache@v1
with: with:
path: target path: target
key: ${{ runner.os }}-cargo-build-target key: ${{ runner.os }}-cargo-build-target-v0
- name: Install Stable - name: Install Stable
uses: actions-rs/toolchain@v1 uses: actions-rs/toolchain@v1

View File

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

View File

@ -9,8 +9,9 @@ pub(crate) struct MagnetLink {
} }
impl MagnetLink { impl MagnetLink {
pub(crate) fn from_metainfo(metainfo: &Metainfo) -> Result<MagnetLink> { /// See `Info::infohash_lossy` for details on when this function is lossy.
let mut link = Self::with_infohash(metainfo.infohash()?); pub(crate) fn from_metainfo_lossy(metainfo: &Metainfo) -> Result<MagnetLink> {
let mut link = Self::with_infohash(metainfo.infohash_lossy()?);
link.set_name(metainfo.info.name.clone()); link.set_name(metainfo.info.name.clone());

View File

@ -114,8 +114,9 @@ impl Metainfo {
}) })
} }
pub(crate) fn infohash(&self) -> Result<Infohash> { /// See `Info::infohash_lossy` for details on when this function is lossy.
self.info.infohash() pub(crate) fn infohash_lossy(&self) -> Result<Infohash> {
self.info.infohash_lossy()
} }
#[cfg(test)] #[cfg(test)]

View File

@ -454,11 +454,13 @@ impl Create {
} }
if self.show { if self.show {
TorrentSummary::from_metainfo(metainfo.clone())?.write(env)?; // We just created this torrent, so no extra fields have been discarded.
TorrentSummary::from_metainfo_lossy(metainfo.clone())?.write(env)?;
} }
if self.print_magnet_link { if self.print_magnet_link {
let mut link = MagnetLink::from_metainfo(&metainfo)?; // We just created this torrent, so no extra fields have been discarded.
let mut link = MagnetLink::from_metainfo_lossy(&metainfo)?;
for peer in self.peers { for peer in self.peers {
link.add_peer(peer); link.add_peer(peer);
} }

View File

@ -15,17 +15,20 @@ impl TorrentSummary {
} }
} }
pub(crate) fn from_metainfo(metainfo: Metainfo) -> Result<Self> { /// See `Info::infohash_lossy` for details on when this function is lossy.
pub(crate) fn from_metainfo_lossy(metainfo: Metainfo) -> Result<Self> {
let bytes = metainfo.serialize()?; let bytes = metainfo.serialize()?;
let size = Bytes(bytes.len().into_u64()); let size = Bytes(bytes.len().into_u64());
let infohash = metainfo.infohash()?; let infohash = metainfo.infohash_lossy()?;
Ok(Self::new(metainfo, infohash, size)) Ok(Self::new(metainfo, infohash, size))
} }
pub(crate) fn from_input(input: &Input) -> Result<Self> { pub(crate) fn from_input(input: &Input) -> Result<Self> {
let metainfo = Metainfo::from_input(input)?; let metainfo = Metainfo::from_input(input)?;
let infohash = Infohash::from_input(input)?;
let size = Bytes(input.data().len().into_u64());
Ok(Self::from_metainfo(metainfo)?) Ok(Self::new(metainfo, infohash, size))
} }
pub(crate) fn write(&self, env: &mut Env) -> Result<()> { pub(crate) fn write(&self, env: &mut Env) -> Result<()> {