Merge pull request #22228 from owncloud/comments-limit-message

Limit comment message to 1k chars

apps/dav/lib/comments/commentnode.php

@@ -24,9 +24,11 @@ namespace OCA\DAV\Comments;
 
 use OCP\Comments\IComment;
 use OCP\Comments\ICommentsManager;
+use OCP\Comments\MessageTooLongException;
 use OCP\ILogger;
 use OCP\IUserManager;
 use OCP\IUserSession;
+use Sabre\DAV\Exception\BadRequest;
 use Sabre\DAV\Exception\Forbidden;
 use Sabre\DAV\Exception\MethodNotAllowed;
 use Sabre\DAV\PropPatch;
@@ -168,6 +170,7 @@ class CommentNode implements \Sabre\DAV\INode, \Sabre\DAV\IProperties {
 	 *
 	 * @param $propertyValue
 	 * @return bool
+	 * @throws BadRequest
 	 * @throws Forbidden
 	 */
 	public function updateComment($propertyValue) {
@@ -178,6 +181,10 @@ class CommentNode implements \Sabre\DAV\INode, \Sabre\DAV\IProperties {
 			return true;
 		} catch (\Exception $e) {
 			$this->logger->logException($e, ['app' => 'dav/comments']);
+			if($e instanceof MessageTooLongException) {
+				$msg = 'Message exceeds allowed character limit of ';
+				throw new BadRequest($msg . IComment::MAX_MESSAGE_LENGTH, 0, $e);
+			}
 			return false;
 		}
 	}

apps/dav/lib/comments/commentsplugin.php

@@ -242,6 +242,9 @@ class CommentsPlugin extends ServerPlugin {
 			return $comment;
 		} catch (\InvalidArgumentException $e) {
 			throw new BadRequest('Invalid input values', 0, $e);
+		} catch (\OCP\Comments\MessageTooLongException $e) {
+			$msg = 'Message exceeds allowed character limit of ';
+			throw new BadRequest($msg . \OCP\Comments\IComment::MAX_MESSAGE_LENGTH, 0,	$e);
 		}
 	}
 

apps/dav/tests/unit/comments/commentnode.php

@@ -22,6 +22,8 @@
 namespace OCA\DAV\Tests\Unit\Comments;
 
 use OCA\DAV\Comments\CommentNode;
