Message ID | 287ec1cd-00b7-b6b8-2fcd-13206ef08d5b@mail.com |
---|---|
State | Dropped |
Headers |
Return-Path: <development-bounces@lists.ipfire.org> Received: from mail01.ipfire.org (unknown [172.28.1.200]) by web02.ipfire.org (Postfix) with ESMTP id 23856600A1 for <patchwork@ipfire.org>; Fri, 29 Dec 2017 15:12:52 +0100 (CET) Received: from mail01.ipfire.org (localhost [IPv6:::1]) by mail01.ipfire.org (Postfix) with ESMTP id 1EA0BB80; Fri, 29 Dec 2017 15:12:50 +0100 (CET) Received: from mout.gmx.com (mout.gmx.com [74.208.4.200]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "mail.gmx.com", Issuer "thawte SSL CA - G2" (verified OK)) by mail01.ipfire.org (Postfix) with ESMTPS id 8FE84A7C for <development@lists.ipfire.org>; Fri, 29 Dec 2017 15:12:46 +0100 (CET) Received: from [192.168.3.1] ([41.79.25.253]) by mail.gmx.com (mrgmxus002 [74.208.5.15]) with ESMTPSA (Nemesis) id 0MfEdA-1eJJjG3SX4-00OpsK for <development@lists.ipfire.org>; Fri, 29 Dec 2017 15:12:33 +0100 Subject: [PATCH] Fix bug 10504: match download's sourceurl mangling in, updxlrator References: <6d686769-7667-5383-6dec-ba2d3dfc5be3@sil.org> To: development@lists.ipfire.org From: Justin Luth <jluth@mail.com> X-Forwarded-Message-Id: <6d686769-7667-5383-6dec-ba2d3dfc5be3@sil.org> Message-ID: <287ec1cd-00b7-b6b8-2fcd-13206ef08d5b@mail.com> Date: Fri, 29 Dec 2017 17:12:27 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <6d686769-7667-5383-6dec-ba2d3dfc5be3@sil.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-Provags-ID: V03:K0:Qam7HVEO1H6wTp7gvP5oOgoHdKutREKO97IkPDLJpoT6rKhZO3x 2cjuzvDLOaz7M69cJ0J7pZZcAN9rtOsUYbV+XPl9RSGOkROQc1+VKC1LyDN2NAG+N2xl9Mw Se/3aCStvI569iBhXpouzp/wU67pEzP2+d0Wrd1VbbkskYX4YUJVawMGJ7W6PVfZQHe2CbF 29v6NH0SV66riltm226Pw== X-UI-Out-Filterresults: notjunk:1; V01:K0:USCao/Na/iI=:TLY+4kwzgzAjm+9PmxyHk6 NY/um11uUCQbN4kWMs5EYaABlS3u7wqRM8ttmV/JyZtpQzhOUcse7NUjwWgpaykgZo8rJeY+C TWIJa72GmYzJSeNZeuYzB6cfsje58MtTAXmhLfVHLDwdIYF+DwE6sGl4j1EQKfB0sRDAlG4Qh t8q5yD3CnRgPqdRz0B7uUa0990FUIzCdVhc+QZGYt89V11eumkqUiROV8FFhbxXuOGGxTt4XW 5HU7tgRLprsVTLLZhkZd6KL8oaa1j5GfRFI9/Ofg3mnAyO5gdXGCHgeOcsxj3cl6quq6Ew5pE YaDg2MjR3BuCYeL/6HadK69dYukfWyixVnGap+YpqV76Xxy9QGyuVsyhkAYVDqBeVUr4iNAzo PbjJfKmfQhlO6X+4WXfFhxagWz5NGkCsQx77eOqH45ZVfXrRnF90IraJyoX7T/UVTOyIzSpkh xYeCyYxmoM/NYCgsBpnGYeXlkoets4BxruXiPsWX0y7jLUeJKpLIfxFsasrQ5swT9BhLn6TI7 We0JnZ0D4JNSwTu+rPmwEpGFz++ZUdg2dW3ZV0aQeJkusQtNJWh4bHADoarN3sWxNNt+Xylu2 T8rVkP/PxaEkQ8W+JuVmDl9Kt2ArrcrJ3bv0KvvhU1D6IffhActqWEe8DtkPHS6mGORW0DJcu 6n2jElj5Q9CeDTEfAghUbHtU5WS9Eyget5lgZLi1NgmuBPxAaqTei+KeulOSY/Bc/BwPdUk4o BkhrbV2lGsmOxu97GfVHMIMLq/A+TmeXnJ1ANwN64GUulFARsR4YWafo4tT2WmD009sbRi5FC FCCylFmn9C+P1IcM/QciiQOc6TQeec0xZ5uxDapmICGxzVU+b0= X-BeenThere: development@lists.ipfire.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: IPFire development talk <development.lists.ipfire.org> List-Unsubscribe: <https://lists.ipfire.org/mailman/options/development>, <mailto:development-request@lists.ipfire.org?subject=unsubscribe> List-Archive: <https://lists.ipfire.org/pipermail/development/> List-Post: <mailto:development@lists.ipfire.org> List-Help: <mailto:development-request@lists.ipfire.org?subject=help> List-Subscribe: <https://lists.ipfire.org/mailman/listinfo/development>, <mailto:development-request@lists.ipfire.org?subject=subscribe> Errors-To: development-bounces@lists.ipfire.org Sender: "Development" <development-bounces@lists.ipfire.org> |
Series |
Fix bug 10504: match download's sourceurl mangling in, updxlrator
|
|
Commit Message
Justin Luth
Dec. 30, 2017, 1:12 a.m. UTC
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.
This is fixed by making updxlrator mangle the url in the
same way as the downloader. apt-get install g++ would
be a good test for this.
Signed-off-by: Justin Luth <jluth@mail.com>
---
I submitted the bug report and attached this patch three years
ago, but the maintainer of updxlrator - although he
incorporated it into his own ipcop packagea - has never
released it (for ipcop) or any of the other promised updates that
he has been working on (in ipfire).
I have a few more fixes for updxlrator that I want to submit,
if this process goes well.
---
config/updxlrator/updxlrator | 2 ++
1 file changed, 2 insertions(+)
Comments
Hello Justin, and welcome to the team. Thanks for your submission. Indeed we have not been touching update accelerator much in the last years. Some people have been working on forks but never submitted their changes and just uploaded them somewhere. Therefore I am happy that you had a look. 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 > This is fixed by making updxlrator mangle the url in the > same way as the downloader. apt-get install g++ would > be a good test for this. > > Signed-off-by: Justin Luth <jluth@mail.com> > > --- > I submitted the bug report and attached this patch three years > ago, but the maintainer of updxlrator - although he > incorporated it into his own ipcop packagea - has never > released it (for ipcop) or any of the other promised updates that > he has been working on (in ipfire). > > I have a few more fixes for updxlrator that I want to submit, > if this process goes well. > --- > config/updxlrator/updxlrator | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/config/updxlrator/updxlrator b/config/updxlrator/updxlrator > index 2ddc6d8e4..b728902f6 100644 > --- a/config/updxlrator/updxlrator > +++ b/config/updxlrator/updxlrator > @@ -345,7 +345,9 @@ sub check_cache > my $sourceurl=$_[0]; > my $cfmirror=$_[4]; > > + $sourceurl =~ s@\%2b@+@ig; > $sourceurl =~ s@\%2f@/@ig; > + $sourceurl =~ s@\%7e@~@ig; > $updfile = substr($sourceurl,rindex($sourceurl,"/")+1); > $updfile =~ s@\%20@ @ig; >
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
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 > > >
diff --git a/config/updxlrator/updxlrator b/config/updxlrator/updxlrator index 2ddc6d8e4..b728902f6 100644 --- a/config/updxlrator/updxlrator +++ b/config/updxlrator/updxlrator @@ -345,7 +345,9 @@ sub check_cache my $sourceurl=$_[0]; my $cfmirror=$_[4]; + $sourceurl =~ s@\%2b@+@ig; $sourceurl =~ s@\%2f@/@ig; + $sourceurl =~ s@\%7e@~@ig; $updfile = substr($sourceurl,rindex($sourceurl,"/")+1); $updfile =~ s@\%20@ @ig;