[OSM-dev] trace page bug

Lars Aronsson lars at aronsson.se
Wed Jul 26 21:50:19 BST 2006


I fail to mark all my traces as public.  I have 88 traces that are 
listed 20 per page.  When I go to page 3 and mark all traces 
(41-60 of 88) as public, all other (1-40 and 61-88) become 
private.

This is due to a trivial logic error in the SVN source file 
www.openstreetmap.org/eruby/upload-gpx.rhtml which I could fix if 
this was C, Java or Perl, but I simply lack experience in the 
details of Ruby to hack this myself.

Here is the current code, starting on line 72:

if loggedin
  if cgi['action'] == 'update'
    dao.gpx_ids_for_user(user_id).each_hash do |row|
      id = row['id']
      rm = cgi["rm_#{id}"] == 'on'
      priv = cgi["priv_#{id}"] == 'on'
      if rm
        dao.schedule_gpx_delete(id.to_i)
        %>
        <i>The gpx file <%= row['name'] %> has been scheduled for deletion.</i><
br>
        <%
      end
      dao.gpx_set_private(id.to_i, !priv) if dao.does_user_own_gpx?(user_id, id.to_i)
    end
  end

At first, it seems backwards to loop over all traces belonging to 
that user. One would think the script should loop over the traces 
mentioned in the CGI call.  However, that could be a security risk 
when users try to modify traces belonging to others and the 
current design isn't all that bad.  The handling of deletes is 
also fully OK. If the loop is changed, it would have to call 
"dao.does_user_own_gpx?" for every trace.

The current problem is with "priv".  First the name is bad, since 
the "on" value indicates publicity and not privacy, which is 
reflected in the !priv argument to the dao function.  The variable 
should perhaps be named "publ" so that "!publ" is the argument.

But the real bug is that this is kept "on" (public) only if the 
trace was mentioned in this CGI call.  All other traces (in my 
case 1-40 and 61-88) will be marked "off" (private).  The 
dao.gpx_set_private() function that modifies the priv/publ status, 
should be called *only* if the variable was present among the 
current CGI variables.  The test "if dao.does_user_own_gpx?" that 
follows is designed for the case that we loop over the present CGI 
variables.  Perhaps the loop should be altered after all?

As I think you all agree, this is a very long description of a 
very trivial bug.  I have not entered it into trac, because I 
don't know how to write it any shorter.  I hope someone else can 
take it from here.


-- 
  Lars Aronsson (lars at aronsson.se)
  Aronsson Datateknik - http://aronsson.se




More information about the dev mailing list