Make the gpx/id/data API call work, and make gpx/create a POST method

so arguments can be handled more robustly.

diff --git a/app/controllers/trace_controller.rb b/app/controllers/trace_controller.rb
index 66583459be5abad19e31eec2f7ee1ff11fde6a63..2001fdb2f619a6ac38c45ab75a78e20915ef78f9 100644 (file)
--- a/app/controllers/trace_controller.rb
+++ b/app/controllers/trace_controller.rb
@@ -151,66 +151,70 @@ class TraceController < ApplicationController
   end
 
   def picture
-    begin
-      trace = Trace.find(params[:id])
+    trace = Trace.find(params[:id])
 
-      if trace.public? or (@user and @user == trace.user)
-        send_file(trace.large_picture_name, :filename => "#{trace.id}.gif", :type => 'image/gif', :disposition => 'inline')
-      else
-        render :nothing, :status => :forbidden
-      end
-    rescue ActiveRecord::RecordNotFound
-      render :nothing => true, :status => :not_found
-    rescue
-      render :nothing => true, :status => :internal_server_error
+    if trace.public? or (@user and @user == trace.user)
+      send_file(trace.large_picture_name, :filename => "#{trace.id}.gif", :type => 'image/gif', :disposition => 'inline')
+    else
+      render :nothing, :status => :forbidden
     end
+  rescue ActiveRecord::RecordNotFound
+    render :nothing => true, :status => :not_found
   end
 
   def icon
-    begin
-      trace = Trace.find(params[:id])
+    trace = Trace.find(params[:id])
 
-      if trace.public? or (@user and @user == trace.user)
-        send_file(trace.icon_picture_name, :filename => "#{trace.id}_icon.gif", :type => 'image/gif', :disposition => 'inline')
-      else
-        render :nothing, :status => :forbidden
-      end
-    rescue ActiveRecord::RecordNotFound
-      render :nothing => true, :status => :not_found
-    rescue
-      render :nothing => true, :status => :internal_server_error
+    if trace.public? or (@user and @user == trace.user)
+      send_file(trace.icon_picture_name, :filename => "#{trace.id}_icon.gif", :type => 'image/gif', :disposition => 'inline')
+    else
+      render :nothing, :status => :forbidden
     end
+  rescue ActiveRecord::RecordNotFound
+    render :nothing => true, :status => :not_found
   end
 
   def api_details
-    begin
-      trace = Trace.find(params[:id])
+    trace = Trace.find(params[:id])
 
-      if trace.public? or trace.user == @user
-        render :text => trace.to_xml.to_s, :content_type => "text/xml"
-      else
-        render :nothing => true, :status => :forbidden
-      end
-    rescue ActiveRecord::RecordNotFound
-      render :nothing => true, :status => :not_found
-    rescue
-      render :nothing => true, :status => :internal_server_error
+    if trace.public? or trace.user == @user
+      render :text => trace.to_xml.to_s, :content_type => "text/xml"
+    else
+      render :nothing => true, :status => :forbidden
     end
+  rescue ActiveRecord::RecordNotFound
+    render :nothing => true, :status => :not_found
   end
 
   def api_data
-    render :action => 'data'
+    trace = Trace.find(params[:id])
+
+    if trace.public? or trace.user == @user
+      send_file(trace.trace_name, :filename => "#{trace.id}#{trace.extension_name}", :type => trace.mime_type, :disposition => 'attachment')
+    else
+      render :nothing => true, :status => :forbidden
+    end
+  rescue ActiveRecord::RecordNotFound
+    render :nothing => true, :status => :not_found
   end
 
   def api_create
-    do_create(params[:filename], params[:tags], params[:description], true) do |f|
-      f.write(request.raw_post)
-    end
+    if request.post?
+      name = params[:file].original_filename.gsub(/[^a-zA-Z0-9.]/, '_') # This makes sure filenames are sane
 
-    if @trace.id
-      render :nothing => true
+      do_create(name, params[:tags], params[:description], params[:public]) do |f|
+        f.write(request[:file].read)
+      end
+
+      if @trace.id
+        render :text => @trace.id.to_s, :content_type => "text/plain"
+      elsif @trace.valid?
+        render :nothing => true, :status => :internal_server_error
+      else
+        render :nothing => true, :status => :bad_request
+      end
     else
-      render :nothing => true, :status => :internal_server_error
+      render :nothing => true, :status => :method_not_allowed
     end
   end
 

diff --git a/app/models/trace.rb b/app/models/trace.rb
index 155e495a34b485b9123323088e45f58c2bfb2094..3eefc185d400c265f1cce211cb4e9b593555d07a 100644 (file)
--- a/app/models/trace.rb
+++ b/app/models/trace.rb
@@ -3,6 +3,7 @@ class Trace < ActiveRecord::Base
 
   validates_presence_of :user_id, :name, :timestamp
   validates_presence_of :description, :on => :create
+  validates_format_of :tagstring, :with => /^[^\/;.,?]*$/
 #  validates_numericality_of :latitude, :longitude
   validates_inclusion_of :public, :inserted, :in => [ true, false]
   

diff --git a/config/routes.rb b/config/routes.rb
index ae7acc1998643d3b6b998dfcb617031742e47c3f..482c5c6cb0c54ab663ee2b2a8d41ff070a8c1b5e 100644 (file)
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -37,7 +37,7 @@ ActionController::Routing::Routes.draw do |map|
   map.connect "api/#{API_VERSION}/user/details", :controller => 'user', :action => 'api_details'
   map.connect "api/#{API_VERSION}/user/gpx_files", :controller => 'user', :action => 'api_gpx_files'
  
-  map.connect "api/#{API_VERSION}/gpx/create/:filename/:description/:tags", :controller => 'trace', :action => 'api_create'
+  map.connect "api/#{API_VERSION}/gpx/create", :controller => 'trace', :action => 'api_create'
   map.connect "api/#{API_VERSION}/gpx/:id/details", :controller => 'trace', :action => 'api_details'
   map.connect "api/#{API_VERSION}/gpx/:id/data", :controller => 'trace', :action => 'api_data'