Merged 17256:18123 from trunk.
authorTom Hughes <tom@compton.nu>
Tue, 13 Oct 2009 20:06:24 +0000 (20:06 +0000)
committerTom Hughes <tom@compton.nu>
Tue, 13 Oct 2009 20:06:24 +0000 (20:06 +0000)
1  2 
app/controllers/application_controller.rb
app/controllers/trace_controller.rb
app/views/layouts/site.html.erb
app/views/site/edit.html.erb
test/functional/changeset_controller_test.rb

index 6dbe9165ca2593b20f5fff3c7e6c46f7469016e6,e36c9842ba6c55eea2e652d453b60d16732bc86e..bcaed456590748d1e86c45ba04962ffbc1af1af7
@@@ -39,6 -39,19 +39,19 @@@ class ApplicationController < ActionCon
      end
    end
  
+   ##
+   # require the user to have cookies enabled in their browser
+   def require_cookies
+     if request.cookies["_osm_session"].to_s == ""
+       if params[:cookie_test].nil?
+         redirect_to params.merge(:cookie_test => "true")
+         return false
+       else
+         @notice = t 'application.require_cookies.cookies_needed'
+       end
+     end
+   end
    # Utility methods to make the controller filter methods easier to read and write.
    def require_allow_read_prefs
      require_capability(:allow_read_prefs)
          @user = User.authenticate(:username => username, :password => passwd) # basic auth
        end
      end
+     # check if the user has been banned
+     unless @user.nil? or @user.active_blocks.empty?
+       # NOTE: need slightly more helpful message than this.
+       render :text => t('application.setup_user_auth.blocked'), :status => :forbidden
+     end
    end
  
    def authorize(realm='Web Password', errormessage="Couldn't authenticate you") 
      rescue OSM::APIError => ex
        report_error ex.message, ex.status
      rescue Exception => ex
 -      logger.info("API threw unexpected #{ex.class} exception: #{ex.message}")
 -      ex.backtrace.each { |l| logger.info(l) }
        report_error "#{ex.class}: #{ex.message}", :internal_server_error
      end
    end
index f06a162fbc00b850d03b971cc7a59ae9ad60ec62,84026e8cd4d420f6f437778df8924db6c6179777..c90558269a6b0b497c7a8a37275846924494753c
@@@ -11,7 -11,8 +11,8 @@@ class TraceController < ApplicationCont
    before_filter :check_api_writable, :only => [:api_create]
    before_filter :require_allow_read_gpx, :only => [:api_details, :api_data]
    before_filter :require_allow_write_gpx, :only => [:api_create]
-  
+   around_filter :api_call_handle_error, :only => [:api_details, :api_data, :api_create]
    # Counts and selects pages of GPX traces for various criteria (by user, tags, public etc.).
    #  target_user - if set, specifies the user to fetch traces for.  if not set will fetch all traces
    def list(target_user = nil, action = "list")
      # 4 - user's traces, not logged in as that user = all user's public traces
      if target_user.nil? # all traces
        if @user
 -        conditions = ["(gpx_files.visibility in ('public', 'identifiable') OR gpx_files.user_id = ?)", @user.id] #1
 +        conditions = ["(gpx_files.visibility <> 'private' OR gpx_files.user_id = ?)", @user.id] #1
        else
 -        conditions  = ["gpx_files.visibility in ('public', 'identifiable')"] #2
 +        conditions  = ["gpx_files.visibility <> 'private'"] #2
        end
      else
        if @user and @user == target_user
          conditions = ["gpx_files.user_id = ?", @user.id] #3 (check vs user id, so no join + can't pick up non-public traces by changing name)
        else
 -        conditions = ["gpx_files.visibility in ('public', 'identifiable') AND gpx_files.user_id = ?", target_user.id] #4
 +        conditions = ["gpx_files.public <> 'private' AND gpx_files.user_id = ?", target_user.id] #4
        end
      end
      
      if params[:trace]
        logger.info(params[:trace][:gpx_file].class.name)
        if params[:trace][:gpx_file].respond_to?(:read)
