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

[Justin Luth <jluth@mail.com>]

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

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