From 5f28fd4114f61300fc76e71524defa3bd1a13193 Mon Sep 17 00:00:00 2001 From: Athou Date: Tue, 4 Nov 2014 11:23:58 +0100 Subject: [PATCH 1/8] initial support for entry filtering --- pom.xml | 6 ++ .../backend/feed/FeedEntryFilter.java | 89 +++++++++++++++++++ .../backend/feed/FeedRefreshUpdater.java | 2 +- .../backend/model/FeedSubscription.java | 3 + .../backend/service/FeedUpdateService.java | 20 ++++- .../resources/changelogs/db.changelog-2.1.xml | 11 +++ src/main/resources/migrations.xml | 1 + .../backend/feed/FeedEntryFilterTest.java | 64 +++++++++++++ 8 files changed, 193 insertions(+), 3 deletions(-) create mode 100644 src/main/java/com/commafeed/backend/feed/FeedEntryFilter.java create mode 100644 src/main/resources/changelogs/db.changelog-2.1.xml create mode 100644 src/test/java/com/commafeed/backend/feed/FeedEntryFilterTest.java diff --git a/pom.xml b/pom.xml index 43157dd6..29840754 100644 --- a/pom.xml +++ b/pom.xml @@ -191,6 +191,12 @@ 1.7.7 + + org.apache.commons + commons-jexl + 2.1.1 + + com.google.inject guice diff --git a/src/main/java/com/commafeed/backend/feed/FeedEntryFilter.java b/src/main/java/com/commafeed/backend/feed/FeedEntryFilter.java new file mode 100644 index 00000000..03d6fbd7 --- /dev/null +++ b/src/main/java/com/commafeed/backend/feed/FeedEntryFilter.java @@ -0,0 +1,89 @@ +package com.commafeed.backend.feed; + +import lombok.Data; +import lombok.RequiredArgsConstructor; + +import org.apache.commons.jexl2.Expression; +import org.apache.commons.jexl2.JexlContext; +import org.apache.commons.jexl2.JexlEngine; +import org.apache.commons.jexl2.JexlInfo; +import org.apache.commons.jexl2.MapContext; +import org.apache.commons.jexl2.introspection.JexlMethod; +import org.apache.commons.jexl2.introspection.JexlPropertyGet; +import org.apache.commons.jexl2.introspection.Uberspect; +import org.apache.commons.jexl2.introspection.UberspectImpl; +import org.apache.commons.lang3.StringUtils; +import org.apache.commons.logging.LogFactory; + +import com.commafeed.backend.model.FeedEntry; + +@RequiredArgsConstructor +public class FeedEntryFilter { + + private static final JexlEngine ENGINE = initEngine(); + + private static JexlEngine initEngine() { + // classloader that prevents object creation + ClassLoader cl = new ClassLoader() { + @Override + public Class loadClass(String name) throws ClassNotFoundException { + return null; + } + + @Override + protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + return null; + } + }; + + // uberspect that prevents access to .class and .getClass() + Uberspect uberspect = new UberspectImpl(LogFactory.getLog(JexlEngine.class)) { + @Override + public JexlPropertyGet getPropertyGet(Object obj, Object identifier, JexlInfo info) { + if ("class".equals(identifier)) { + return null; + } + return super.getPropertyGet(obj, identifier, info); + } + + @Override + public JexlMethod getMethod(Object obj, String method, Object[] args, JexlInfo info) { + if ("getClass".equals(method)) { + return null; + } + return super.getMethod(obj, method, args, info); + } + }; + + JexlEngine engine = new JexlEngine(uberspect, null, null, null); + engine.setClassLoader(cl); + return engine; + } + + private final String filter; + + public boolean matchesEntry(FeedEntry entry) { + if (StringUtils.isBlank(filter)) { + return true; + } + + Expression expression = ENGINE.createExpression(filter); + + JexlContext context = new MapContext(); + context.set("title", entry.getContent().getTitle().toLowerCase()); + context.set("author", entry.getContent().getAuthor().toLowerCase()); + context.set("content", entry.getContent().getContent().toLowerCase()); + context.set("url", entry.getUrl().toLowerCase()); + + return (boolean) expression.evaluate(context); + } + + @Data + private static class Model { + private String title; + private String author; + private String content; + private String url; + } + +} diff --git a/src/main/java/com/commafeed/backend/feed/FeedRefreshUpdater.java b/src/main/java/com/commafeed/backend/feed/FeedRefreshUpdater.java index b80de7c0..52beb7ca 100644 --- a/src/main/java/com/commafeed/backend/feed/FeedRefreshUpdater.java +++ b/src/main/java/com/commafeed/backend/feed/FeedRefreshUpdater.java @@ -193,7 +193,7 @@ public class FeedRefreshUpdater implements Managed { boolean inserted = new UnitOfWork(sessionFactory) { @Override protected Boolean runInSession() throws Exception { - return feedUpdateService.addEntry(feed, entry); + return feedUpdateService.addEntry(feed, entry, subscriptions); } }.run(); if (inserted) { diff --git a/src/main/java/com/commafeed/backend/model/FeedSubscription.java b/src/main/java/com/commafeed/backend/model/FeedSubscription.java index 363665eb..5ef2e711 100644 --- a/src/main/java/com/commafeed/backend/model/FeedSubscription.java +++ b/src/main/java/com/commafeed/backend/model/FeedSubscription.java @@ -40,4 +40,7 @@ public class FeedSubscription extends AbstractModel { private Integer position; + @Column(length = 4096) + private String filter = "author.contains('a')"; + } diff --git a/src/main/java/com/commafeed/backend/service/FeedUpdateService.java b/src/main/java/com/commafeed/backend/service/FeedUpdateService.java index e0c68e17..0f51651e 100644 --- a/src/main/java/com/commafeed/backend/service/FeedUpdateService.java +++ b/src/main/java/com/commafeed/backend/service/FeedUpdateService.java @@ -1,6 +1,7 @@ package com.commafeed.backend.service; import java.util.Date; +import java.util.List; import javax.inject.Inject; import javax.inject.Singleton; @@ -10,21 +11,26 @@ import lombok.RequiredArgsConstructor; import org.apache.commons.codec.digest.DigestUtils; import com.commafeed.backend.dao.FeedEntryDAO; +import com.commafeed.backend.dao.FeedEntryStatusDAO; +import com.commafeed.backend.feed.FeedEntryFilter; import com.commafeed.backend.model.Feed; import com.commafeed.backend.model.FeedEntry; import com.commafeed.backend.model.FeedEntryContent; +import com.commafeed.backend.model.FeedEntryStatus; +import com.commafeed.backend.model.FeedSubscription; @RequiredArgsConstructor(onConstructor = @__({ @Inject })) @Singleton public class FeedUpdateService { private final FeedEntryDAO feedEntryDAO; + private final FeedEntryStatusDAO feedEntryStatusDAO; private final FeedEntryContentService feedEntryContentService; /** * this is NOT thread-safe */ - public boolean addEntry(Feed feed, FeedEntry entry) { + public boolean addEntry(Feed feed, FeedEntry entry, List subscriptions) { Long existing = feedEntryDAO.findExisting(entry.getGuid(), feed); if (existing != null) { @@ -36,8 +42,18 @@ public class FeedUpdateService { entry.setContent(content); entry.setInserted(new Date()); entry.setFeed(feed); - feedEntryDAO.saveOrUpdate(entry); + + // if filter does not match the entry, mark it as read + for (FeedSubscription sub : subscriptions) { + FeedEntryFilter filter = new FeedEntryFilter(sub.getFilter()); + if (!filter.matchesEntry(entry)) { + FeedEntryStatus status = new FeedEntryStatus(sub.getUser(), sub, entry); + status.setRead(true); + feedEntryStatusDAO.saveOrUpdate(status); + } + } + return true; } } diff --git a/src/main/resources/changelogs/db.changelog-2.1.xml b/src/main/resources/changelogs/db.changelog-2.1.xml new file mode 100644 index 00000000..df158560 --- /dev/null +++ b/src/main/resources/changelogs/db.changelog-2.1.xml @@ -0,0 +1,11 @@ + + + + + + + + + + diff --git a/src/main/resources/migrations.xml b/src/main/resources/migrations.xml index acc1dafa..8da3a73d 100644 --- a/src/main/resources/migrations.xml +++ b/src/main/resources/migrations.xml @@ -9,5 +9,6 @@ + \ No newline at end of file diff --git a/src/test/java/com/commafeed/backend/feed/FeedEntryFilterTest.java b/src/test/java/com/commafeed/backend/feed/FeedEntryFilterTest.java new file mode 100644 index 00000000..679dc63e --- /dev/null +++ b/src/test/java/com/commafeed/backend/feed/FeedEntryFilterTest.java @@ -0,0 +1,64 @@ +package com.commafeed.backend.feed; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import com.commafeed.backend.model.FeedEntry; +import com.commafeed.backend.model.FeedEntryContent; + +public class FeedEntryFilterTest { + + private FeedEntry entry; + private FeedEntryContent content; + + @Before + public void init() { + entry = new FeedEntry(); + entry.setUrl("https://github.com/Athou/commafeed"); + + content = new FeedEntryContent(); + content.setAuthor("Athou"); + content.setTitle("Merge pull request #662 from Athou/dw8"); + content.setContent("Merge pull request #662 from Athou/dw8"); + entry.setContent(content); + + } + + @Test + public void emptyFilterMatchesFilter() { + FeedEntryFilter filter = new FeedEntryFilter(null); + Assert.assertTrue(filter.matchesEntry(entry)); + } + + @Test + public void blankFilterMatchesFilter() { + FeedEntryFilter filter = new FeedEntryFilter(""); + Assert.assertTrue(filter.matchesEntry(entry)); + } + + @Test + public void simpleExpression() { + FeedEntryFilter filter = new FeedEntryFilter("author eq 'athou'"); + Assert.assertTrue(filter.matchesEntry(entry)); + } + + @Test + public void newIsDisabled() { + FeedEntryFilter filter = new FeedEntryFilter("null eq new ('java.lang.String', 'athou')"); + Assert.assertTrue(filter.matchesEntry(entry)); + } + + @Test + public void getClassMethodIsDisabled() { + FeedEntryFilter filter = new FeedEntryFilter("null eq ''.getClass()"); + Assert.assertTrue(filter.matchesEntry(entry)); + } + + @Test + public void dotClassIsDisabled() { + FeedEntryFilter filter = new FeedEntryFilter("null eq ''.class"); + Assert.assertTrue(filter.matchesEntry(entry)); + } + +} From a0c70d326fa2d98f977336ce9b8b1e0df5515069 Mon Sep 17 00:00:00 2001 From: Athou Date: Tue, 4 Nov 2014 15:09:45 +0100 Subject: [PATCH 2/8] class not used anymore --- .../com/commafeed/backend/feed/FeedEntryFilter.java | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/main/java/com/commafeed/backend/feed/FeedEntryFilter.java b/src/main/java/com/commafeed/backend/feed/FeedEntryFilter.java index 03d6fbd7..89bb8076 100644 --- a/src/main/java/com/commafeed/backend/feed/FeedEntryFilter.java +++ b/src/main/java/com/commafeed/backend/feed/FeedEntryFilter.java @@ -1,6 +1,5 @@ package com.commafeed.backend.feed; -import lombok.Data; import lombok.RequiredArgsConstructor; import org.apache.commons.jexl2.Expression; @@ -77,13 +76,4 @@ public class FeedEntryFilter { return (boolean) expression.evaluate(context); } - - @Data - private static class Model { - private String title; - private String author; - private String content; - private String url; - } - } From 5ce2823d0be2fd80fc9d8c66e245d92bc9703919 Mon Sep 17 00:00:00 2001 From: Athou Date: Tue, 4 Nov 2014 15:22:43 +0100 Subject: [PATCH 3/8] strip html tags --- .../java/com/commafeed/backend/feed/FeedEntryFilter.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/commafeed/backend/feed/FeedEntryFilter.java b/src/main/java/com/commafeed/backend/feed/FeedEntryFilter.java index 89bb8076..bc43e0a4 100644 --- a/src/main/java/com/commafeed/backend/feed/FeedEntryFilter.java +++ b/src/main/java/com/commafeed/backend/feed/FeedEntryFilter.java @@ -13,6 +13,7 @@ import org.apache.commons.jexl2.introspection.Uberspect; import org.apache.commons.jexl2.introspection.UberspectImpl; import org.apache.commons.lang3.StringUtils; import org.apache.commons.logging.LogFactory; +import org.jsoup.Jsoup; import com.commafeed.backend.model.FeedEntry; @@ -69,9 +70,9 @@ public class FeedEntryFilter { Expression expression = ENGINE.createExpression(filter); JexlContext context = new MapContext(); - context.set("title", entry.getContent().getTitle().toLowerCase()); + context.set("title", Jsoup.parse(entry.getContent().getTitle()).text().toLowerCase()); context.set("author", entry.getContent().getAuthor().toLowerCase()); - context.set("content", entry.getContent().getContent().toLowerCase()); + context.set("content", Jsoup.parse(entry.getContent().getContent()).text().toLowerCase()); context.set("url", entry.getUrl().toLowerCase()); return (boolean) expression.evaluate(context); From 8f2ba5e186f7452dc11eadd1146f528a52c28da6 Mon Sep 17 00:00:00 2001 From: Athou Date: Tue, 4 Nov 2014 16:01:37 +0100 Subject: [PATCH 4/8] initial ui for entry filtering --- src/main/app/js/controllers.js | 10 +++++--- .../app/templates/feeds.feed_details.html | 8 ++++++ .../backend/feed/FeedEntryFilter.java | 1 + .../backend/model/FeedSubscription.java | 2 +- .../frontend/model/Subscription.java | 4 +++ .../request/FeedModificationRequest.java | 3 +++ .../commafeed/frontend/resource/FeedREST.java | 25 +++++++++++++++++++ 7 files changed, 49 insertions(+), 4 deletions(-) diff --git a/src/main/app/js/controllers.js b/src/main/app/js/controllers.js index 7ae8d904..9eb1747d 100644 --- a/src/main/app/js/controllers.js +++ b/src/main/app/js/controllers.js @@ -322,17 +322,21 @@ module.controller('FeedDetailsCtrl', ['$scope', '$state', '$stateParams', 'FeedS $scope.save = function() { var sub = $scope.sub; + $scope.error = null; FeedService.modify({ id : sub.id, name : sub.name, position : sub.position, - categoryId : sub.categoryId + categoryId : sub.categoryId, + filter : sub.filter }, function() { CategoryService.init(); $state.transitionTo('feeds.view', { _id : 'all', _type : 'category' }); + }, function(e) { + $scope.error = e.data; }); }; }]); @@ -489,7 +493,7 @@ module.controller('ToolbarCtrl', [ type : $stateParams._type, id : $stateParams._id, olderThan : olderThan, - keywords: $location.search().q, + keywords : $location.search().q, read : true }); }; @@ -882,7 +886,7 @@ module.controller('FeedListCtrl', [ service.mark({ id : $scope.selectedId, olderThan : olderThan || $scope.timestamp, - keywords: $location.search().q, + keywords : $location.search().q, read : true }, function() { CategoryService.refresh(function() { diff --git a/src/main/app/templates/feeds.feed_details.html b/src/main/app/templates/feeds.feed_details.html index 699b25f9..3a173bd6 100644 --- a/src/main/app/templates/feeds.feed_details.html +++ b/src/main/app/templates/feeds.feed_details.html @@ -3,6 +3,7 @@

{{ 'details.feed_details' | translate }}

+
{{ error }}
@@ -69,6 +70,13 @@
+
+ +
+ +
+
+
diff --git a/src/main/java/com/commafeed/backend/feed/FeedEntryFilter.java b/src/main/java/com/commafeed/backend/feed/FeedEntryFilter.java index bc43e0a4..79ab69c8 100644 --- a/src/main/java/com/commafeed/backend/feed/FeedEntryFilter.java +++ b/src/main/java/com/commafeed/backend/feed/FeedEntryFilter.java @@ -56,6 +56,7 @@ public class FeedEntryFilter { }; JexlEngine engine = new JexlEngine(uberspect, null, null, null); + engine.setStrict(true); engine.setClassLoader(cl); return engine; } diff --git a/src/main/java/com/commafeed/backend/model/FeedSubscription.java b/src/main/java/com/commafeed/backend/model/FeedSubscription.java index 5ef2e711..92a08cec 100644 --- a/src/main/java/com/commafeed/backend/model/FeedSubscription.java +++ b/src/main/java/com/commafeed/backend/model/FeedSubscription.java @@ -41,6 +41,6 @@ public class FeedSubscription extends AbstractModel { private Integer position; @Column(length = 4096) - private String filter = "author.contains('a')"; + private String filter; } diff --git a/src/main/java/com/commafeed/frontend/model/Subscription.java b/src/main/java/com/commafeed/frontend/model/Subscription.java index a395b184..99e8d387 100644 --- a/src/main/java/com/commafeed/frontend/model/Subscription.java +++ b/src/main/java/com/commafeed/frontend/model/Subscription.java @@ -35,6 +35,7 @@ public class Subscription implements Serializable { sub.setUnread(unreadCount.getUnreadCount()); sub.setNewestItemTime(unreadCount.getNewestItemTime()); sub.setCategoryId(category == null ? null : String.valueOf(category.getId())); + sub.setFilter(subscription.getFilter()); return sub; } @@ -77,4 +78,7 @@ public class Subscription implements Serializable { @ApiModelProperty("date of the newest item") private Date newestItemTime; + @ApiModelProperty(value = "JEXL string evaluated on new entries to mark them as read if they do not match") + private String filter; + } \ No newline at end of file diff --git a/src/main/java/com/commafeed/frontend/model/request/FeedModificationRequest.java b/src/main/java/com/commafeed/frontend/model/request/FeedModificationRequest.java index c8b44876..595693e4 100644 --- a/src/main/java/com/commafeed/frontend/model/request/FeedModificationRequest.java +++ b/src/main/java/com/commafeed/frontend/model/request/FeedModificationRequest.java @@ -24,4 +24,7 @@ public class FeedModificationRequest implements Serializable { @ApiModelProperty(value = "new display position, null if not changed") private Integer position; + @ApiModelProperty(value = "JEXL string evaluated on new entries to mark them as read if they do not match") + private String filter; + } diff --git a/src/main/java/com/commafeed/frontend/resource/FeedREST.java b/src/main/java/com/commafeed/frontend/resource/FeedREST.java index cd29b06d..b16b349a 100644 --- a/src/main/java/com/commafeed/frontend/resource/FeedREST.java +++ b/src/main/java/com/commafeed/frontend/resource/FeedREST.java @@ -44,12 +44,15 @@ import com.commafeed.backend.cache.CacheService; import com.commafeed.backend.dao.FeedCategoryDAO; import com.commafeed.backend.dao.FeedEntryStatusDAO; import com.commafeed.backend.dao.FeedSubscriptionDAO; +import com.commafeed.backend.feed.FeedEntryFilter; import com.commafeed.backend.feed.FeedFetcher; import com.commafeed.backend.feed.FeedQueues; import com.commafeed.backend.feed.FeedUtils; import com.commafeed.backend.feed.FetchedFeed; import com.commafeed.backend.model.Feed; import com.commafeed.backend.model.FeedCategory; +import com.commafeed.backend.model.FeedEntry; +import com.commafeed.backend.model.FeedEntryContent; import com.commafeed.backend.model.FeedEntryStatus; import com.commafeed.backend.model.FeedSubscription; import com.commafeed.backend.model.User; @@ -93,6 +96,20 @@ import com.wordnik.swagger.annotations.ApiParam; @Singleton public class FeedREST { + private static final FeedEntry TEST_ENTRY = initTestEntry(); + + private static FeedEntry initTestEntry() { + FeedEntry entry = new FeedEntry(); + entry.setUrl("https://github.com/Athou/commafeed"); + + FeedEntryContent content = new FeedEntryContent(); + content.setAuthor("Athou"); + content.setTitle("Merge pull request #662 from Athou/dw8"); + content.setContent("Merge pull request #662 from Athou/dw8"); + entry.setContent(content); + return entry; + } + private final FeedSubscriptionDAO feedSubscriptionDAO; private final FeedCategoryDAO feedCategoryDAO; private final FeedEntryStatusDAO feedEntryStatusDAO; @@ -418,7 +435,15 @@ public class FeedREST { Preconditions.checkNotNull(req); Preconditions.checkNotNull(req.getId()); + try { + new FeedEntryFilter(req.getFilter()).matchesEntry(TEST_ENTRY); + } catch (Exception e) { + Throwable root = Throwables.getRootCause(e); + return Response.status(Status.BAD_REQUEST).entity(root.getMessage()).build(); + } + FeedSubscription subscription = feedSubscriptionDAO.findById(user, req.getId()); + subscription.setFilter(req.getFilter()); if (StringUtils.isNotBlank(req.getName())) { subscription.setTitle(req.getName()); From c8fded3c565af37122ff949aed86af24a5b3b772 Mon Sep 17 00:00:00 2001 From: Athou Date: Tue, 4 Nov 2014 16:19:50 +0100 Subject: [PATCH 5/8] don't crash if we cannot evaluate the filter --- .../commafeed/backend/service/FeedUpdateService.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/commafeed/backend/service/FeedUpdateService.java b/src/main/java/com/commafeed/backend/service/FeedUpdateService.java index 0f51651e..033fc734 100644 --- a/src/main/java/com/commafeed/backend/service/FeedUpdateService.java +++ b/src/main/java/com/commafeed/backend/service/FeedUpdateService.java @@ -7,6 +7,7 @@ import javax.inject.Inject; import javax.inject.Singleton; import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; import org.apache.commons.codec.digest.DigestUtils; @@ -19,6 +20,7 @@ import com.commafeed.backend.model.FeedEntryContent; import com.commafeed.backend.model.FeedEntryStatus; import com.commafeed.backend.model.FeedSubscription; +@Slf4j @RequiredArgsConstructor(onConstructor = @__({ @Inject })) @Singleton public class FeedUpdateService { @@ -47,7 +49,13 @@ public class FeedUpdateService { // if filter does not match the entry, mark it as read for (FeedSubscription sub : subscriptions) { FeedEntryFilter filter = new FeedEntryFilter(sub.getFilter()); - if (!filter.matchesEntry(entry)) { + boolean matches = true; + try { + matches = filter.matchesEntry(entry); + } catch (Exception e) { + log.error("could not evaluate filter {}", sub.getFilter(), e); + } + if (!matches) { FeedEntryStatus status = new FeedEntryStatus(sub.getUser(), sub, entry); status.setRead(true); feedEntryStatusDAO.saveOrUpdate(status); From eea0c24d2badb987f65987ab311d564b79a0fb69 Mon Sep 17 00:00:00 2001 From: Athou Date: Tue, 4 Nov 2014 16:25:34 +0100 Subject: [PATCH 6/8] engine is now strict and throws exceptions instead of returning nulls --- .../com/commafeed/backend/feed/FeedEntryFilterTest.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/test/java/com/commafeed/backend/feed/FeedEntryFilterTest.java b/src/test/java/com/commafeed/backend/feed/FeedEntryFilterTest.java index 679dc63e..256c1a4c 100644 --- a/src/test/java/com/commafeed/backend/feed/FeedEntryFilterTest.java +++ b/src/test/java/com/commafeed/backend/feed/FeedEntryFilterTest.java @@ -1,5 +1,6 @@ package com.commafeed.backend.feed; +import org.apache.commons.jexl2.JexlException; import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -43,16 +44,16 @@ public class FeedEntryFilterTest { Assert.assertTrue(filter.matchesEntry(entry)); } - @Test + @Test(expected = JexlException.class) public void newIsDisabled() { FeedEntryFilter filter = new FeedEntryFilter("null eq new ('java.lang.String', 'athou')"); - Assert.assertTrue(filter.matchesEntry(entry)); + filter.matchesEntry(entry); } - @Test + @Test(expected = JexlException.class) public void getClassMethodIsDisabled() { FeedEntryFilter filter = new FeedEntryFilter("null eq ''.getClass()"); - Assert.assertTrue(filter.matchesEntry(entry)); + filter.matchesEntry(entry); } @Test From a94ef980bbe03b265b0249a83fdb5c9eee4e8c32 Mon Sep 17 00:00:00 2001 From: Athou Date: Fri, 7 Nov 2014 08:46:09 +0100 Subject: [PATCH 7/8] cannot loop forever --- .../backend/feed/FeedEntryFilter.java | 40 +++++++++++++++++-- .../backend/service/FeedUpdateService.java | 3 +- .../commafeed/frontend/resource/FeedREST.java | 3 +- .../backend/feed/FeedEntryFilterTest.java | 24 ++++++----- 4 files changed, 55 insertions(+), 15 deletions(-) diff --git a/src/main/java/com/commafeed/backend/feed/FeedEntryFilter.java b/src/main/java/com/commafeed/backend/feed/FeedEntryFilter.java index 79ab69c8..f9ca6a5e 100644 --- a/src/main/java/com/commafeed/backend/feed/FeedEntryFilter.java +++ b/src/main/java/com/commafeed/backend/feed/FeedEntryFilter.java @@ -1,12 +1,20 @@ package com.commafeed.backend.feed; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + import lombok.RequiredArgsConstructor; -import org.apache.commons.jexl2.Expression; import org.apache.commons.jexl2.JexlContext; import org.apache.commons.jexl2.JexlEngine; +import org.apache.commons.jexl2.JexlException; import org.apache.commons.jexl2.JexlInfo; import org.apache.commons.jexl2.MapContext; +import org.apache.commons.jexl2.Script; import org.apache.commons.jexl2.introspection.JexlMethod; import org.apache.commons.jexl2.introspection.JexlPropertyGet; import org.apache.commons.jexl2.introspection.Uberspect; @@ -63,12 +71,17 @@ public class FeedEntryFilter { private final String filter; - public boolean matchesEntry(FeedEntry entry) { + public boolean matchesEntry(FeedEntry entry) throws FeedEntryFilterException { if (StringUtils.isBlank(filter)) { return true; } - Expression expression = ENGINE.createExpression(filter); + Script script = null; + try { + script = ENGINE.createScript(filter); + } catch (JexlException e) { + throw new FeedEntryFilterException("Exception while parsing expression " + filter, e); + } JexlContext context = new MapContext(); context.set("title", Jsoup.parse(entry.getContent().getTitle()).text().toLowerCase()); @@ -76,6 +89,25 @@ public class FeedEntryFilter { context.set("content", Jsoup.parse(entry.getContent().getContent()).text().toLowerCase()); context.set("url", entry.getUrl().toLowerCase()); - return (boolean) expression.evaluate(context); + Callable callable = script.callable(context); + Future future = Executors.newFixedThreadPool(1).submit(callable); + Object result = null; + try { + result = future.get(500, TimeUnit.MILLISECONDS); + } catch (InterruptedException e) { + throw new FeedEntryFilterException("interrupted while evaluating expression " + filter, e); + } catch (ExecutionException e) { + throw new FeedEntryFilterException("Exception while evaluating expression " + filter, e); + } catch (TimeoutException e) { + throw new FeedEntryFilterException("Took too long evaluating expression " + filter, e); + } + return (boolean) result; + } + + @SuppressWarnings("serial") + public static class FeedEntryFilterException extends Exception { + public FeedEntryFilterException(String message, Throwable t) { + super(message, t); + } } } diff --git a/src/main/java/com/commafeed/backend/service/FeedUpdateService.java b/src/main/java/com/commafeed/backend/service/FeedUpdateService.java index 033fc734..437268af 100644 --- a/src/main/java/com/commafeed/backend/service/FeedUpdateService.java +++ b/src/main/java/com/commafeed/backend/service/FeedUpdateService.java @@ -14,6 +14,7 @@ import org.apache.commons.codec.digest.DigestUtils; import com.commafeed.backend.dao.FeedEntryDAO; import com.commafeed.backend.dao.FeedEntryStatusDAO; import com.commafeed.backend.feed.FeedEntryFilter; +import com.commafeed.backend.feed.FeedEntryFilter.FeedEntryFilterException; import com.commafeed.backend.model.Feed; import com.commafeed.backend.model.FeedEntry; import com.commafeed.backend.model.FeedEntryContent; @@ -52,7 +53,7 @@ public class FeedUpdateService { boolean matches = true; try { matches = filter.matchesEntry(entry); - } catch (Exception e) { + } catch (FeedEntryFilterException e) { log.error("could not evaluate filter {}", sub.getFilter(), e); } if (!matches) { diff --git a/src/main/java/com/commafeed/frontend/resource/FeedREST.java b/src/main/java/com/commafeed/frontend/resource/FeedREST.java index b16b349a..b9382d39 100644 --- a/src/main/java/com/commafeed/frontend/resource/FeedREST.java +++ b/src/main/java/com/commafeed/frontend/resource/FeedREST.java @@ -45,6 +45,7 @@ import com.commafeed.backend.dao.FeedCategoryDAO; import com.commafeed.backend.dao.FeedEntryStatusDAO; import com.commafeed.backend.dao.FeedSubscriptionDAO; import com.commafeed.backend.feed.FeedEntryFilter; +import com.commafeed.backend.feed.FeedEntryFilter.FeedEntryFilterException; import com.commafeed.backend.feed.FeedFetcher; import com.commafeed.backend.feed.FeedQueues; import com.commafeed.backend.feed.FeedUtils; @@ -437,7 +438,7 @@ public class FeedREST { try { new FeedEntryFilter(req.getFilter()).matchesEntry(TEST_ENTRY); - } catch (Exception e) { + } catch (FeedEntryFilterException e) { Throwable root = Throwables.getRootCause(e); return Response.status(Status.BAD_REQUEST).entity(root.getMessage()).build(); } diff --git a/src/test/java/com/commafeed/backend/feed/FeedEntryFilterTest.java b/src/test/java/com/commafeed/backend/feed/FeedEntryFilterTest.java index 256c1a4c..869710b6 100644 --- a/src/test/java/com/commafeed/backend/feed/FeedEntryFilterTest.java +++ b/src/test/java/com/commafeed/backend/feed/FeedEntryFilterTest.java @@ -1,10 +1,10 @@ package com.commafeed.backend.feed; -import org.apache.commons.jexl2.JexlException; import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import com.commafeed.backend.feed.FeedEntryFilter.FeedEntryFilterException; import com.commafeed.backend.model.FeedEntry; import com.commafeed.backend.model.FeedEntryContent; @@ -27,39 +27,45 @@ public class FeedEntryFilterTest { } @Test - public void emptyFilterMatchesFilter() { + public void emptyFilterMatchesFilter() throws FeedEntryFilterException { FeedEntryFilter filter = new FeedEntryFilter(null); Assert.assertTrue(filter.matchesEntry(entry)); } @Test - public void blankFilterMatchesFilter() { + public void blankFilterMatchesFilter() throws FeedEntryFilterException { FeedEntryFilter filter = new FeedEntryFilter(""); Assert.assertTrue(filter.matchesEntry(entry)); } @Test - public void simpleExpression() { + public void simpleExpression() throws FeedEntryFilterException { FeedEntryFilter filter = new FeedEntryFilter("author eq 'athou'"); Assert.assertTrue(filter.matchesEntry(entry)); } - @Test(expected = JexlException.class) - public void newIsDisabled() { + @Test(expected = FeedEntryFilterException.class) + public void newIsDisabled() throws FeedEntryFilterException { FeedEntryFilter filter = new FeedEntryFilter("null eq new ('java.lang.String', 'athou')"); filter.matchesEntry(entry); } - @Test(expected = JexlException.class) - public void getClassMethodIsDisabled() { + @Test(expected = FeedEntryFilterException.class) + public void getClassMethodIsDisabled() throws FeedEntryFilterException { FeedEntryFilter filter = new FeedEntryFilter("null eq ''.getClass()"); filter.matchesEntry(entry); } @Test - public void dotClassIsDisabled() { + public void dotClassIsDisabled() throws FeedEntryFilterException { FeedEntryFilter filter = new FeedEntryFilter("null eq ''.class"); Assert.assertTrue(filter.matchesEntry(entry)); } + @Test(expected = FeedEntryFilterException.class) + public void cannotLoopForever() throws FeedEntryFilterException { + FeedEntryFilter filter = new FeedEntryFilter("while(true) {}"); + filter.matchesEntry(entry); + } + } From 281e0153769b5d7826c85be4b48b3a1416ce9a37 Mon Sep 17 00:00:00 2001 From: Athou Date: Mon, 10 Nov 2014 09:12:10 +0100 Subject: [PATCH 8/8] help block added --- src/main/app/i18n/en.js | 2 ++ src/main/app/sass/generic/_misc.scss | 4 ++++ src/main/app/templates/feeds.feed_details.html | 5 +++-- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/main/app/i18n/en.js b/src/main/app/i18n/en.js index f5d9e90d..f7d01143 100644 --- a/src/main/app/i18n/en.js +++ b/src/main/app/i18n/en.js @@ -98,6 +98,8 @@ "next_refresh" : "Next refresh", "queued_for_refresh" : "Queued for refresh", "feed_url" : "Feed URL", + "filtering_expression" : "Filtering expression", + "filtering_expression_help" : "If not empty, an expression evaluating to 'true' or 'false'. If false, new entries for this feed will be marked as read automatically.\nAvailable variables are 'title', 'content', 'url' and 'author' and their content is converted to lower case for convenience.\nExample: url.contains('youtube') or (author eq 'athou' and title.contains('github').\nComplete available syntax is available here.", "generate_api_key_first" : "Generate an API key in your profile first.", "unsubscribe" : "Unsubscribe", "unsubscribe_confirmation" : "Are you sure you want to unsubscribe from this feed?", diff --git a/src/main/app/sass/generic/_misc.scss b/src/main/app/sass/generic/_misc.scss index a60f5dbe..29fcd431 100644 --- a/src/main/app/sass/generic/_misc.scss +++ b/src/main/app/sass/generic/_misc.scss @@ -43,6 +43,10 @@ label { display: block; } +.pre-wrap { + white-space: pre-wrap; +} + .form-horizontal .control-group { margin-bottom: 10px; } diff --git a/src/main/app/templates/feeds.feed_details.html b/src/main/app/templates/feeds.feed_details.html index 3a173bd6..cee5d685 100644 --- a/src/main/app/templates/feeds.feed_details.html +++ b/src/main/app/templates/feeds.feed_details.html @@ -71,9 +71,10 @@
- -
+ +
+