diff options
author | Jeremy Benoist <jeremy.benoist@gmail.com> | 2017-05-11 08:14:29 +0200 |
---|---|---|
committer | Jeremy Benoist <jeremy.benoist@gmail.com> | 2017-05-31 13:59:45 +0200 |
commit | e668a8124c46d47add4248963d77f3b29b37b3ce (patch) | |
tree | b9b56d19b06fe268d025c3591119470162efc99a | |
parent | 4423b88c5b2c2d530b0a83a822f521a61ca4d4b8 (diff) | |
download | wallabag-e668a8124c46d47add4248963d77f3b29b37b3ce.tar.gz wallabag-e668a8124c46d47add4248963d77f3b29b37b3ce.tar.zst wallabag-e668a8124c46d47add4248963d77f3b29b37b3ce.zip |
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)
7 files changed, 74 insertions, 43 deletions
diff --git a/src/Wallabag/ApiBundle/Controller/EntryRestController.php b/src/Wallabag/ApiBundle/Controller/EntryRestController.php index 31bb67fd..dfd04fb4 100644 --- a/src/Wallabag/ApiBundle/Controller/EntryRestController.php +++ b/src/Wallabag/ApiBundle/Controller/EntryRestController.php | |||
@@ -280,6 +280,10 @@ class EntryRestController extends WallabagRestController | |||
280 | * {"name"="tags", "dataType"="string", "required"=false, "format"="tag1,tag2,tag3", "description"="a comma-separated list of tags."}, | 280 | * {"name"="tags", "dataType"="string", "required"=false, "format"="tag1,tag2,tag3", "description"="a comma-separated list of tags."}, |
281 | * {"name"="starred", "dataType"="integer", "required"=false, "format"="1 or 0", "description"="entry already starred"}, | 281 | * {"name"="starred", "dataType"="integer", "required"=false, "format"="1 or 0", "description"="entry already starred"}, |
282 | * {"name"="archive", "dataType"="integer", "required"=false, "format"="1 or 0", "description"="entry already archived"}, | 282 | * {"name"="archive", "dataType"="integer", "required"=false, "format"="1 or 0", "description"="entry already archived"}, |
283 | * {"name"="content", "dataType"="string", "required"=false, "description"="Content of the entry"}, | ||
284 | * {"name"="language", "dataType"="string", "required"=false, "description"="Language of the entry"}, | ||
285 | * {"name"="preview_picture", "dataType"="string", "required"=false, "description"="Preview picture of the entry"}, | ||
286 | * {"name"="published_at", "dataType"="datetime", "format"="YYYY-MM-DDTHH:II:SS+TZ", "required"=false, "description"="Published date of the entry"}, | ||
283 | * } | 287 | * } |
284 | * ) | 288 | * ) |
285 | * | 289 | * |
@@ -293,30 +297,42 @@ class EntryRestController extends WallabagRestController | |||
293 | $title = $request->request->get('title'); | 297 | $title = $request->request->get('title'); |
294 | $isArchived = $request->request->get('archive'); | 298 | $isArchived = $request->request->get('archive'); |
295 | $isStarred = $request->request->get('starred'); | 299 | $isStarred = $request->request->get('starred'); |
300 | $content = $request->request->get('content'); | ||
301 | $language = $request->request->get('language'); | ||
302 | $picture = $request->request->get('preview_picture'); | ||
303 | $publishedAt = $request->request->get('published_at'); | ||
296 | 304 | ||
297 | $entry = $this->get('wallabag_core.entry_repository')->findByUrlAndUserId($url, $this->getUser()->getId()); | 305 | $entry = $this->get('wallabag_core.entry_repository')->findByUrlAndUserId($url, $this->getUser()->getId()); |
298 | 306 | ||
299 | if (false === $entry) { | 307 | if (false === $entry) { |
300 | $entry = new Entry($this->getUser()); | 308 | $entry = new Entry($this->getUser()); |
301 | try { | ||
302 | $entry = $this->get('wallabag_core.content_proxy')->updateEntry( | ||
303 | $entry, | ||
304 | $url | ||
305 | ); | ||
306 | } catch (\Exception $e) { | ||
307 | $this->get('logger')->error('Error while saving an entry', [ | ||
308 | 'exception' => $e, | ||
309 | 'entry' => $entry, | ||
310 | ]); | ||
311 | $entry->setUrl($url); | ||
312 | } | ||
313 | } | 309 | } |
314 | 310 | ||
315 | if (!is_null($title)) { | 311 | try { |
316 | $entry->setTitle($title); | 312 | $entry = $this->get('wallabag_core.content_proxy')->updateEntry( |
313 | $entry, | ||
314 | $url, | ||
315 | [ | ||
316 | 'title' => $title, | ||
317 | 'html' => $content, | ||
318 | 'url' => $url, | ||
319 | 'language' => $language, | ||
320 | 'date' => $publishedAt, | ||
321 | // faking the preview picture | ||
322 | 'open_graph' => [ | ||
323 | 'og_image' => $picture, | ||
324 | ], | ||
325 | ] | ||
326 | ); | ||
327 | } catch (\Exception $e) { | ||
328 | $this->get('logger')->error('Error while saving an entry', [ | ||
329 | 'exception' => $e, | ||
330 | 'entry' => $entry, | ||
331 | ]); | ||
332 | $entry->setUrl($url); | ||
317 | } | 333 | } |
318 | 334 | ||
319 | $tags = $request->request->get('tags', ''); | 335 | $tags = $request->request->get('tags', []); |
320 | if (!empty($tags)) { | 336 | if (!empty($tags)) { |
321 | $this->get('wallabag_core.tags_assigner')->assignTagsToEntry($entry, $tags); | 337 | $this->get('wallabag_core.tags_assigner')->assignTagsToEntry($entry, $tags); |
322 | } | 338 | } |
diff --git a/src/Wallabag/CoreBundle/Helper/ContentProxy.php b/src/Wallabag/CoreBundle/Helper/ContentProxy.php index 4b3e6fbb..e06ad3d6 100644 --- a/src/Wallabag/CoreBundle/Helper/ContentProxy.php +++ b/src/Wallabag/CoreBundle/Helper/ContentProxy.php | |||
@@ -45,6 +45,18 @@ class ContentProxy | |||
45 | */ | 45 | */ |
46 | public function updateEntry(Entry $entry, $url, array $content = []) | 46 | public function updateEntry(Entry $entry, $url, array $content = []) |
47 | { | 47 | { |
48 | // ensure content is a bit cleaned up | ||
49 | if (!empty($content['html'])) { | ||
50 | $content['html'] = htmLawed($content['html'], [ | ||
51 | 'safe' => 1, | ||
52 | // which means: do not remove iframe elements | ||
53 | 'elements' => '*+iframe', | ||
54 | 'deny_attribute' => 'style', | ||
55 | 'comment' => 1, | ||
56 | 'cdata' => 1, | ||
57 | ]); | ||
58 | } | ||
59 | |||
48 | // do we have to fetch the content or the provided one is ok? | 60 | // do we have to fetch the content or the provided one is ok? |
49 | if (empty($content) || false === $this->validateContent($content)) { | 61 | if (empty($content) || false === $this->validateContent($content)) { |
50 | $fetchedContent = $this->graby->fetchContent($url); | 62 | $fetchedContent = $this->graby->fetchContent($url); |
@@ -57,7 +69,7 @@ class ContentProxy | |||
57 | } | 69 | } |
58 | 70 | ||
59 | $title = $content['title']; | 71 | $title = $content['title']; |
60 | if (!$title && isset($content['open_graph']['og_title'])) { | 72 | if (!$title && !empty($content['open_graph']['og_title'])) { |
61 | $title = $content['open_graph']['og_title']; | 73 | $title = $content['open_graph']['og_title']; |
62 | } | 74 | } |
63 | 75 | ||
@@ -65,7 +77,7 @@ class ContentProxy | |||
65 | if (false === $html) { | 77 | if (false === $html) { |
66 | $html = $this->fetchingErrorMessage; | 78 | $html = $this->fetchingErrorMessage; |
67 | 79 | ||
68 | if (isset($content['open_graph']['og_description'])) { | 80 | if (!empty($content['open_graph']['og_description'])) { |
69 | $html .= '<p><i>But we found a short description: </i></p>'; | 81 | $html .= '<p><i>But we found a short description: </i></p>'; |
70 | $html .= $content['open_graph']['og_description']; | 82 | $html .= $content['open_graph']['og_description']; |
71 | } | 83 | } |
@@ -76,8 +88,12 @@ class ContentProxy | |||
76 | $entry->setContent($html); | 88 | $entry->setContent($html); |
77 | $entry->setHttpStatus(isset($content['status']) ? $content['status'] : ''); | 89 | $entry->setHttpStatus(isset($content['status']) ? $content['status'] : ''); |
78 | 90 | ||
79 | if (isset($content['date']) && null !== $content['date'] && '' !== $content['date']) { | 91 | if (!empty($content['date'])) { |
80 | $entry->setPublishedAt(new \DateTime($content['date'])); | 92 | try { |
93 | $entry->setPublishedAt(new \DateTime($content['date'])); | ||
94 | } catch (\Exception $e) { | ||
95 | $this->logger->warn('Error while defining date', ['e' => $e, 'url' => $url, 'date' => $content['date']]); | ||
96 | } | ||
81 | } | 97 | } |
82 | 98 | ||
83 | if (!empty($content['authors'])) { | 99 | if (!empty($content['authors'])) { |
@@ -97,12 +113,12 @@ class ContentProxy | |||
97 | $entry->setDomainName($domainName); | 113 | $entry->setDomainName($domainName); |
98 | } | 114 | } |
99 | 115 | ||
100 | if (isset($content['open_graph']['og_image']) && $content['open_graph']['og_image']) { | 116 | if (!empty($content['open_graph']['og_image'])) { |
101 | $entry->setPreviewPicture($content['open_graph']['og_image']); | 117 | $entry->setPreviewPicture($content['open_graph']['og_image']); |
102 | } | 118 | } |
103 | 119 | ||
104 | // if content is an image define as a preview too | 120 | // if content is an image define as a preview too |
105 | if (isset($content['content_type']) && in_array($this->mimeGuesser->guess($content['content_type']), ['jpeg', 'jpg', 'gif', 'png'], true)) { | 121 | if (!empty($content['content_type']) && in_array($this->mimeGuesser->guess($content['content_type']), ['jpeg', 'jpg', 'gif', 'png'], true)) { |
106 | $entry->setPreviewPicture($content['url']); | 122 | $entry->setPreviewPicture($content['url']); |
107 | } | 123 | } |
108 | 124 | ||
@@ -128,6 +144,6 @@ class ContentProxy | |||
128 | */ | 144 | */ |
129 | private function validateContent(array $content) | 145 | private function validateContent(array $content) |
130 | { | 146 | { |
131 | return isset($content['title']) && isset($content['html']) && isset($content['url']) && isset($content['language']) && isset($content['content_type']); | 147 | return !empty($content['title']) && !empty($content['html']) && !empty($content['url']); |
132 | } | 148 | } |
133 | } | 149 | } |
diff --git a/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php b/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php index bf7d373a..1b0c06d2 100644 --- a/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php +++ b/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php | |||
@@ -342,6 +342,9 @@ class EntryRestControllerTest extends WallabagApiTestCase | |||
342 | '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', | 342 | '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', |
343 | 'tags' => 'google', | 343 | 'tags' => 'google', |
344 | 'title' => 'New title for my article', | 344 | 'title' => 'New title for my article', |
345 | 'content' => 'my content', | ||
346 | 'language' => 'de_DE', | ||
347 | 'published_at' => '2016-09-08T11:55:58+0200', | ||
345 | ]); | 348 | ]); |
346 | 349 | ||
347 | $this->assertEquals(200, $this->client->getResponse()->getStatusCode()); | 350 | $this->assertEquals(200, $this->client->getResponse()->getStatusCode()); |
@@ -355,6 +358,9 @@ class EntryRestControllerTest extends WallabagApiTestCase | |||
355 | $this->assertEquals('New title for my article', $content['title']); | 358 | $this->assertEquals('New title for my article', $content['title']); |
356 | $this->assertEquals(1, $content['user_id']); | 359 | $this->assertEquals(1, $content['user_id']); |
357 | $this->assertCount(2, $content['tags']); | 360 | $this->assertCount(2, $content['tags']); |
361 | $this->assertSame('my content', $content['content']); | ||
362 | $this->assertSame('de_DE', $content['language']); | ||
363 | $this->assertSame('2016-09-08T11:55:58+0200', $content['published_at']); | ||
358 | } | 364 | } |
359 | 365 | ||
360 | public function testPostSameEntry() | 366 | public function testPostSameEntry() |
diff --git a/tests/Wallabag/ImportBundle/Controller/WallabagV1ControllerTest.php b/tests/Wallabag/ImportBundle/Controller/WallabagV1ControllerTest.php index 4ca6e623..2c492c20 100644 --- a/tests/Wallabag/ImportBundle/Controller/WallabagV1ControllerTest.php +++ b/tests/Wallabag/ImportBundle/Controller/WallabagV1ControllerTest.php | |||
@@ -112,16 +112,16 @@ class WallabagV1ControllerTest extends WallabagCoreTestCase | |||
112 | ->get('doctrine.orm.entity_manager') | 112 | ->get('doctrine.orm.entity_manager') |
113 | ->getRepository('WallabagCoreBundle:Entry') | 113 | ->getRepository('WallabagCoreBundle:Entry') |
114 | ->findByUrlAndUserId( | 114 | ->findByUrlAndUserId( |
115 | 'https://framablog.org/2014/02/05/framabag-service-libre-gratuit-interview-developpeur/', | 115 | 'http://www.framablog.org/index.php/post/2014/02/05/Framabag-service-libre-gratuit-interview-developpeur', |
116 | $this->getLoggedInUserId() | 116 | $this->getLoggedInUserId() |
117 | ); | 117 | ); |
118 | 118 | ||
119 | $this->assertGreaterThan(1, $body = $crawler->filter('body')->extract(['_text'])); | 119 | $this->assertGreaterThan(1, $body = $crawler->filter('body')->extract(['_text'])); |
120 | $this->assertContains('flashes.import.notice.summary', $body[0]); | 120 | $this->assertContains('flashes.import.notice.summary', $body[0]); |
121 | 121 | ||
122 | $this->assertNotEmpty($content->getMimetype(), 'Mimetype for http://www.framablog.org is ok'); | 122 | $this->assertEmpty($content->getMimetype(), 'Mimetype for http://www.framablog.org is empty'); |
123 | $this->assertNotEmpty($content->getPreviewPicture(), 'Preview picture for http://www.framablog.org is ok'); | 123 | $this->assertEmpty($content->getPreviewPicture(), 'Preview picture for http://www.framablog.org is empty'); |
124 | $this->assertNotEmpty($content->getLanguage(), 'Language for http://www.framablog.org is ok'); | 124 | $this->assertEmpty($content->getLanguage(), 'Language for http://www.framablog.org is empty'); |
125 | 125 | ||
126 | $tags = $content->getTags(); | 126 | $tags = $content->getTags(); |
127 | $this->assertContains('foot', $tags, 'It includes the "foot" tag'); | 127 | $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 18a02522..9df827ea 100644 --- a/tests/Wallabag/ImportBundle/Controller/WallabagV2ControllerTest.php +++ b/tests/Wallabag/ImportBundle/Controller/WallabagV2ControllerTest.php | |||
@@ -119,9 +119,9 @@ class WallabagV2ControllerTest extends WallabagCoreTestCase | |||
119 | $this->getLoggedInUserId() | 119 | $this->getLoggedInUserId() |
120 | ); | 120 | ); |
121 | 121 | ||
122 | $this->assertNotEmpty($content->getMimetype(), 'Mimetype for http://www.liberation.fr is ok'); | 122 | $this->assertEmpty($content->getMimetype(), 'Mimetype for http://www.liberation.fr is empty'); |
123 | $this->assertNotEmpty($content->getPreviewPicture(), 'Preview picture for http://www.liberation.fr is ok'); | 123 | $this->assertEmpty($content->getPreviewPicture(), 'Preview picture for http://www.liberation.fr is empty'); |
124 | $this->assertNotEmpty($content->getLanguage(), 'Language for http://www.liberation.fr is ok'); | 124 | $this->assertEmpty($content->getLanguage(), 'Language for http://www.liberation.fr is empty'); |
125 | 125 | ||
126 | $tags = $content->getTags(); | 126 | $tags = $content->getTags(); |
127 | $this->assertContains('foot', $tags, 'It includes the "foot" tag'); | 127 | $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 254f0a25..25eedd1b 100644 --- a/tests/Wallabag/ImportBundle/Import/ReadabilityImportTest.php +++ b/tests/Wallabag/ImportBundle/Import/ReadabilityImportTest.php | |||
@@ -67,14 +67,14 @@ class ReadabilityImportTest extends \PHPUnit_Framework_TestCase | |||
67 | 67 | ||
68 | public function testImport() | 68 | public function testImport() |
69 | { | 69 | { |
70 | $readabilityImport = $this->getReadabilityImport(false, 24); | 70 | $readabilityImport = $this->getReadabilityImport(false, 23); |
71 | $readabilityImport->setFilepath(__DIR__.'/../fixtures/readability.json'); | 71 | $readabilityImport->setFilepath(__DIR__.'/../fixtures/readability.json'); |
72 | 72 | ||
73 | $entryRepo = $this->getMockBuilder('Wallabag\CoreBundle\Repository\EntryRepository') | 73 | $entryRepo = $this->getMockBuilder('Wallabag\CoreBundle\Repository\EntryRepository') |
74 | ->disableOriginalConstructor() | 74 | ->disableOriginalConstructor() |
75 | ->getMock(); | 75 | ->getMock(); |
76 | 76 | ||
77 | $entryRepo->expects($this->exactly(24)) | 77 | $entryRepo->expects($this->exactly(23)) |
78 | ->method('findByUrlAndUserId') | 78 | ->method('findByUrlAndUserId') |
79 | ->willReturn(false); | 79 | ->willReturn(false); |
80 | 80 | ||
@@ -88,14 +88,14 @@ class ReadabilityImportTest extends \PHPUnit_Framework_TestCase | |||
88 | ->getMock(); | 88 | ->getMock(); |
89 | 89 | ||
90 | $this->contentProxy | 90 | $this->contentProxy |
91 | ->expects($this->exactly(24)) | 91 | ->expects($this->exactly(23)) |
92 | ->method('updateEntry') | 92 | ->method('updateEntry') |
93 | ->willReturn($entry); | 93 | ->willReturn($entry); |
94 | 94 | ||
95 | $res = $readabilityImport->import(); | 95 | $res = $readabilityImport->import(); |
96 | 96 | ||
97 | $this->assertTrue($res); | 97 | $this->assertTrue($res); |
98 | $this->assertEquals(['skipped' => 0, 'imported' => 24, 'queued' => 0], $readabilityImport->getSummary()); | 98 | $this->assertEquals(['skipped' => 0, 'imported' => 23, 'queued' => 0], $readabilityImport->getSummary()); |
99 | } | 99 | } |
100 | 100 | ||
101 | public function testImportAndMarkAllAsRead() | 101 | public function testImportAndMarkAllAsRead() |
@@ -165,7 +165,7 @@ class ReadabilityImportTest extends \PHPUnit_Framework_TestCase | |||
165 | ->getMock(); | 165 | ->getMock(); |
166 | 166 | ||
167 | $producer | 167 | $producer |
168 | ->expects($this->exactly(24)) | 168 | ->expects($this->exactly(23)) |
169 | ->method('publish'); | 169 | ->method('publish'); |
170 | 170 | ||
171 | $readabilityImport->setProducer($producer); | 171 | $readabilityImport->setProducer($producer); |
@@ -173,7 +173,7 @@ class ReadabilityImportTest extends \PHPUnit_Framework_TestCase | |||
173 | $res = $readabilityImport->setMarkAsRead(true)->import(); | 173 | $res = $readabilityImport->setMarkAsRead(true)->import(); |
174 | 174 | ||
175 | $this->assertTrue($res); | 175 | $this->assertTrue($res); |
176 | $this->assertEquals(['skipped' => 0, 'imported' => 0, 'queued' => 24], $readabilityImport->getSummary()); | 176 | $this->assertEquals(['skipped' => 0, 'imported' => 0, 'queued' => 23], $readabilityImport->getSummary()); |
177 | } | 177 | } |
178 | 178 | ||
179 | public function testImportWithRedis() | 179 | public function testImportWithRedis() |
@@ -211,7 +211,7 @@ class ReadabilityImportTest extends \PHPUnit_Framework_TestCase | |||
211 | $res = $readabilityImport->setMarkAsRead(true)->import(); | 211 | $res = $readabilityImport->setMarkAsRead(true)->import(); |
212 | 212 | ||
213 | $this->assertTrue($res); | 213 | $this->assertTrue($res); |
214 | $this->assertEquals(['skipped' => 0, 'imported' => 0, 'queued' => 24], $readabilityImport->getSummary()); | 214 | $this->assertEquals(['skipped' => 0, 'imported' => 0, 'queued' => 23], $readabilityImport->getSummary()); |
215 | 215 | ||
216 | $this->assertNotEmpty($redisMock->lpop('readability')); | 216 | $this->assertNotEmpty($redisMock->lpop('readability')); |
217 | } | 217 | } |
diff --git a/tests/Wallabag/ImportBundle/fixtures/readability.json b/tests/Wallabag/ImportBundle/fixtures/readability.json index 32f6fa53..88b66c46 100644 --- a/tests/Wallabag/ImportBundle/fixtures/readability.json +++ b/tests/Wallabag/ImportBundle/fixtures/readability.json | |||
@@ -11,13 +11,6 @@ | |||
11 | "archive": false | 11 | "archive": false |
12 | }, | 12 | }, |
13 | { | 13 | { |
14 | "article__title": "Réfugiés: l'UE va créer 100 000 places d'accueil dans les Balkans", | ||
15 | "article__url": "http://www.liberation.fr/planete/2015/10/26/refugies-l-ue-va-creer-100-000-places-d-accueil-dans-les-balkans_1408867", | ||
16 | "archive": false, | ||
17 | "date_added": "2016-09-08T11:55:58+0200", | ||
18 | "favorite": false | ||
19 | }, | ||
20 | { | ||
21 | "article__title": "No title found", | 14 | "article__title": "No title found", |
22 | "article__url": "http://news.nationalgeographic.com/2016/02/160211-albatrosses-mothers-babies-animals-science/&sf20739758=1", | 15 | "article__url": "http://news.nationalgeographic.com/2016/02/160211-albatrosses-mothers-babies-animals-science/&sf20739758=1", |
23 | "archive": false, | 16 | "archive": false, |