Allow other fields to be send using API

Entry API can now have these new fields:
- content
- language
- preview_picture
- published_at

Re-use the ContentProxy to be able to do the same using the web UI (in the future).
htmLawed is used to clean stuff from content, I hope it’ll be enough to avoid security breach.

Lower content validation when we want to update an entry with content already defined. Before, language & content_type were required. If there weren’t provided, we re-fetched the content using graby. I think these fields aren’t required for an entry to be created. So I removed them.
Which means some import from the v1 export won’t be re-fetched since they provide content, url & title.

Also, remove liberation link from Readability import to avoid overlaping import (from wallabag v1, which had the same link)

diff --git a/src/Wallabag/ApiBundle/Controller/EntryRestController.php b/src/Wallabag/ApiBundle/Controller/EntryRestController.php
index 31bb67fd7d834ed391f08b2c942ca78d6c1056f9..dfd04fb4d6c9c7c66f9dcbe4ef0a0baf3104f15c 100644 (file)
--- a/src/Wallabag/ApiBundle/Controller/EntryRestController.php
+++ b/src/Wallabag/ApiBundle/Controller/EntryRestController.php
@@ -280,6 +280,10 @@ class EntryRestController extends WallabagRestController
      *          {"name"="tags", "dataType"="string", "required"=false, "format"="tag1,tag2,tag3", "description"="a comma-separated list of tags."},
      *          {"name"="starred", "dataType"="integer", "required"=false, "format"="1 or 0", "description"="entry already starred"},
      *          {"name"="archive", "dataType"="integer", "required"=false, "format"="1 or 0", "description"="entry already archived"},
+     *          {"name"="content", "dataType"="string", "required"=false, "description"="Content of the entry"},
+     *          {"name"="language", "dataType"="string", "required"=false, "description"="Language of the entry"},
+     *          {"name"="preview_picture", "dataType"="string", "required"=false, "description"="Preview picture of the entry"},
+     *          {"name"="published_at", "dataType"="datetime", "format"="YYYY-MM-DDTHH:II:SS+TZ", "required"=false, "description"="Published date of the entry"},
      *       }
      * )
      *
@@ -293,30 +297,42 @@ class EntryRestController extends WallabagRestController
         $title = $request->request->get('title');
         $isArchived = $request->request->get('archive');
         $isStarred = $request->request->get('starred');
+        $content = $request->request->get('content');
+        $language = $request->request->get('language');
+        $picture = $request->request->get('preview_picture');
+        $publishedAt = $request->request->get('published_at');
 
         $entry = $this->get('wallabag_core.entry_repository')->findByUrlAndUserId($url, $this->getUser()->getId());
 
         if (false === $entry) {
             $entry = new Entry($this->getUser());
-            try {
-                $entry = $this->get('wallabag_core.content_proxy')->updateEntry(
-                    $entry,
-                    $url
-                );
-            } catch (\Exception $e) {
-                $this->get('logger')->error('Error while saving an entry', [
-                    'exception' => $e,
-                    'entry' => $entry,
-                ]);
-                $entry->setUrl($url);
-            }
         }
 
