Squirrel.Windows: Spaces in filename cause ReleaseEntry Regex to fail
I’m in a situation where Squirrel updating fails when iterating over one particular release entry
E6651B768DB82076E7113086558548B4DA4F8F6F Artifact - BoxHeader - e7ee795c-99e1-4ecb-a72c-9a41461d8198.xml.shasum 8669
using this Regex:
^([0-9a-fA-F]{40})\s+(\S+)\s+(\d+)[\r]*$
The culprit appears to be that (\S+)
will only match non-whitespace characters. I’ve played a bit around with the Regex Tester at http://regexstorm.net/tester, and my suggestion would be to change the Regex to something similar to
^([0-9a-fA-F]{40})\s+(.*).shasum[\s+](\d+)[\r]*$
We don’t really want to be forced to change the filenames or escape the spaces, so I’m interested in a solution where we can avoid that. Thanks in advance.
About this issue
- Original URL
- State: closed
- Created 6 years ago
- Reactions: 3
- Comments: 23 (21 by maintainers)
Commits related to this issue
- Fix #1283 (Spaces in filename cause ReleaseEntry Regex to fail) — committed to cbenard/Squirrel.Windows by cbenard 6 years ago
- Fix #1283 (Spaces in filename cause ReleaseEntry Regex to fail) — committed to cbenard/Squirrel.Windows by cbenard 6 years ago
Having the same problem. Any time Squirrel decides to use a .diff (or .bsdiff) to represent the difference between old and new versions of a file, it generates a corresponding .shasum file along with the .diff. Here’s an example of the shasum contents for one of our packages:
09E36653DEFCAFEDA55115AD81AC84E1B8D4A1FF Install Bloom Literacy Fonts.exe.shasum 12992
When applying the patch in DeltaPackage.applyDiffToFile, Squirrel calls VerifyPatchedFile(), which reads the .shasum file and passes the contents to ReleaseEntry.ParseReleaseEntry(), which fails to parse it because it can’t match the spaces in the file name.
Squirrel then concludes that the patch is faulty, aborts the incremental update, and downloads the whole package, defeating the incremental update feature entirely.
I agree that Frederik’s fix (or even just changing (\S+) to (.+) would fix the problem.
Got the PR ready for you @paulcbetts after @JohnThomson and I worked through the regex. I hope you’re able to accept the PR.
That one looks good. Consider it co-signed.
^([0-9a-fA-F]{40})\s+(\S[\S ]*?)\s+(\d+)[\r]*$
Now if we can only get @paulcbetts to look at this issue still open from April… 😃
Must have been really sleepy yesterday.
^([0-9a-fA-F]{40})\s+(\S[\S ]*?)\s+(\d+)[\r]*$
.OK, so what I intended to suggest was
(\S.*?)
, or in full^([0-9a-fA-F]{40})\s+(\S.*?)\s+(\d+)\r*$
Thanks.I assumed we were talking about matching per-line. You’re right about the 3 spaces thing. I didn’t even see the original regex which is a bit more complicated than it needs to be.
It does seem like a pretty easy fix, especially if you know each line begins with 40 upper-case hex characters for the hash and ends with a set of numbers for length and has a filename in the middle delineated with spaces before/after. I’m surprised it’s still open. I was subscribed to it for some reason, but I can’t even remember if I had the issue.