looks_like_feed? is slow on some urls about feedbag HOT 5 CLOSED

Yeah, this seems to be a nasty bug that the URL is uncovering because of the + sign on the URL.

This diff may fix it:

diff --git a/lib/feedbag.rb b/lib/feedbag.rb
index b145fa3..6df550d 100644
--- a/lib/feedbag.rb
+++ b/lib/feedbag.rb
@@ -167,7 +167,7 @@ class Feedbag
   end

   def looks_like_feed?(url)
-    if url =~ /(\.(rdf|xml|rss)(\?([\w'\-%]?(=[\w'\-%.]*)?(&|#)?)+)?(:[\w'\-%]+)?$|feed=(rss|atom)|(atom|feed)\/?$)/i
+    if url =~ /(\.(rdf|xml|rss)(\?([\w'\-%]?(=[\w'\-%.]*)?(&|#|\+)?)+)?(:[\w'\-%]+)?$|feed=(rss|atom)|(atom|feed)\/?$)/i
       true
     else
       false

Any way you can give this a shot?

from feedbag.

Thanks @damog for quick answer! Unfortunately, it doesn't help:

url = 'https://link.springer.com/search.rss?facet-content-type=Article&facet-journal-id=41116&channel-name=Living+Reviews+in+Solar+Physics'
url =~ /(\.(rdf|xml|rss)(\?([\w'\-%]?(=[\w'\-%.]*)?(&|#|\+)?)+)?(:[\w'\-%]+)?$|feed=(rss|atom)|(atom|feed)\/?$)/i

is still slow (waited for more than 20 seconds then cancelled).

I wonder if you can check it without regexp.

E.g., by parsing url and checking if path contains rdf/xml/rss and or other conditions.

from feedbag.

You seem to be manually passing the URL to the regex. This may or may not work (it surely doesn't from your example :)). But you may be skipping the parsing that Nokogiri makes on the HTML elements of the page. I suggest you try using the library directly. With some debug statements, the provided regex work with the following URLs:

irb(main):003:0> url = 'https://link.springer.com/search.rss?facet-content-type=Article&facet-journal-id=41116&channel-name=Living+Reviews+in+Solar+Physics'
=> "https://link.springer.com/search.rss?facet-content-type=Article&facet-journal-id=41116&channel-name=Living+Reviews+in+Solar+Physics"
irb(main):004:0> url =~ /(\.(rdf|xml|rss)(\?([\w'\-%]?(=[\w'\-%.]*)?(&|#|\+)?)+)?(:[\w'\-%]+)?$|feed=(rss|atom)|(atom|feed)\/?$)/i
=> 32
irb(main):005:0> url = 'http://link.springer.com/search.rss?facet-content-type=Article&facet-journal-id=11207&channel-name=Solar%20Physics'
=> "http://link.springer.com/search.rss?facet-content-type=Article&facet-journal-id=11207&channel-name=Solar%20Physics"
irb(main):006:0> url =~ /(\.(rdf|xml|rss)(\?([\w'\-%]?(=[\w'\-%.]*)?(&|#|\+)?)+)?(:[\w'\-%]+)?$|feed=(rss|atom)|(atom|feed)\/?$)/i
=> 31
irb(main):007:0>

The next culprit seems to be the interpretation of & → & so adding the ; to the regex may also work. You can test this manually too:

 url =~ /(\.(rdf|xml|rss)(\?([\w'\-%]?(=[\w'\-%.]*)?(&|#|\+|\;)?)+)?(:[\w'\-%]+)?$|feed=(rss|atom)|(atom|feed)\/?$)/i

I'm not committing changes just yet as these quick fixes will need further deep dive.

Let me know how it works.
dm

from feedbag.

Thanks @damog! Let me mention that the url that I used directly ( https://link.springer.com/search.rss?facet-content-type=Article&facet-journal-id=41116&channel-name=Living+Reviews+in+Solar+Physics ) was taken from debugging the feedbag, so it was after Nokogiri parsing. But I'll try to do as you suggested anyway to see if it helps in other places and get back to you.

In general, I'd rather refrain from using complex regexp in favor of clear simple checks if possible, since it'll also make the code more readable and understandable and easier to extend. Thinks like url.path.match(/\.(rdf|xml|rss)$/) are clearly easier to debug in case of problems and even if the code becomes slower, I think that http request of fetching the feed takes much more than any set of such checks. Also, if we already know that such Regexp is slow in some cases, then at some moment one will definitely face such case even if some of the precautions are made, because we can't be sure that all precautions were made.

WDYT?

from feedbag.

Sure, happy to receive your patch and merge it whenever you have it ready.

from feedbag.