Merge remote branch 'tristil/239-380-identity-error-handling'

app/controllers/requests_controller.rb

@@ -35,8 +35,15 @@ class RequestsController < ApplicationController
     begin
       rel_hash = relationship_flow(params[:request][:destination_url].strip)
     rescue Exception => e
-      raise e unless e.message.include? "not found"
+      if e.message.include? "not found"
-      flash[:error] = I18n.t 'requests.create.error'
+        flash[:error] = I18n.t 'requests.create.error'
+      elsif e.message.include?  "Connection timed out"
+        flash[:error] = I18n.t 'requests.create.error_server'
+      elsif e.message == "Identifier is invalid"
+        flash[:error] = I18n.t 'requests.create.invalid_identity'
+      else
+        raise e
+      end
       respond_with :location => aspect
       return
     end

app/models/person.rb

@@ -92,7 +92,11 @@ class Person
   end
 
   def self.by_webfinger(identifier, opts = {})
-    #need to check if this is a valid email structure, maybe should do in JS
+    # Raise an error if identifier has a port number 
+    raise "Identifier is invalid" if(identifier.strip.match(/\:\d+$/))
+    # Raise an error if identifier is not a valid email (generous regexp)
+    raise "Identifier is invalid" if !(identifier =~ /\A.*\@.*\..*\Z/)
+
     query = /#{Regexp.escape(identifier.gsub('acct:', '').to_s)}/i
     local_person = Person.first(:diaspora_handle => query)
 

config/locales/diaspora/en.yml

@@ -229,6 +229,8 @@ en:
           ignore: "Ignored friend request."
       create:
           error: "No diaspora seed found with this email!"
+          invalid_identity: "This identity is not properly formatted"
+          error_server: "Problem with other server. Maybe it doesn't exist?"
           yourself: "You cannot befriend yourself!"
           already_friends: "You are already friends with %{destination_url}!"
           success: "A friend request was sent to %{destination_url}."

spec/controllers/requests_controller_spec.rb

@@ -22,4 +22,34 @@ describe RequestsController do
     response.should redirect_to aspect_path(@user.aspects[0].id.to_s)
   end
 
+  it "should not error out when requesting an invalid identity" do
+    put("create", "request" => {
+      "destination_url" => "not_a_@valid_email",
+      "aspect_id" => @user.aspects[0].id 
+      } 
+    )
+    response.should redirect_to aspect_path(@user.aspects[0].id.to_s)
+  end
+
+  it "should not error out when requesting an invalid identity with a port number" do
+    put("create", "request" => {
+      "destination_url" => "johndoe@email.com:3000",
+      "aspect_id" => @user.aspects[0].id 
+      } 
+    )
+    response.should redirect_to aspect_path(@user.aspects[0].id.to_s)
+  end
+
+  it "should not error out when requesting an identity from an invalid server" do
+    stub_request(:get, /notadiasporaserver\.com/).to_raise(Errno::ETIMEDOUT)
+    put("create", "request" => {
+      "destination_url" => "johndoe@notadiasporaserver.com",
+      "aspect_id" => @user.aspects[0].id 
+      } 
+    )
+    response.should redirect_to aspect_path(@user.aspects[0].id.to_s)
+  end
+
+
+
 end

spec/models/person_spec.rb

@@ -187,6 +187,26 @@ describe Person do
       end
     end
 
+    it 'identifier should be a valid email' do
+      stub_success("joe.valid+email@my-address.com")
+      Proc.new { 
+        Person.by_webfinger("joe.valid+email@my-address.com")
+      }.should_not raise_error(RuntimeError, "Identifier is invalid")
+
+      stub_success("not_a_@valid_email")
+      Proc.new { 
+        Person.by_webfinger("not_a_@valid_email")
+      }.should raise_error(RuntimeError, "Identifier is invalid")
+
+    end
+
+    it 'should not accept a port number' do
+      stub_success("eviljoe@diaspora.local:3000")
+      Proc.new { 
+        Person.by_webfinger('eviljoe@diaspora.local:3000')
+      }.should raise_error(RuntimeError, "Identifier is invalid")
+    end
+
     it 'creates a stub for a remote user' do
       stub_success("tom@tom.joindiaspora.com")
       tom = Person.by_webfinger('tom@tom.joindiaspora.com')