From 4b588ccefb0803e84d0e6f3d09063c1bd57ffbe4 Mon Sep 17 00:00:00 2001
From: Joseph Method <tristil@gmail.com>
Date: Tue, 19 Oct 2010 23:03:07 -0400
Subject: [PATCH] Addresses [#380] and [#239] by handling the errors from bad
 identities

---
 app/controllers/requests_controller.rb       | 11 +++++--
 app/models/person.rb                         |  6 +++-
 config/locales/diaspora/en.yml               |  2 ++
 spec/controllers/requests_controller_spec.rb | 30 ++++++++++++++++++++
 spec/models/person_spec.rb                   | 20 +++++++++++++
 5 files changed, 66 insertions(+), 3 deletions(-)

diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb
index 347d8bfce1..4a10821aad 100644
--- a/app/controllers/requests_controller.rb
+++ b/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"
-      flash[:error] = I18n.t 'requests.create.error'
+      if e.message.include? "not found"
+        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
diff --git a/app/models/person.rb b/app/models/person.rb
index 8a9011fa4d..136af95b96 100644
--- a/app/models/person.rb
+++ b/app/models/person.rb
@@ -85,7 +85,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)
 
diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml
index 45d80437a7..5f09b603ce 100644
--- a/config/locales/diaspora/en.yml
+++ b/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}."
diff --git a/spec/controllers/requests_controller_spec.rb b/spec/controllers/requests_controller_spec.rb
index eb3720cd26..0beb8d3736 100644
--- a/spec/controllers/requests_controller_spec.rb
+++ b/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
diff --git a/spec/models/person_spec.rb b/spec/models/person_spec.rb
index fd5ea0162e..7506d04336 100644
--- a/spec/models/person_spec.rb
+++ b/spec/models/person_spec.rb
@@ -197,6 +197,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')
-- 
GitLab