-         do_create(params[:trace][:gpx_file], params[:trace][:tagstring],
-                   params[:trace][:description], params[:trace][:visibility])
+         begin
+           do_create(params[:trace][:gpx_file], params[:trace][:tagstring],
+                     params[:trace][:description], params[:trace][:visibility])
+         rescue
+         end
  
          if @trace.id
            logger.info("id is #{@trace.id}")
    end
  
    def georss
 -    conditions = ["gpx_files.visibility in ('public', 'identifiable')"]
 +    conditions = ["gpx_files.visibility <> 'private'"]
  
      if params[:display_name]
        conditions[0] += " AND users.display_name = ?"
      if request.post?
        tags = params[:tags] || ""
        description = params[:description] || ""
-       visibility = params[:visibility] || false
+       visibility = params[:visibility]
  
-       if params[:public] && !visibility
-         visibility = "public"
+       if visibility.nil?
+         if params[:public] && params[:public].to_i.nonzero?
+           visibility = "public"
+         else
+           visibility = "private"
+         end
        end
-       
        if params[:file].respond_to?(:read)
          do_create(params[:file], tags, description, visibility)
  
@@@ -341,20 -349,35 +349,35 @@@ privat
        :timestamp => Time.now.getutc
      })
  
-     # Save the trace object
-     if @trace.save
-       # Rename the temporary file to the final name
-       FileUtils.mv(filename, @trace.trace_name)
+     Trace.transaction do
+       begin
+         # Save the trace object
+         @trace.save!
  
-       # Clear the inserted flag to make the import daemon load the trace
-       @trace.inserted = false
-       @trace.save!
-     else
-       # Remove the file as we have failed to update the database
-       FileUtils.rm_f(filename)
+         # Rename the temporary file to the final name
+         FileUtils.mv(filename, @trace.trace_name)
+       rescue Exception => ex
+         # Remove the file as we have failed to update the database
+         FileUtils.rm_f(filename)
+         # Pass the exception on
+         raise
+       end
+       begin
+         # Clear the inserted flag to make the import daemon load the trace
+         @trace.inserted = false
+         @trace.save!
+       rescue Exception => ex
+         # Remove the file as we have failed to update the database
+         FileUtils.rm_f(@trace.trace_name)
+         # Pass the exception on
+         raise
+       end
      end
-     
-     # Finally save the user's preferred previacy level
+     # Finally save the user's preferred privacy level
      if pref = @user.preferences.find(:first, :conditions => {:k => "gps.trace.visibility"})
        pref.v = visibility
        pref.save
index f072544aa21367fdbbbb0d55c41dc5998baadd4c,1e4e56e6acf9e14de9807ed526bdcd1987fda2c4..d1983a9f0ba2e4cb72c62517bfe7406fd5b9b9a1
@@@ -1,14 -1,11 +1,15 @@@
  <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
  <html xmlns="http://www.w3.org/1999/xhtml" xml:lang="<%= I18n.locale %>" lang="<%= I18n.locale %>" dir="<%= t'html.dir' %>">
    <head>
 +    <meta name="viewport" content="width=device-width, minimum-scale=1.0, maximum-scale=1.0"/>
+     <%= javascript_strings %>
      <%= javascript_include_tag 'prototype' %>
      <%= javascript_include_tag 'site' %>
      <!--[if lt IE 7]><%= javascript_include_tag 'pngfix' %><![endif]--> <!-- thanks, microsoft! -->
 -    <%= stylesheet_link_tag 'site' %>
 +    <%= stylesheet_link_tag 'common' %>
 +    <!--[if IE]><%= stylesheet_link_tag 'site', :media => "screen" %><![endif]--> <!-- IE is totally broken with CSS media queries -->
 +    <%= stylesheet_link_tag 'site-sml', :media => "only screen and (max-width: 481px)" %>
 +    <%= stylesheet_link_tag 'site', :media => "screen and (min-width: 482px)" %> 
      <%= stylesheet_link_tag 'print', :media => "print" %>
      <%= tag("link", { :rel => "search", :type => "application/opensearchdescription+xml", :title => "OpenStreetMap Search", :href => "/opensearch/osm.xml" }) %>
      <%= tag("meta", { :name => "description", :content => "OpenStreetMap is the free wiki world map." }) %>
