Re: SVN 1.7.0. alpha source code

2011/6/15 Daniel Shahaf <d.s_at_daniel.shahaf.name>:
> Rainer Jung wrote on Tue, Jun 14, 2011 at 22:25:35 +0200:
>> On 14.06.2011 20:47, C. Michael Pilato wrote:
>> > On 06/14/2011 09:15 AM, C. Michael Pilato wrote:
>> >> On 06/14/2011 05:40 AM, Hyrum K Wright wrote:
>> >>> [ dropping infra from this bit, 'cause I believe it to be a local issue. ]
>> >>>
>> >>> In trying to debug a problem with our download page, I noticed that it
>> >>> appears we aren't respecting the Preferred query argument when
>> >>> selecting a mirror.  For instance, going to
>> >>>   http://subversion.apache.org/download/?Preferred=ftp%3A%2F%2Fftp.gbnet.net%2Fpub%2Fapache%2Fdist%2F
>> >>>
>> >>> doesn't select the ftp.gbnet.net mirror, as I believe it should.
>> >>> Instead the mirror cgi picks a mirror as if no Preferred flag was
>> >>> given.
>> >>
>> >> Ours isn't the only instance of a download.cgi script exhibiting this
>> >> problem, though.  See, for example, http://httpd.apache.org/download.cgi,
>> >> which appears to be having the same issue.
>> >
>> > Re-adding infra, as I believe there might be a more global issue here.  for
>> > example, when I visit http://httpd.apache.org/download.cgi and select any
>> > particular mirror and then click "Change", the mirror I selected is
>> > represented into the URL of the fetched page (in the Preferred= CGI query
>> > param), but the mirror I selected is *not* the one chosen as the preferred one.
>>
>> Sorry if I'm too late, or the following is not helpful. I noticed and
>> analyzed a similar (same?) problem in October and wrote to infra.
>>
>> Daniel Shahaf responded, but I don't know whether the fix was actually
>> applied.
>>
>> For details please have a look at the mail thread "Two recent bugs in
>> mirrors.cgi" between Daniel and me on October 22nd 2010 on the infra
>> list. I don't know whether there are public links, but the member mail
>> search finds it.
>>
>
> The relevant lists are privately archived, but download.cgi lives in
> a public repository,
> https://svn.apache.org/repos/asf/infrastructure/site/trunk/content/dyn/**/*.cgi
>

Looking at the mirrors.cgi script at
/site/trunk/content/dyn/mirrors/mirrors.cgi at the above link, I
think there is a bug there.

There is the following code fragment in parse_mirrors():

  # Check if the requested Preferred mirror is in the list
  # Note the user-requested mirror doesn't have a trailing-slash
  prefmir=None
  if preferred:
    for mir in mirrors:
      if mir[2][:-1]==preferred:
        prefmir=mir
        break

The "the user-requested mirror doesn't have a trailing-slash" comment
is wrong and thus the code following it is wrong.

The form that the user submits is generated from the data provided by
this very script and the mirrors there always have the trailing
slashes.

The values without slashes may come a) from bookmarks or links that
use hardcoded old value for "Preferred" argument, b) if something else
is using the mirrors database file directly without appending the
slashes.

So I think the following patch will fix this (not tested):
[[[
Index: mirrors.cgi
===================================================================
--- mirrors.cgi (revision 1135864)
+++ mirrors.cgi (working copy)
@@ -91,11 +91,11 @@
       break

   # Check if the requested Preferred mirror is in the list
- # Note the user-requested mirror doesn't have a trailing-slash
+ # Note the user-requested mirror may have no trailing-slash
   prefmir=None
   if preferred:
     for mir in mirrors:
- if mir[2][:-1]==preferred:
+ if mir[2].startswith(preferred):
         prefmir=mir
         break
   # Otherwise pick a preferred mirror from our country
]]]

Best regards,
Konstantin Kolinko