From: Tom Hughes Date: Tue, 13 Oct 2009 20:06:24 +0000 (+0000) Subject: Merged 17256:18123 from trunk. X-Git-Tag: live~6555^2~7 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/ef40b61ff40881966ae4280cfdcb9f92965e9d10?hp=-c Merged 17256:18123 from trunk. --- ef40b61ff40881966ae4280cfdcb9f92965e9d10 diff --combined app/controllers/application_controller.rb index 6dbe9165c,e36c9842b..bcaed4565 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@@ -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) @@@ -78,6 -91,12 +91,12 @@@ @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") @@@ -174,6 -193,8 +193,6 @@@ 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 diff --combined app/controllers/trace_controller.rb index f06a162fb,84026e8cd..c90558269 --- a/app/controllers/trace_controller.rb +++ b/app/controllers/trace_controller.rb @@@ -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") @@@ -45,15 -46,15 +46,15 @@@ # 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 @@@ -128,8 -129,11 +129,11 @@@ 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}") @@@ -207,7 -211,7 +211,7 @@@ 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 = ?" @@@ -293,12 -297,16 +297,16 @@@ 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 diff --combined app/views/layouts/site.html.erb index f072544aa,1e4e56e6a..d1983a9f0 --- a/app/views/layouts/site.html.erb +++ b/app/views/layouts/site.html.erb @@@ -1,14 -1,11 +1,15 @@@ + + <%= javascript_strings %> <%= javascript_include_tag 'prototype' %> <%= javascript_include_tag 'site' %> - <%= stylesheet_link_tag 'site' %> + <%= stylesheet_link_tag 'common' %> + + <%= 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 @@@ <% 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')) %> | + <%= 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')) %> + <%= link_to t('layouts.welcome_user_link_tooltip'), {:controller => 'user', :action => 'view', :display_name => @user.display_name} %> | <%= yield :greeting %> <% inbox_attributes = {} diff --combined app/views/site/edit.html.erb index 142d7a3f3,b29c0f200..51389afc1 --- a/app/views/site/edit.html.erb +++ b/app/views/site/edit.html.erb @@@ -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')) %>'; } } @@@ -78,9 -78,10 +78,10 @@@ 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"); } diff --combined test/functional/changeset_controller_test.rb index 46f8a1a6a,76ec0866a..a840dd233 --- a/test/functional/changeset_controller_test.rb +++ b/test/functional/changeset_controller_test.rb @@@ -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 "" - 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 - - # 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. @@@ -1286,16 -1337,32 +1286,32 @@@ EO 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]