@@@ -26,8 -23,7 +27,8 @@@
  
      <span id="greeting">
        <% if @user and @user.id %>
 -        <%= t 'layouts.welcome_user', :user_link => (link_to h(@user.display_name), {:controller => 'user', :action => 'view', :display_name => @user.display_name}, :title => t('layouts.welcome_user_link_tooltip')) %> | 
 +        <span id="full-greeting"><%= t 'layouts.welcome_user', :user_link => (link_to h(@user.display_name), {:controller => 'user', :action => 'view', :display_name => @user.display_name}, :title => t('layouts.welcome_user_link_tooltip')) %></span> 
 +        <span id="small-greeting"><%= link_to t('layouts.welcome_user_link_tooltip'), {:controller => 'user', :action => 'view', :display_name => @user.display_name} %></span> | 
          <%= yield :greeting %>
          <%
          inbox_attributes = {}
index 142d7a3f312f1678b2a890c49bd7ab4144a1658e,b29c0f200e84032f4b0464f98a29a1a436f00c9f..51389afc16181b34c6348146cfb8bbefe6da57ca
@@@ -12,7 -12,7 +12,7 @@@
  <% else %>
  <% content_for :greeting do %>
  <% if @user and !@user.home_lon.nil? and !@user.home_lat.nil? %>
 -<%= link_to_function t('layouts.home'), "setPosition(#{@user.home_lat}, #{@user.home_lon}, 10)", { :title => t('layouts.home_tooltip') } %> |
 +<%= link_to_function 'home', "setPosition(#{@user.home_lat}, #{@user.home_lon}, 10)" %> |
  <% end %>
  <% end %>
  
@@@ -65,7 -65,7 +65,7 @@@ zoom='14' if zoom.nil
    
    window.onbeforeunload=function() {
      if (!changesaved) {
-       return "<%= t 'site.edit.potlatch_unsaved_changes' %>";
+       return '<%= escape_javascript(t('site.edit.potlatch_unsaved_changes')) %>';
      }
    }
  
      fo.addVariable('token','<%= session[:token] %>');
      if (lat) { fo.addVariable('lat',lat); }
      if (lon) { fo.addVariable('long',lon); }
-     <% if params['gpx']  %>fo.addVariable('gpx' ,'<%= h(params['gpx'] ) %>');<% end %>
-     <% if params['way']  %>fo.addVariable('way' ,'<%= h(params['way'] ) %>');<% end %>
-     <% if params['node'] %>fo.addVariable('node','<%= h(params['node']) %>');<% end %>
+     <% if params['gpx']     %>fo.addVariable('gpx'     ,'<%= h(params['gpx']    ) %>');<% end %>
+     <% if params['way']     %>fo.addVariable('way'     ,'<%= h(params['way']    ) %>');<% end %>
+     <% if params['node']    %>fo.addVariable('node'    ,'<%= h(params['node']   ) %>');<% end %>
+     <% if params['tileurl'] %>fo.addVariable('custombg','<%= h(params['tileurl']) %>');<% end %>
      fo.write("map");
    }
  
