From 16a3cdd8c94c38509128c40e4898a7f08d7d9e40 Mon Sep 17 00:00:00 2001
From: Ryan Barrett <git@ryanb.org>
Date: Tue, 2 Feb 2016 18:12:22 -0800
Subject: [PATCH] fix callback param URL validation

---
 controllers/API.php | 18 +++++-------------
 tests/APITest.php   | 24 ++++++++++--------------
 2 files changed, 15 insertions(+), 27 deletions(-)

diff --git a/controllers/API.php b/controllers/API.php
index 633c860..e116abc 100644
--- a/controllers/API.php
+++ b/controllers/API.php
@@ -54,24 +54,16 @@ class API {
 
     $urlregex = '/^https?:\/\/[^ ]+\.[^ ]+$/';
 
-    # Verify source and target are URLs
-    if(!preg_match($urlregex, $source) || !preg_match($urlregex, $target)) {
+    # Verify source, target, and callback are URLs
+    $callback = $request->get('callback');
+    if(!preg_match($urlregex, $source) || !preg_match($urlregex, $target) ||
+       ($callback && !preg_match($urlregex, $callback))) {
       return $this->respond($response, 400, [
         'error' => 'invalid_parameter',
-        'error_description' => 'The source or target parameters were invalid'
+        'error_description' => 'The source, target, or callback parameters were invalid'
       ]);
     }
 
-    # If a callback was provided, verify it is a URL
-    if($callback=$request->get('callback')) {
-      if(!preg_match($urlregex, $source) || !preg_match($urlregex, $target)) {
-        return $this->respond($response, 400, [
-          'error' => 'invalid_parameter',
-          'error_description' => 'The callback parameter was invalid'
-        ]);
-      }
-    }
-
     # Verify the token is valid
     $role = ORM::for_table('roles')->where('token', $token)->find_one();
 
diff --git a/tests/APITest.php b/tests/APITest.php
index cf53e73..9012386 100644
--- a/tests/APITest.php
+++ b/tests/APITest.php
@@ -87,20 +87,16 @@ class APITest extends PHPUnit_Framework_TestCase {
   public function testInvalidURLs() {
     $this->_createExampleAccount();
 
-    $response = $this->webmention(['token'=>'a','source'=>'notaurl','target'=>'alsonotaurl']);
-    $this->assertEquals(400, $response->getStatusCode());
-    $data = json_decode($response->getContent());
-    $this->assertEquals('invalid_parameter', $data->error);
-
-    $response = $this->webmention(['token'=>'a','source'=>'http://source.example','target'=>'alsonotaurl']);
-    $this->assertEquals(400, $response->getStatusCode());
-    $data = json_decode($response->getContent());
-    $this->assertEquals('invalid_parameter', $data->error);
-
-    $response = $this->webmention(['token'=>'a','source'=>'notaurl','target'=>'http://target.example']);
-    $this->assertEquals(400, $response->getStatusCode());
-    $data = json_decode($response->getContent());
-    $this->assertEquals('invalid_parameter', $data->error);
+    foreach ([['token'=>'a','source'=>'notaurl','target'=>'alsonotaurl'],
+              ['token'=>'a','source'=>'http://source.example','target'=>'alsonotaurl'],
+              ['token'=>'a','source'=>'notaurl','target'=>'http://target.example'],
+              ['token'=>'a','source'=>'http://source.example','target'=>'http://target.example','callback'=>'notaurl']
+             ] as $params) {
+      $response = $this->webmention($params);
+      $this->assertEquals(400, $response->getStatusCode());
+      $data = json_decode($response->getContent());
+      $this->assertEquals('invalid_parameter', $data->error);
+    }
   }
 
   public function testNoLinkToSource() {