From mboxrd@z Thu Jan 1 00:00:00 1970 From: Justin Luth To: development@lists.ipfire.org Subject: Re: [PATCH] Fix bug 10504: match download's sourceurl mangling in, updxlrator Date: Sat, 30 Dec 2017 18:57:53 +0300 Message-ID: In-Reply-To: <1514644060.3685.11.camel@ipfire.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============7107613517658892465==" List-Id: --===============7107613517658892465== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On 12/30/2017 05:27 PM, Michael Tremer wrote: > On Fri, 2017-12-29 at 17:12 +0300, Justin Luth wrote: >> Updatexlrator stores its files in a hash of the URL. >> >> The download utility mangles the URL for [+/~], but >> the updxlrator only does it for [/]. Thus, download >> stores the result as one hash, and updxlrator looks for it >> with a different hash. The result is that the file is >> re-downloaded every time by both the client, and updxlrator. > Wouldn't it be a better idea to generally escape/unescape the URLs? There i= s a > perl module that does that: > > http://search.cpan.org/dist/URI/lib/URI/Escape.pm > > Your changes certainly make sense, but there are more characters that could > cause the same problem here. > > Let me know if that would make sense, too. > > Best, > -Michael I don't know what the impact would be of unescaping more characters. It=20 might affect negatively affect cached downloads - causing them to be redownloaded. So I am=20 reluctant to do anything other than to fix the obvious bug, especially since I'm not at all a=20 perl programmer. Your portrayal of "my patch fixes some things, but others could cause=20 the same problem" isn't exactly accurate. I'm only fixing the "one module does it one way, but=20 the other module does it another way" problem. In that sense, the two modules are now=20 identical, and so there are no more similar changes that can be made. I'm not sure what "problem" was caused by these characters in the first=20 place, and why they needed to be unescaped. The safest thing is to simply match the download=20 mangling and leave it at that. That's all that I'm comfortable changing - I'm happy if a real=20 programmer takes over from here and looks at the "duplicates" like bug 10344. Updatexlrator is the main tool that keeps us using IPFire, so thanks for=20 looking at my patches. I have one more big one (bug 11558) that I've seriously reworked today=20 and want to test on my IPFire to confirm that it still works as expected. One consideration is that many other people may have made changes to=20 updxlrator (for adding additional caches for example) and since it has been unchanged for so=20 long they might unexpectedly lose those customizations. So, a warning at least is due in=20 the release notes. It might also be good to just hold off on these patches until I get my 4th=20 one approved. In any case, I hope to test it heavily next week, and get it submitted. Justin --===============7107613517658892465==--