Downloading smaller file with NoResume doesn't truncate

[frontierpsycho]

This is related to #23 and should have been fixed by 799551cb35da12b490c245c195e7293d7e11e591, but it's not. I'm not sure whether it's expected behaviour, but I don't think it is.

We have a file on disk, and we're downloading a newer version of it with grab. If the newer version is smaller, the file is not truncated, even with NoResume enabled.

I followed the code path (using good ol' fmt.Printf :) ) and discovered that this is because none of the blocks that set writeFlags are accessed in our case. In validateLocal, we exit before the NoResume block is reached, via this block:

        // determine expected file size
    size := resp.Request.Size
    if size == 0 && resp.HTTPResponse != nil {
        size = resp.HTTPResponse.ContentLength
    }
    if size == 0 {
        return c.headRequest
    }

as size is zero.

Then, in headRequest, there's this block:

        if resp.Request.NoResume {
        return c.getRequest
    }

that doesn't add the proper writeFlags. This means that when we get to openWriter, the writeFlags are untouched, as initialized by Do:

        writeFlags: os.O_CREATE | os.O_WRONLY,

And thus the file is not truncated.

What I did as a temporary fix is to set the flags in headRequest, just like you do in validateLocal:

        if resp.Request.NoResume {
                resp.writeFlags = os.O_TRUNC | os.O_WRONLY
        return c.getRequest
    }

However, I think that perhaps the NoResume check and the setting of the flags should be in one place, rather than two, but I'm not familiar enough with the library to know where that would be, so I thought I'd report this and leave it to you. Sorry this isn't a pull request, we're not using github plus my time is unfortunately limited at the moment.

[frontierpsycho]

Oh, and one more thing: we also ran into a problem with only O_TRUNC: it deleted our files. To fix it, we needed to change it to:

resp.writeFlags = os.O_CREATE | os.O_TRUNC | os.O_WRONLY

Might be related or not, just mentioning it.

[cavaliercoder]

Thanks for raising this. It was a bug as you described. The desired behavior if the remote file is smaller than the local file is:

• return ErrBadSize
• or truncate the local file if Request.NoResume is true

I've cleaned up the code so all write flags are set in one place (openWriter) and added a test to catch regressions for this issue.

[frontierpsycho]

Thanks a lot! Glad I could help!