From 347ad3e223e2582124d56b0d7174886bde194c16 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 9 Feb 2016 03:14:30 +0100 Subject: [PATCH 1/2] Limit comment message to 1k chars --- lib/private/comments/comment.php | 8 +++++- lib/public/comments/icomment.php | 5 ++++ .../comments/messagetoolongexception.php | 27 +++++++++++++++++++ tests/lib/comments/comment.php | 10 +++++++ 4 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 lib/public/comments/messagetoolongexception.php diff --git a/lib/private/comments/comment.php b/lib/private/comments/comment.php index 36189d9523..27e63c9855 100644 --- a/lib/private/comments/comment.php +++ b/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; } diff --git a/lib/public/comments/icomment.php b/lib/public/comments/icomment.php index e695b5193f..a7f4b4c617 100644 --- a/lib/public/comments/icomment.php +++ b/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); diff --git a/lib/public/comments/messagetoolongexception.php b/lib/public/comments/messagetoolongexception.php new file mode 100644 index 0000000000..5b2809ae9c --- /dev/null +++ b/lib/public/comments/messagetoolongexception.php @@ -0,0 +1,27 @@ + + * + * @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 + * + */ +namespace OCP\Comments; + +/** + * Exception for not found entity + * @since 9.0.0 + */ +class MessageTooLongException extends \Exception {} diff --git a/tests/lib/comments/comment.php b/tests/lib/comments/comment.php index e6f9c941c9..9b3f2ab166 100644 --- a/tests/lib/comments/comment.php +++ b/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); + } + } From bbc86e0756429b4c51e245d6dcf3ad5a5a1785eb Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 9 Feb 2016 13:59:13 +0100 Subject: [PATCH 2/2] on DAV throw Bad Request if provided message is too long --- apps/dav/lib/comments/commentnode.php | 7 ++ apps/dav/lib/comments/commentsplugin.php | 3 + apps/dav/tests/unit/comments/commentnode.php | 39 ++++++++ .../tests/unit/comments/commentsplugin.php | 93 +++++++++++++++++++ .../comments/messagetoolongexception.php | 4 +- 5 files changed, 144 insertions(+), 2 deletions(-) diff --git a/apps/dav/lib/comments/commentnode.php b/apps/dav/lib/comments/commentnode.php index d3cd53bceb..339abc6382 100644 --- a/apps/dav/lib/comments/commentnode.php +++ b/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; } } diff --git a/apps/dav/lib/comments/commentsplugin.php b/apps/dav/lib/comments/commentsplugin.php index 56d94cc33e..7abf6e71ee 100644 --- a/apps/dav/lib/comments/commentsplugin.php +++ b/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); } } diff --git a/apps/dav/tests/unit/comments/commentnode.php b/apps/dav/tests/unit/comments/commentnode.php index 8d1bf06ab6..8ebc5c2ff2 100644 --- a/apps/dav/tests/unit/comments/commentnode.php +++ b/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 */ diff --git a/apps/dav/tests/unit/comments/commentsplugin.php b/apps/dav/tests/unit/comments/commentsplugin.php index 9822137bbe..d6f489f5e8 100644 --- a/apps/dav/tests/unit/comments/commentsplugin.php +++ b/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 */ diff --git a/lib/public/comments/messagetoolongexception.php b/lib/public/comments/messagetoolongexception.php index 5b2809ae9c..054cecf320 100644 --- a/lib/public/comments/messagetoolongexception.php +++ b/lib/public/comments/messagetoolongexception.php @@ -21,7 +21,7 @@ namespace OCP\Comments; /** - * Exception for not found entity + * Exception thrown when a comment message exceeds the allowed character limit * @since 9.0.0 */ -class MessageTooLongException extends \Exception {} +class MessageTooLongException extends \OverflowException {}