index 46f8a1a6a6990ed0a34658018399a06db8f312b8,76ec0866adcf9899df439126009147ef95b583cd..a840dd2338e8b85b9b8fff30222063705c02c6e9
@@@ -419,6 -419,57 +419,6 @@@ EO
      end
    end
  
 -  def test_upload_large_changeset
 -    basic_authorization users(:public_user).email, "test"
 -
 -    # create a changeset
 -    content "<osm><changeset/></osm>"
 -    put :create
 -    assert_response :success, "Should be able to create a changeset: #{@response.body}"
 -    changeset_id = @response.body.to_i
 -    
 -    # upload some widely-spaced nodes, spiralling positive and negative to cause 
 -    # largest bbox over-expansion possible.
 -    diff = <<EOF
 -<osmChange>
 - <create>
 -  <node id='-1' lon='-20' lat='-10' changeset='#{changeset_id}'/>
 -  <node id='-10' lon='20'  lat='10' changeset='#{changeset_id}'/>
 -  <node id='-2' lon='-40' lat='-20' changeset='#{changeset_id}'/>
 -  <node id='-11' lon='40'  lat='20' changeset='#{changeset_id}'/>
 -  <node id='-3' lon='-60' lat='-30' changeset='#{changeset_id}'/>
 -  <node id='-12' lon='60'  lat='30' changeset='#{changeset_id}'/>
 -  <node id='-4' lon='-80' lat='-40' changeset='#{changeset_id}'/>
 -  <node id='-13' lon='80'  lat='40' changeset='#{changeset_id}'/>
 -  <node id='-5' lon='-100' lat='-50' changeset='#{changeset_id}'/>
 -  <node id='-14' lon='100'  lat='50' changeset='#{changeset_id}'/>
 -  <node id='-6' lon='-120' lat='-60' changeset='#{changeset_id}'/>
 -  <node id='-15' lon='120'  lat='60' changeset='#{changeset_id}'/>
 -  <node id='-7' lon='-140' lat='-70' changeset='#{changeset_id}'/>
 -  <node id='-16' lon='140'  lat='70' changeset='#{changeset_id}'/>
 -  <node id='-8' lon='-160' lat='-80' changeset='#{changeset_id}'/>
 -  <node id='-17' lon='160'  lat='80' changeset='#{changeset_id}'/>
 -  <node id='-9' lon='-179.9' lat='-89.9' changeset='#{changeset_id}'/>
 -  <node id='-18' lon='179.9'  lat='89.9' changeset='#{changeset_id}'/>
 - </create>
 -</osmChange>
 -EOF
 -
 -    # upload it, which used to cause an error like "PGError: ERROR: 
 -    # integer out of range" (bug #2152). but shouldn't any more.
 -    content diff
 -    post :upload, :id => changeset_id
 -    assert_response :success, 
 -      "can't upload a spatially-large diff to changeset: #{@response.body}"
 -
 -    # check that the changeset bbox is within bounds
 -    cs = Changeset.find(changeset_id)
 -    assert cs.min_lon >= -180 * SCALE, "Minimum longitude (#{cs.min_lon / SCALE}) should be >= -180 to be valid."
 -    assert cs.max_lon <=  180 * SCALE, "Maximum longitude (#{cs.max_lon / SCALE}) should be <= 180 to be valid."
 -    assert cs.min_lat >=  -90 * SCALE, "Minimum latitude (#{cs.min_lat / SCALE}) should be >= -90 to be valid."
 -    assert cs.max_lat >=   90 * SCALE, "Maximum latitude (#{cs.max_lat / SCALE}) should be <= 90 to be valid."
 -  end
 -
    ##
    # test that deleting stuff in a transaction doesn't bypass the checks
    # to ensure that used elements are not deleted.
      assert_response :success, "can't get changesets in bbox"
      assert_changesets [1]
  
+     # not found when looking for changesets of non-existing users
+     get :query, :user => User.maximum(:id) + 1
+     assert_response :not_found
+     get :query, :display_name => " "
+     assert_response :not_found
      # can't get changesets of user 1 without authenticating
      get :query, :user => users(:normal_user).id
-     assert_response :not_found, "shouldn't be able to get changesets by non-public user"
+     assert_response :not_found, "shouldn't be able to get changesets by non-public user (ID)"
+     get :query, :display_name => users(:normal_user).display_name
+     assert_response :not_found, "shouldn't be able to get changesets by non-public user (name)"
  
      # but this should work
      basic_authorization "test@openstreetmap.org", "test"
      get :query, :user => users(:normal_user).id
-     assert_response :success, "can't get changesets by user"
+     assert_response :success, "can't get changesets by user ID"
+     assert_changesets [1,3,6]
+     get :query, :display_name => users(:normal_user).display_name
+     assert_response :success, "can't get changesets by user name"
      assert_changesets [1,3,6]
  
+     # check that the correct error is given when we provide both UID and name
+     get :query, :user => users(:normal_user).id, :display_name => users(:normal_user).display_name
+     assert_response :bad_request, "should be a bad request to have both ID and name specified"
      get :query, :user => users(:normal_user).id, :open => true
      assert_response :success, "can't get changesets by user and open"
      assert_changesets [1]