Hi,
This patch fix the autoupdater url error message. If the mirror url inside the site.conf ends with "/". The autoupdater will add additionally a "/" without checking. This patch add a check of endig urls.
--- admin/autoupdater/files/usr/sbin/autoupdater | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/admin/autoupdater/files/usr/sbin/autoupdater b/admin/autoupdater/files/usr/sbin/autoupdater index be3e393..716fce4 100755 --- a/admin/autoupdater/files/usr/sbin/autoupdater +++ b/admin/autoupdater/files/usr/sbin/autoupdater @@ -188,9 +188,15 @@ local function read_manifest(mirror) return ret end
+function string.ends(String,End) + return End=='' or string.sub(String,-string.len(End))==End +end
-- Downloads the firmware image from a mirror to a given output file local function fetch_firmware(mirror, filename, output) + if string.ends(mirror, "/") then + mirror = string.sub(mirror,1,-2) + end if autoupdater_util.exec('wget', '-T', '120', '-O', output, mirror .. '/' .. filename) ~= 0 then io.stderr:write('Error downloading the image from ' .. mirror .. '\n') return false
On 07/14/2016 08:47 PM, Jan-Tarek Butt wrote:
Hi,
This patch fix the autoupdater url error message. If the mirror url inside the site.conf ends with "/". The autoupdater will add additionally a "/" without checking. This patch add a check of endig urls.
What webservers actually have problems with double slashes? I've tested with Nginx, which doesn't care.
admin/autoupdater/files/usr/sbin/autoupdater | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/admin/autoupdater/files/usr/sbin/autoupdater b/admin/autoupdater/files/usr/sbin/autoupdater index be3e393..716fce4 100755 --- a/admin/autoupdater/files/usr/sbin/autoupdater +++ b/admin/autoupdater/files/usr/sbin/autoupdater @@ -188,9 +188,15 @@ local function read_manifest(mirror) return ret end +function string.ends(String,End)
- return End=='' or string.sub(String,-string.len(End))==End
+end -- Downloads the firmware image from a mirror to a given output file local function fetch_firmware(mirror, filename, output)
- if string.ends(mirror, "/") then
- mirror = string.sub(mirror,1,-2)
- end if autoupdater_util.exec('wget', '-T', '120', '-O', output, mirror .. '/' .. filename) ~= 0 then io.stderr:write('Error downloading the image from ' .. mirror .. '\n') return false
This patch fix the autoupdater url error message. If the mirror url inside the site.conf ends with "/". The autoupdater will add additionally a "/" without checking. This patch add a check of endig urls.
What webservers actually have problems with double slashes? I've tested with Nginx, which doesn't care.
dont know, but the nginx logs are full of messages. Also it is not realy nice if it just ignored.
cheers Tarek
On 07/14/2016 09:29 PM, Jan-Tarek Butt wrote:
This patch fix the autoupdater url error message. If the mirror url inside the site.conf ends with "/". The autoupdater will add additionally a "/" without checking. This patch add a check of endig urls.
What webservers actually have problems with double slashes? I've tested with Nginx, which doesn't care.
dont know, but the nginx logs are full of messages. Also it is not realy nice if it just ignored.
cheers Tarek
I can't find anything wrong with it, our nginx doesn't print any errors. Single and double slash should be equivalent, so client software doesn't need to care.
Hi,
Am Thu, 14 Jul 2016 21:56:20 +0200 schrieb Matthias Schiffer mschiffer@universe-factory.net:
Single and double slash should be equivalent, so client software doesn't need to care.
They may feel equivalent, but they are not :)
See https://tools.ietf.org/html/rfc3986#page-13: "Thus, characters in the reserved set are protected from normalization ..."
The "equivalence" decision process is specified here: https://tools.ietf.org/html/rfc3986#page-39 Removal of double slashes is not listed (e.g. in "Syntax-Based Normalization").
Here is the condensed recipe (extracted from the standards) for serializing a URL (which is used for testing two URLs for equivalence): https://url.spec.whatwg.org/#url-serializing
Quote from above: [..] append "/", followed by the strings in url’s path (including empty strings), separated from each other by "/" [..]
Thus multiple consecutive slashes shall not be reduced to one.
Nevertheless nginx is kind enough to work around duplicate slashes by default: http://nginx.org/en/docs/http/ngx_http_core_module.html#merge_slashes
According to the RFCs above, I think the autoupdater should not request URLs with duplicate slashes, if it intends to retrieve a different URL with only single slashes.
Lars
On 07/15/2016 02:29 AM, Lars Kruse wrote:
Hi,
Am Thu, 14 Jul 2016 21:56:20 +0200 schrieb Matthias Schiffer mschiffer@universe-factory.net:
Single and double slash should be equivalent, so client software doesn't need to care.
They may feel equivalent, but they are not :)
See https://tools.ietf.org/html/rfc3986#page-13: "Thus, characters in the reserved set are protected from normalization ..."
The "equivalence" decision process is specified here: https://tools.ietf.org/html/rfc3986#page-39 Removal of double slashes is not listed (e.g. in "Syntax-Based Normalization").
Here is the condensed recipe (extracted from the standards) for serializing a URL (which is used for testing two URLs for equivalence): https://url.spec.whatwg.org/#url-serializing
Quote from above: [..] append "/", followed by the strings in url’s path (including empty strings), separated from each other by "/" [..]
Thus multiple consecutive slashes shall not be reduced to one.
Nevertheless nginx is kind enough to work around duplicate slashes by default: http://nginx.org/en/docs/http/ngx_http_core_module.html#merge_slashes
According to the RFCs above, I think the autoupdater should not request URLs with duplicate slashes, if it intends to retrieve a different URL with only single slashes.
Lars
I meant equivalence on filesystem level, not on HTTP level, so the RFCs don't really apply... But I guess there might be webservers and filesystems which don't like double slashes.
Well, I've added a simpler fix, just to be sure. This is probably moot, as the Lua-based autoupdater won't make it into another major Gluon release, but will be replaced completely.
-- NeoRaider
-
Jan-Tarek Butt -
Lars Kruse -
Matthias Schiffer