-        if (!is_null($title)) {
-            $entry->setTitle($title);
+        try {
+            $entry = $this->get('wallabag_core.content_proxy')->updateEntry(
+                $entry,
+                $url,
+                [
+                    'title' => $title,
+                    'html' => $content,
+                    'url' => $url,
+                    'language' => $language,
+                    'date' => $publishedAt,
+                    // faking the preview picture
+                    'open_graph' => [
+                        'og_image' => $picture,
+                    ],
+                ]
+            );
+        } catch (\Exception $e) {
+            $this->get('logger')->error('Error while saving an entry', [
+                'exception' => $e,
+                'entry' => $entry,
+            ]);
+            $entry->setUrl($url);
         }
 
-        $tags = $request->request->get('tags', '');
+        $tags = $request->request->get('tags', []);
         if (!empty($tags)) {
             $this->get('wallabag_core.tags_assigner')->assignTagsToEntry($entry, $tags);
         }

diff --git a/src/Wallabag/CoreBundle/Helper/ContentProxy.php b/src/Wallabag/CoreBundle/Helper/ContentProxy.php
index 4b3e6fbb161fd176ad875b5f3b1d8857037d47df..e06ad3d6602ca19cac904d849d7dff0470b4aaf3 100644 (file)
--- a/src/Wallabag/CoreBundle/Helper/ContentProxy.php
+++ b/src/Wallabag/CoreBundle/Helper/ContentProxy.php
@@ -45,6 +45,18 @@ class ContentProxy
      */
     public function updateEntry(Entry $entry, $url, array $content = [])
     {
+        // ensure content is a bit cleaned up
+        if (!empty($content['html'])) {
+            $content['html'] = htmLawed($content['html'], [
+                'safe' => 1,
+                // which means: do not remove iframe elements
+                'elements' => '*+iframe',
+                'deny_attribute' => 'style',
+                'comment' => 1,
+                'cdata' => 1,
+            ]);
+        }
+
         // do we have to fetch the content or the provided one is ok?
         if (empty($content) || false === $this->validateContent($content)) {
             $fetchedContent = $this->graby->fetchContent($url);
@@ -57,7 +69,7 @@ class ContentProxy
         }
 
         $title = $content['title'];
-        if (!$title && isset($content['open_graph']['og_title'])) {
+        if (!$title && !empty($content['open_graph']['og_title'])) {
             $title = $content['open_graph']['og_title'];
         }
 
@@ -65,7 +77,7 @@ class ContentProxy
         if (false === $html) {
             $html = $this->fetchingErrorMessage;
 
-            if (isset($content['open_graph']['og_description'])) {
+            if (!empty($content['open_graph']['og_description'])) {
                 $html .= '<p><i>But we found a short description: </i></p>';
                 $html .= $content['open_graph']['og_description'];
             }
@@ -76,8 +88,12 @@ class ContentProxy
         $entry->setContent($html);
         $entry->setHttpStatus(isset($content['status']) ? $content['status'] : '');
 
-        if (isset($content['date']) && null !== $content['date'] && '' !== $content['date']) {
-            $entry->setPublishedAt(new \DateTime($content['date']));
+        if (!empty($content['date'])) {
+            try {
+                $entry->setPublishedAt(new \DateTime($content['date']));
+            } catch (\Exception $e) {
+                $this->logger->warn('Error while defining date', ['e' => $e, 'url' => $url, 'date' => $content['date']]);
+            }
         }
 
         if (!empty($content['authors'])) {
@@ -97,12 +113,12 @@ class ContentProxy
             $entry->setDomainName($domainName);
         }
 
-        if (isset($content['open_graph']['og_image']) && $content['open_graph']['og_image']) {
+        if (!empty($content['open_graph']['og_image'])) {
             $entry->setPreviewPicture($content['open_graph']['og_image']);
         }
 
         // if content is an image define as a preview too
-        if (isset($content['content_type']) && in_array($this->mimeGuesser->guess($content['content_type']), ['jpeg', 'jpg', 'gif', 'png'], true)) {
+        if (!empty($content['content_type']) && in_array($this->mimeGuesser->guess($content['content_type']), ['jpeg', 'jpg', 'gif', 'png'], true)) {
             $entry->setPreviewPicture($content['url']);
         }
 
@@ -128,6 +144,6 @@ class ContentProxy
      */
     private function validateContent(array $content)
     {
-        return isset($content['title']) && isset($content['html']) && isset($content['url']) && isset($content['language']) && isset($content['content_type']);
+        return !empty($content['title']) && !empty($content['html']) && !empty($content['url']);
     }
 }

diff --git a/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php b/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php
index bf7d373aa94d57f8716ed59f5f6d02f37ccd3b6f..1b0c06d2ec7de95f669d09243f87ca228aeef756 100644 (file)
--- a/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php
+++ b/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php
@@ -342,6 +342,9 @@ class EntryRestControllerTest extends WallabagApiTestCase
             'url' => 'http://www.lemonde.fr/pixels/article/2015/03/28/plongee-dans-l-univers-d-ingress-le-jeu-de-google-aux-frontieres-du-reel_4601155_4408996.html',
             'tags' => 'google',
             'title' => 'New title for my article',
+            'content' => 'my content',
+            'language' => 'de_DE',
+            'published_at' => '2016-09-08T11:55:58+0200',
         ]);
 
         $this->assertEquals(200, $this->client->getResponse()->getStatusCode());
@@ -355,6 +358,9 @@ class EntryRestControllerTest extends WallabagApiTestCase
         $this->assertEquals('New title for my article', $content['title']);
         $this->assertEquals(1, $content['user_id']);
         $this->assertCount(2, $content['tags']);
+        $this->assertSame('my content', $content['content']);
+        $this->assertSame('de_DE', $content['language']);
+        $this->assertSame('2016-09-08T11:55:58+0200', $content['published_at']);
     }
 
     public function testPostSameEntry()

diff --git a/tests/Wallabag/ImportBundle/Controller/WallabagV1ControllerTest.php b/tests/Wallabag/ImportBundle/Controller/WallabagV1ControllerTest.php
index 4ca6e623e02251a8076ec00f730697178a0fc541..2c492c207fc469acc7cf562ac4f3484764223c6d 100644 (file)
--- a/tests/Wallabag/ImportBundle/Controller/WallabagV1ControllerTest.php
+++ b/tests/Wallabag/ImportBundle/Controller/WallabagV1ControllerTest.php
@@ -112,16 +112,16 @@ class WallabagV1ControllerTest extends WallabagCoreTestCase
             ->get('doctrine.orm.entity_manager')
             ->getRepository('WallabagCoreBundle:Entry')
             ->findByUrlAndUserId(
-                'https://framablog.org/2014/02/05/framabag-service-libre-gratuit-interview-developpeur/',
+                'http://www.framablog.org/index.php/post/2014/02/05/Framabag-service-libre-gratuit-interview-developpeur',
                 $this->getLoggedInUserId()
             );
 
         $this->assertGreaterThan(1, $body = $crawler->filter('body')->extract(['_text']));
         $this->assertContains('flashes.import.notice.summary', $body[0]);
 
-        $this->assertNotEmpty($content->getMimetype(), 'Mimetype for http://www.framablog.org is ok');
-        $this->assertNotEmpty($content->getPreviewPicture(), 'Preview picture for http://www.framablog.org is ok');
-        $this->assertNotEmpty($content->getLanguage(), 'Language for http://www.framablog.org is ok');
+        $this->assertEmpty($content->getMimetype(), 'Mimetype for http://www.framablog.org is empty');
+        $this->assertEmpty($content->getPreviewPicture(), 'Preview picture for http://www.framablog.org is empty');
+        $this->assertEmpty($content->getLanguage(), 'Language for http://www.framablog.org is empty');
 
         $tags = $content->getTags();
         $this->assertContains('foot', $tags, 'It includes the "foot" tag');

diff --git a/tests/Wallabag/ImportBundle/Controller/WallabagV2ControllerTest.php b/tests/Wallabag/ImportBundle/Controller/WallabagV2ControllerTest.php
index 18a025226c5c3fdad89977cc632bfae44a0fc126..9df827ea94d66f1957030d1fee2a818a5d541701 100644 (file)
--- a/tests/Wallabag/ImportBundle/Controller/WallabagV2ControllerTest.php
+++ b/tests/Wallabag/ImportBundle/Controller/WallabagV2ControllerTest.php
@@ -119,9 +119,9 @@ class WallabagV2ControllerTest extends WallabagCoreTestCase
                 $this->getLoggedInUserId()
             );
 
-        $this->assertNotEmpty($content->getMimetype(), 'Mimetype for http://www.liberation.fr is ok');
-        $this->assertNotEmpty($content->getPreviewPicture(), 'Preview picture for http://www.liberation.fr is ok');
-        $this->assertNotEmpty($content->getLanguage(), 'Language for http://www.liberation.fr is ok');
+        $this->assertEmpty($content->getMimetype(), 'Mimetype for http://www.liberation.fr is empty');
+        $this->assertEmpty($content->getPreviewPicture(), 'Preview picture for http://www.liberation.fr is empty');
+        $this->assertEmpty($content->getLanguage(), 'Language for http://www.liberation.fr is empty');
 
         $tags = $content->getTags();
         $this->assertContains('foot', $tags, 'It includes the "foot" tag');

diff --git a/tests/Wallabag/ImportBundle/Import/ReadabilityImportTest.php b/tests/Wallabag/ImportBundle/Import/ReadabilityImportTest.php
index 254f0a25c3df68ef14bdd3807ba5fc29a0dbe3fa..25eedd1bebf6a1c01543850dde6c4b5692e3390c 100644 (file)
--- a/tests/Wallabag/ImportBundle/Import/ReadabilityImportTest.php
+++ b/tests/Wallabag/ImportBundle/Import/ReadabilityImportTest.php
@@ -67,14 +67,14 @@ class ReadabilityImportTest extends \PHPUnit_Framework_TestCase
 
     public function testImport()
     {
-        $readabilityImport = $this->getReadabilityImport(false, 24);
+        $readabilityImport = $this->getReadabilityImport(false, 23);
         $readabilityImport->setFilepath(__DIR__.'/../fixtures/readability.json');
 
         $entryRepo = $this->getMockBuilder('Wallabag\CoreBundle\Repository\EntryRepository')
             ->disableOriginalConstructor()
             ->getMock();
 
-        $entryRepo->expects($this->exactly(24))
+        $entryRepo->expects($this->exactly(23))
             ->method('findByUrlAndUserId')
             ->willReturn(false);
 
@@ -88,14 +88,14 @@ class ReadabilityImportTest extends \PHPUnit_Framework_TestCase
             ->getMock();
 
         $this->contentProxy
-            ->expects($this->exactly(24))
+            ->expects($this->exactly(23))
             ->method('updateEntry')
             ->willReturn($entry);
 
         $res = $readabilityImport->import();
 
         $this->assertTrue($res);
-        $this->assertEquals(['skipped' => 0, 'imported' => 24, 'queued' => 0], $readabilityImport->getSummary());
+        $this->assertEquals(['skipped' => 0, 'imported' => 23, 'queued' => 0], $readabilityImport->getSummary());
     }
 
     public function testImportAndMarkAllAsRead()
@@ -165,7 +165,7 @@ class ReadabilityImportTest extends \PHPUnit_Framework_TestCase
             ->getMock();
 
         $producer
-            ->expects($this->exactly(24))
+            ->expects($this->exactly(23))
             ->method('publish');
 
         $readabilityImport->setProducer($producer);
@@ -173,7 +173,7 @@ class ReadabilityImportTest extends \PHPUnit_Framework_TestCase
         $res = $readabilityImport->setMarkAsRead(true)->import();
 
         $this->assertTrue($res);
-        $this->assertEquals(['skipped' => 0, 'imported' => 0, 'queued' => 24], $readabilityImport->getSummary());
+        $this->assertEquals(['skipped' => 0, 'imported' => 0, 'queued' => 23], $readabilityImport->getSummary());
     }
 
     public function testImportWithRedis()
@@ -211,7 +211,7 @@ class ReadabilityImportTest extends \PHPUnit_Framework_TestCase
         $res = $readabilityImport->setMarkAsRead(true)->import();
 
         $this->assertTrue($res);
-        $this->assertEquals(['skipped' => 0, 'imported' => 0, 'queued' => 24], $readabilityImport->getSummary());
+        $this->assertEquals(['skipped' => 0, 'imported' => 0, 'queued' => 23], $readabilityImport->getSummary());
 
         $this->assertNotEmpty($redisMock->lpop('readability'));
     }

diff --git a/tests/Wallabag/ImportBundle/fixtures/readability.json b/tests/Wallabag/ImportBundle/fixtures/readability.json
index 32f6fa530c904ff5bf33a7970db501152af787d8..88b66c468b45711f6561e2a610120d35b3593de8 100644 (file)
--- a/tests/Wallabag/ImportBundle/fixtures/readability.json
+++ b/tests/Wallabag/ImportBundle/fixtures/readability.json
@@ -10,13 +10,6 @@
             "article__title": "We Looked At 167,943 Tweets & Found Out Hashtags Are Worthless",
             "archive": false
         },
-        {
-            "article__title": "Réfugiés: l'UE va créer 100 000 places d'accueil dans les Balkans",
-            "article__url": "http://www.liberation.fr/planete/2015/10/26/refugies-l-ue-va-creer-100-000-places-d-accueil-dans-les-balkans_1408867",
-            "archive": false,
-            "date_added": "2016-09-08T11:55:58+0200",
-            "favorite": false
-        },
         {
             "article__title": "No title found",
             "article__url": "http://news.nationalgeographic.com/2016/02/160211-albatrosses-mothers-babies-animals-science/&sf20739758=1",