Re: [PATCH] Fix bug 10504: match download's sourceurl mangling in, updxlrator

[Michael Tremer <michael.tremer@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 2910 bytes --]

Hello,

I just merged all the patches into next so they are queued for Core
Update 118 now.

Please have a look that everything got applied correctly.

And of course please test :)

Best,
-Michael

On Sat, 2017-12-30 at 18:57 +0300, Justin Luth wrote:
> 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 is 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 
> might affect negatively
> affect cached downloads - causing them to be redownloaded. So I am 
> reluctant to do anything
> other than to fix the obvious bug, especially since I'm not at all a 
> perl programmer.
> 
> Your portrayal of "my patch fixes some things, but others could cause 
> the same problem" isn't
> exactly accurate. I'm only fixing the "one module does it one way, but 
> the other module
> does it another way" problem. In that sense, the two modules are now 
> 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 
> place, and why they needed
> to be unescaped. The safest thing is to simply match the download 
> mangling and leave it at that.
> That's all that I'm comfortable changing - I'm happy if a real 
> 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 
> looking at my patches.
> I have one more big one (bug 11558) that I've seriously reworked today 
> 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 
> updxlrator (for adding
> additional caches for example) and since it has been unchanged for so 
> long they might
> unexpectedly lose those customizations. So, a warning at least is due in 
> the release notes. It
> might also be good to just hold off on these patches until I get my 4th 
> one approved. In any case, I hope
> to test it heavily next week, and get it submitted.
> Justin
> 
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]