Buffering IO when modifying a zip archive

[hniksic]

I don't know what type to pass to ZipWriter::new_append(). Specifically:

• When reading an archive, one calls ZipArcihve::new() with a type that implements Read and Seek, such as BufReader<File>.
• When writing an archive, one calls ZipWriter::new() with a type that implements Write and Seek, such as BufWriter<File>.
• When appending to an archive, one calls ZipWriter::new_append() with... what type?

The problem is that new_append() requires its argument to implement all of Read, Write, and Seek, and no buffering IO wrapper provided by the standard library satisfies those bounds. Just giving it File will work, but won't do any buffering, which is not something you'd want to use in production. External crates such as bufstream don't support seeking. Examples don't appear to show how to invoke ZipWriter::new_append(), and tests only call it with io::Cursor<Vec<u8>>.

Currently I am using this hack helper to achieve this. Is there an easier way?

use std::io::{self, BufReader, BufWriter, Read, Seek, Write};
use std::fs::File;

pub struct BufReadWrite {
    r: BufReader<File>,
    w: BufWriter<File>,
}

impl BufReadWrite {
    pub fn new(f: File) -> io::Result<Self> {
        let fr = f;
        let fw = fr.try_clone()?;
        Ok(BufReadWrite {
            r: BufReader::new(fr),
            w: BufWriter::new(fw),
        })
    }
}

impl Read for BufReadWrite {
    fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
        self.r.read(buf)
    }
}

impl Write for BufReadWrite {
    fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
        self.w.write(buf)
    }

    fn flush(&mut self) -> io::Result<()> {
        self.w.flush()
    }
}

impl Seek for BufReadWrite {
    fn seek(&mut self, pos: io::SeekFrom) -> io::Result<u64> {
        self.w.flush()?;
        // BufReader's implementation of Seek::seek() guarantees to immediately
        // seek the underlying handle even if the seek is within the buffer
        // bounds. This is why this `seek()` works for writing as well.
        self.r.seek(pos)
    }
}

[Plecra]

A slightly more flexible hack might be

struct BufReadWrite<T> {
    r: BufReader<T>,
    w: BufWriter<T>,
}
impl BufReadWrite {
    pub fn new(data: T) -> Self {
        Self {
            r: BufReader::new(data.clone()),
            w: BufWriter::new(data),
        }
    }
}
// btw, in the case of a `File` I think the `Read` and `Write` implementations ought
// to flush each other's buffers since they're moving the underlying cursor. This is
// where the hack starts to show its cracks :D

let datasource = BufReadWriter::new(&my_file); // references are `Clone`

However there is no great standard library type here. The most straightforward solution is reading your archive into memory before fiddling with it. Where that's not an option, a custom type will need to be implemented that properly provides buffered read and write. If you'd like a hand with that, let me know and I'll write a gist 😄 .

I'm going to leave the issue open as there's a possibility we can solve this by relaxing the generic bound and providing some way to switch out the reader type for a writer type. Don't get your hopes up though!

As always this is trivial with the new API 😛 I'll be pushing an alpha version as soon as I can once I've finished my current contract

let mut writable_archive = zip::Archive::open(io::BufReader::new(file))?
    // This is what's tough to implement at the moment - `ZipArchive` and `ZipWriter` aren't so easy to convert between
    .map_data(|b| io::BufWriter::new(b.into_inner()));

[hniksic]

The modified approach would probably work for my use case. Its downside is that it requires the BufReadWrite to be generic over &File, which means you couldn't e.g. open the File and create the ZipArchive in the same function and return the ZipArchive to the caller - that'd be self-referential. At one point I experimented with BufReadWrite::new(data: T) wrapping data in Rc and storing BufReader<Rc<T>> and BufWriter<Rc<T>>. Unfortunately that gets hairy very quickly because of more complex trait bounds and because Rc<File> doesn't implement the IO traits that &File does, requiring yet another in-between wrapper. It was a lot of complex code for very little gain.

btw, in the case of a File I think the Read and Write implementations ought to flush each other's buffers since they're moving the underlying cursor. This is where the hack starts to show its cracks :D

Note that my <BufReadWriter as Seek>::seek() does flush the write buffer right before self.r.seek(). Flushing in the other direction is not a thing because seek() only ever seeks through BufReader. I'd be interested if you found other cracks, though.

Where that's not an option, a custom type will need to be implemented that properly provides buffered read and write. If you'd like a hand with that, let me know and I'll write a gist.

It's not so much a case of needing a hand with the implementation, but of having a "blessed" implementation, either exposed by the library or just provided in the examples folder, which is used by others and updated as issues are found.

[Plecra]

The new_append API was a happy discovery when a user wanted to use the current ZipWriter struct with existing data. However, it is built on top of what was already there, giving it its warts.

It will be tricky to massage the ZipWriter API into being able to support this but we might be able to do it eventually. I'll leave this here so that we can keep it in mind as a goal for future changes.