on DAV throw Bad Request if provided message is too long

5 files changed, 144 insertions, 2 deletions

diff --git a/apps/dav/lib/comments/commentnode.php b/apps/dav/lib/comments/commentnode.php
index d3cd53bceb1..339abc6382d 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 56d94cc33e9..7abf6e71ee5 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 8d1bf06ab60..8ebc5c2ff2c 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 {
 
@@ -199,6 +201,43 @@ class CommentsNode extends \Test\TestCase {
 	}
 
 	/**
+	 * @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
 	 */
 	public function testUpdateForbiddenByUser() {
diff --git a/apps/dav/tests/unit/comments/commentsplugin.php b/apps/dav/tests/unit/comments/commentsplugin.php
index 9822137bbea..d6f489f5e80 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 {
@@ -506,6 +507,98 @@ class CommentsPlugin extends \Test\TestCase {
 	}
 
 	/**
+	 * @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
 	 */
 	public function testOnReportInvalidNode() {
diff --git a/lib/public/comments/messagetoolongexception.php b/lib/public/comments/messagetoolongexception.php
index 5b2809ae9ce..054cecf320f 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 {}