+use OCP\Comments\IComment;
+use OCP\Comments\MessageTooLongException;
 
 class CommentsNode extends \Test\TestCase {
 
@@ -198,6 +200,43 @@ class CommentsNode extends \Test\TestCase {
 		$this->assertFalse($this->node->updateComment($msg));
 	}
 
+	/**
+	 * @expectedException \Sabre\DAV\Exception\BadRequest
+	 * @expectedExceptionMessage Message exceeds allowed character limit of
+	 */
+	public function testUpdateCommentMessageTooLongException() {
+		$user = $this->getMock('\OCP\IUser');
+
+		$user->expects($this->once())
+			->method('getUID')
+			->will($this->returnValue('alice'));
+
+		$this->userSession->expects($this->once())
+			->method('getUser')
+			->will($this->returnValue($user));
+
+		$this->comment->expects($this->once())
+			->method('setMessage')
+			->will($this->throwException(new MessageTooLongException()));
+
+		$this->comment->expects($this->any())
+			->method('getActorType')
+			->will($this->returnValue('users'));
+
+		$this->comment->expects($this->any())
+			->method('getActorId')
+			->will($this->returnValue('alice'));
+
+		$this->commentsManager->expects($this->never())
+			->method('save');
+
+		$this->logger->expects($this->once())
+			->method('logException');
+
+		// imagine 'foo' has >1k characters. comment is mocked anyway.
+		$this->node->updateComment('foo');
+	}
+
 	/**
 	 * @expectedException \Sabre\DAV\Exception\Forbidden
 	 */

apps/dav/tests/unit/comments/commentsplugin.php

@@ -23,6 +23,7 @@ namespace OCA\DAV\Tests\Unit\Comments;
 
 use OC\Comments\Comment;
 use OCA\DAV\Comments\CommentsPlugin as CommentsPluginImplementation;
+use OCP\Comments\IComment;
 use Sabre\DAV\Exception\NotFound;
 
 class CommentsPlugin extends \Test\TestCase {
@@ -505,6 +506,98 @@ class CommentsPlugin extends \Test\TestCase {
 		$this->plugin->httpPost($request, $response);
 	}
 
+	/**
+	 * @expectedException \Sabre\DAV\Exception\BadRequest
+	 * @expectedExceptionMessage Message exceeds allowed character limit of
+	 */
+	public function testCreateCommentMessageTooLong() {
+		$commentData = [
+			'actorType' => 'users',
+			'verb' => 'comment',
+			'message' => str_pad('', IComment::MAX_MESSAGE_LENGTH + 1, 'x'),
+		];
+
+		$comment = new Comment([
+				'objectType' => 'files',
+				'objectId' => '42',
+				'actorType' => 'users',
+				'actorId' => 'alice',
+				'verb' => 'comment',
+			]);
+		$comment->setId('23');
+
+		$path = 'comments/files/42';
+
+		$requestData = json_encode($commentData);
+
+		$user = $this->getMock('OCP\IUser');
+		$user->expects($this->once())
+			->method('getUID')
+			->will($this->returnValue('alice'));
+
+		$node = $this->getMockBuilder('\OCA\DAV\Comments\EntityCollection')
+			->disableOriginalConstructor()
+			->getMock();
+		$node->expects($this->once())
+			->method('getName')
+			->will($this->returnValue('files'));
+		$node->expects($this->once())
+			->method('getId')
+			->will($this->returnValue('42'));
+
+		$node->expects($this->never())
+			->method('setReadMarker');
+
+		$this->commentsManager->expects($this->once())
+			->method('create')
+			->with('users', 'alice', 'files', '42')
+			->will($this->returnValue($comment));
+
+		$this->userSession->expects($this->once())
+			->method('getUser')
+			->will($this->returnValue($user));
+
+		// technically, this is a shortcut. Inbetween EntityTypeCollection would
+		// be returned, but doing it exactly right would not be really
+		// unit-testing like, as it would require to haul in a lot of other
+		// things.
+		$this->tree->expects($this->any())
+			->method('getNodeForPath')
+			->with('/' . $path)
+			->will($this->returnValue($node));
+
+		$request = $this->getMockBuilder('Sabre\HTTP\RequestInterface')
+			->disableOriginalConstructor()
+			->getMock();
+
+		$response = $this->getMockBuilder('Sabre\HTTP\ResponseInterface')
+			->disableOriginalConstructor()
+			->getMock();
+
+		$request->expects($this->once())
+			->method('getPath')
+			->will($this->returnValue('/' . $path));
+
+		$request->expects($this->once())
+			->method('getBodyAsString')
+			->will($this->returnValue($requestData));
+
+		$request->expects($this->once())
+			->method('getHeader')
+			->with('Content-Type')
+			->will($this->returnValue('application/json'));
+
+		$response->expects($this->never())
+			->method('setHeader');
+
+		$this->server->expects($this->any())
+			->method('getRequestUri')
+			->will($this->returnValue($path));
+		$this->plugin->initialize($this->server);
+
+		$this->plugin->httpPost($request, $response);
+	}
+
 	/**
 	 * @expectedException \Sabre\DAV\Exception\ReportNotSupported
 	 */

lib/private/comments/comment.php

@@ -22,6 +22,7 @@ namespace OC\Comments;
 
 use OCP\Comments\IComment;
 use OCP\Comments\IllegalIDChangeException;
+use OCP\Comments\MessageTooLongException;
 
 class Comment implements IComment {
 
@@ -184,13 +185,18 @@ class Comment implements IComment {
 	 *
 	 * @param string $message
 	 * @return IComment
+	 * @throws MessageTooLongException
 	 * @since 9.0.0
 	 */
 	public function setMessage($message) {
 		if(!is_string($message)) {
 			throw new \InvalidArgumentException('String expected.');
 		}
-		$this->data['message'] = trim($message);
+		$message = trim($message);
+		if(mb_strlen($message, 'UTF-8') > IComment::MAX_MESSAGE_LENGTH) {
+			throw new MessageTooLongException('Comment message must not exceed ' . IComment::MAX_MESSAGE_LENGTH . ' characters');
+		}
+		$this->data['message'] = $message;
 		return $this;
 	}
 

lib/public/comments/icomment.php

@@ -29,6 +29,7 @@ namespace OCP\Comments;
  * @since 9.0.0
  */
 interface IComment {
+	const MAX_MESSAGE_LENGTH = 1000;
 
 	/**
 	 * returns the ID of the comment
@@ -119,8 +120,12 @@ interface IComment {
 	/**
 	 * sets the message of the comment and returns itself
 	 *
+	 * When the given message length exceeds MAX_MESSAGE_LENGTH an
+	 * MessageTooLongException shall be thrown.
+	 *
 	 * @param string $message
 	 * @return IComment
+	 * @throws MessageTooLongException
 	 * @since 9.0.0
 	 */
 	public function setMessage($message);

lib/public/comments/messagetoolongexception.php

@@ -0,0 +1,27 @@
+<?php
+/**
+ * @author Arthur Schiwon <blizzz@owncloud.com>
+ *
+ * @copyright Copyright (c) 2016, ownCloud, Inc.
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+namespace OCP\Comments;
+
+/**
+ * Exception thrown when a comment message exceeds the allowed character limit
+ * @since 9.0.0
+ */
+class MessageTooLongException extends \OverflowException {}

tests/lib/comments/comment.php

@@ -2,6 +2,7 @@
 
 namespace Test\Comments;
 
+use OCP\Comments\IComment;
 use Test\TestCase;
 
 class Test_Comments_Comment extends TestCase
@@ -107,6 +108,15 @@ class Test_Comments_Comment extends TestCase
 		$comment->$setter($type, $id);
 	}
 
+	/**
+	 * @expectedException \OCP\Comments\MessageTooLongException
+	 */
+	public function testSetUberlongMessage() {
+		$comment = new \OC\Comments\Comment();
+		$msg = str_pad('', IComment::MAX_MESSAGE_LENGTH + 1, 'x');
+		$comment->setMessage($msg);
+	}
+
 
 
 }