More-elegant solution wanted for "-k" flag

[DeeDeeG]

Edit: The main bug has been fixed, so scroll down to here for where this is at now: #33 (comment)

(Original comment below.)

---

Hi folks.

Just filing a quick bug report...

Steps to Reproduce:

• Have the full payload file downloaded onto your local hard-drive
  • (If it isn't saved to the current directory, user can use the -i flag or -o flag to point zsync2 to where the payload is. Doesn't matter for this bug.)
• Use the -k flag of zsync2, telling zsync2 to save the .zsync file locally

Expected Results

zsync2 should save the .zsync file to my local hard-drive if the -k flag is used.

Actual results

If zsync2 analyzes the payload on user's hard-drive, and sees that it is fully up-to-date, zsync2 will not save the .zsync file on the user's hard-disk.

[DeeDeeG]

P.S.:
Thanks for making this project. It's exactly what I was looking for. zsync is a valuable tool, but as it stands, it has some updates I'd like to see made. This project basically makes the updates I was hoping to see in zsync.

[TheAssassin]

Shouldn't be too hard to fix. Will look into it.

[TheAssassin]

Will quick-fix this in a PR. Then, the .zsync file will be stored every time it is fetched from the Internet (might occur 2-4 times then, but that's an acceptable trade-off IMO).

[TheAssassin]

Seems to work from my quick test. Please try and reopen if the issue persists.

[DeeDeeG]

I tried it again (built from the latest commit, fcfad40) and it worked.

(When using the -k option, the .zsync file is saved to my hard-drive now, even if the payload file is totally up-to-date.)

[DeeDeeG]

Hmm...

Actually, latest zsync2 (fcfad40) doesn't totally fix the bug. It doesn't save a valid .zsync file now, only the first 1024 bytes.

I noticed something strange with Wireshark:

Whenever zsync2 runs, it does an HTTP RANGE request for the first 1024 bytes of the .zsync file... (It does this twice in a row). Then, zsync2 requests the full .zsync file (also twice??) and saves it to the disk if the -k flag is given. There is still a problem if the payload is already up-to-date, though.

Status of this bug before PR #37:

• zsync2 would not save any .zsync file to disk when payload file is up-to-date

Status of this bug after PR #37:

• zsync2 only saves the first 1024-bits of the .zsync file to disk when the payload file is already up-to-date

[TheAssassin]

AppImageUpdate tries to minimize the amount of data it needs to download for a simple update check. It tries to download only the .zsync header with the hash, and since we only have that data, we can only write that out.

We'd have to treat -k separately it seems.

Why it's downloading the full file twice? No idea, doesn't make very much sense to me either. Please open a separate issue.

[DeeDeeG]

AppImageUpdate tries to minimize the amount of data it needs to download for a simple update check. It tries to download only the .zsync header with the hash [. . .]

Yes, I can see why the first 1024 bytes would be useful. For reference if anyone reading this is curious, here is some info I can see from the first 1024 bytes of the disco desktop daily image's .zsync file...

[me]@[computer]:~/$ cat ~/.zsyncs/disco-desktop-amd64.iso.zsync 
zsync: 0.6.2
Filename: disco-desktop-amd64.iso
MTime: Wed, 02 Jan 2019 03:13:24 +0000
Blocksize: 4096
Length: 2132803584
Hash-Lengths: 2,3,5
URL: disco-desktop-amd64.iso
SHA-1: c81d78492024546c71fb15cd205ba9b153e6755e

[a few bytes of binary data here (zsync chunk hashes, I think?)]

[TheAssassin]

Yeah, that's chunk data following the header fields. 1024 bytes just made sense as a block size for downloading those data. The issue was there's MiB-size .zsync files for instance, but even 100kiB can be too much if downloaded every hour by some update check etc.

[TheAssassin]

So, "being smart" is a bit difficult here. There's two ways what we could do here:

a) keep "being smart": add some sort of state variable to see whether the .zsync file has been written out already, deactivate header-only request until the file has been written out

b) just make an extra request

[DeeDeeG]

By the way, original zsync with the -k flag makes an HTTP request with an "If-Modified-Since [Mtime listed in the local .zsync file]".

Assuming the local .zsync file is up-to-date, the zsync program may not have to download any metadata.

Edit: Server returns a "304 Not Modified" response if there are no changes.

Edit 2: The date zsync uses actually is simpler. It's "If-Modified-Since [modification date/time of local .zsync file on disk]"

[TheAssassin]

We've decided earlier that this is not reliable enough for update checks etc. (CC @pamarcos), therefore we make these header-only requests at all. I think this is sort-of overengineering, we should just make a full request and overwrite the original file. At least I don't have time to invest in this stuff now. PRs welcome.

For now, I'm going to implement a). I'll send a PR in a minute, please try the binaries built there.

[DeeDeeG]

I added this in an edit to the previous omment, so I'm not sure if you saw it:

Original zsync actually uses the modification date on disk of the .zsync file. Not sure if that is easier to use than reading from the .zsync file.

I think this is sort-of overengineering, we should just make a full request and overwrite the original file. At least I don't have time to invest in this stuff now. PRs welcome.

Yeah, I'm not sure how many people even use -k other than myself, so I'd understand not throwing a lot of resources at this. (I'm not much of a programmer at the moment, never wrote much beyond "hello world" examples... But if I had the know-how, I would definitely contribute a PR.)

[TheAssassin]

We'll simply leave the issue open for someone who's interested. I wish I could invest 2 weeks and revamp zsync2...

[DeeDeeG]

Could rename the issue to something like "Need a more elegant solution for -k" since the main thing is fixed now.

[TheAssassin]

If you can edit it, please feel free to do so.

[DeeDeeG]

Now that #37 and #39 are merged, there are up to four requests for the whole .zsync file. (For more details, I made a comment over in #38: #38 (comment))

While it's nice for the -k flag to work perfectly (full functionality and works consistently), I wonder if it is worth the extra network bandwidth/data being used. I appreciate the effort and the quick responses to my pull requests... but maybe in the short-term it would be better to revert #37 and #39 in master? They can go in a WIP branch for fixing up -k, maybe.

I don't want to cause a regression in zsync2's most-popular use-case, which I think is as part of AppImage's updater tool.