nilsbraden / ttrss-reader-fork

An Android-Client for the self-hosted Tiny Tiny RSS feedreader
https://www.nilsbraden.de/TTRSS-Reader/
151 stars 40 forks source link

http auth for embedded images #325

Closed pmaziere closed 7 years ago

pmaziere commented 8 years ago

Hello,

I configured my ttrss server to cache rss content images and it happens that this ttrss server requires http authentication access.

These images are displayed correctly on the web interface, but are replaced by a blue square with an interrogation point in ttrss-reader.

I can only guess that the reason could be that ttrss-reader fails to provide the necessary data for http auth when accessing these urls, despite the fact that these data are set and used to connect to the ttrss server.

If I'm right, could there be a check to compare these image urls and the ttrss server url so that when they are the same, http auth data provided to connect to the ttrss server are also used to access these images urls ?

thanks

mdewilde commented 7 years ago

If you are referring to images in articles, then I believe you actually have the opposite problem: HTTP authentication is set at a global level. However, article images are not fetched from your tt-rss instance, but rather from the original server hosting the RSS feed.

The app is then trying to authenticate with your credentials to that server, and of course that fails.

Pull request https://github.com/nilsbraden/ttrss-reader-fork/pull/327 fixes this problem by using HTTP authentication only for communication with your tt-rss instance.

pmaziere commented 7 years ago

I may be wrong, but I think you missed my point: the images are cached by the tt-rss server so that the original image URLs are replaced by the cached image URLs hosted on my tt-rss server. Moreover I do not use the single user mode.

mdewilde commented 7 years ago

@pmaziere I was not aware that tt-rss can cache images on your server. I'm curious to see if it also sends the cached image links in the article API.

As to this app, I do know that this app simple extracts any URL found in the actual article HTML content and downloads that.

Note that the possible fix I was referring to is https://github.com/nilsbraden/ttrss-reader-fork/pull/327/commits/436eae89982c25efe3c6bd8e2850752c6b17251a?diff=split#diff-7d9c284a795f6d4e17ae59db2c0badb7L245 and is not strictly the single user issue fix, but rather a byproduct of the fix.

mdewilde commented 7 years ago

@pmaziere I tested this and as I assumed, the API does NOT link to the image in your local web cache, but rather links the image from the original article.

When viewing the following article on my tt-rss instance website, the images are hosted in my site. Retrieved through the API, all images are on the original server:

{ "seq": 0, "status": 0, "content": [ { "id": "396618", "title": "US unlikely to meet targets set after Paris climate agreement", "link": "http://arstechnica.com/science/2016/10/us-unlikely-to-meet-targets-set-after-paris-climate-agreement/", "labels": [], "unread": true, "marked": false, "published": false, "comments": "", "author": "Roheeni Saxena", "updated": 1475783580, "content": "<div id=\"rss-wrap\">\n<figure class=\"intro-image intro-left\"><img src=\"https://cdn.arstechnica.net/wp-content/uploads/2014/04/7375813928_6326fd3323_b-640x615.jpg\"><p class=\"caption\" style=\"font-size:0.8em\"><a href=\"https://cdn.arstechnica.net/wp-content/uploads/2014/04/7375813928_6326fd3323_b.jpg\" class=\"enlarge-link\" data-height=\"985\" data-width=\"1024\">Enlarge</a> (credit: <a rel=\"nofollow\" class=\"caption-link\" href=\"https://www.flickr.com/photos/gsfc/7375813928/in/photolist-ceLYvE-9DTDXi-ffkwRW-binsBK-7179o-ch8suW-aqazvX-47Y5sq-jq8ouH-ch8t4W-8FnV7-6yPqsR-cCg8gy-e1EQPs-8FnV4-ch8rR9-cTb5j-ffkErQ-9GzoFH-aDRXNk-ftodF-ch8s6f-dALjCT-dALkbK-e389qT-8dguTW-9DTDX6-d6mHnY-dALk2z-afZPuJ-6aXAH3-HpcrT-bH2LpH-6jLwnX-8t96NE-29nkNT-cFb9Am-binyhX-dALjfZ-5fsbNS-emZ4Sd-ch8rFm-7Q6S9N-HugSV-9gtQh9-92BjFh-jzm1fa-ch8rrJ-6mDG1T-4VUDUs\">NASA/NOAA GOES Project</a>)</p> </figure><div><a name=\"page-1\"></a></div>\n<p>In 2015, representatives from 196 countries met for the Paris Climate talks, setting greenhouse gas emissions targets for the year 2025. As part of this agreement, the US has set what are called \"intended nationally determined contributions,\" which are planned reductions in carbon emissions. A recent paper in <em>Nature Climate Change</em> examined the current federal policies and determined that it is unlikely the US will meet its own targets as things now stand.</p>\n<p>The paper attempted a comprehensive evaluation of historical and projected greenhouse gas emissions in the US, and it put a particular emphasis on the most influential policy years. These were 2005 (the year the Kyoto Protocol went into effect and the Montreal action plan was developed) and 2025 (the date for reaching the targeted goals of the Paris climate talks). Researchers built a model that included historical and projected estimates of both climate data and energy use. The team then used the model to test the potential effects of several different pieces of climate policy that have been proposed or passed in recent years.</p>\n<p>They found that the <a href=\"https://www.epa.gov/cleanpowerplan/clean-power-plan-existing-power-plants\">EPA’s Clean Power Plan</a> would be the largest contributor to greenhouse gas emissions reductions, producing an estimated drop of 221 to 267 million tons of CO<sub>2</sub> equivalent. However, the team did find that an earlier, more ambitious version of the Clean Power Plan would have had an even larger effect.</p>\n</div><p><a href=\"http://arstechnica.com/science/2016/10/us-unlikely-to-meet-targets-set-after-paris-climate-agreement/#p3\">Read 5 remaining paragraphs</a> | <a href=\"http://arstechnica.com/science/2016/10/us-unlikely-to-meet-targets-set-after-paris-climate-agreement/?comments=1\">Comments</a></p><div class=\"feedflare\">\n<a href=\"http://feeds.arstechnica.com/~ff/arstechnica/index?a=4kstOt4iUus:UJ6PG3HytxI:V_sGLiPBpWU\"><img src=\"http://feeds.feedburner.com/~ff/arstechnica/index?i=4kstOt4iUus:UJ6PG3HytxI:V_sGLiPBpWU\" border=\"0\"></img></a> <a href=\"http://feeds.arstechnica.com/~ff/arstechnica/index?a=4kstOt4iUus:UJ6PG3HytxI:F7zBnMyn0Lo\"><img src=\"http://feeds.feedburner.com/~ff/arstechnica/index?i=4kstOt4iUus:UJ6PG3HytxI:F7zBnMyn0Lo\" border=\"0\"></img></a> <a href=\"http://feeds.arstechnica.com/~ff/arstechnica/index?a=4kstOt4iUus:UJ6PG3HytxI:qj6IDK7rITs\"><img src=\"http://feeds.feedburner.com/~ff/arstechnica/index?d=qj6IDK7rITs\" border=\"0\"></img></a> <a href=\"http://feeds.arstechnica.com/~ff/arstechnica/index?a=4kstOt4iUus:UJ6PG3HytxI:yIl2AUoC8zA\"><img src=\"http://feeds.feedburner.com/~ff/arstechnica/index?d=yIl2AUoC8zA\" border=\"0\"></img></a>\n</div>", "feed_id": "7", "attachments": [], "score": 0, "feed_title": "Ars Technica", "note": null, "lang": "en" } ] }

That having been said, it still does seem that the fix above will also fix this issue.

pmaziere commented 7 years ago

It did not occur to me that the API would send something different than what is presented in the web interface. The point of caching the images is to avoid to download them multiple times, both for privacy issues and bandwidth saving.

Considering your fix, am I right if I say that it does not fix the API content issue, but only the HTTP authentication global setting issue ? As a side effect, it happens to fix my issue too, but the real fix should be made in tt-rss itself, since web interface and content sent by the API should be the same. I guess this should be reported as a bug of tt-rss.

What do you think ?

Anyway, thanks for your PR side effect ;)

pmaziere commented 7 years ago

While 1.90.1 is working fine, I build and installed 436eae89982c25efe3c6bd8e2850752c6b17251a, but it seems that @mdewilde PR makes ttrss-reader crash on with my Replicant 4.2 0004 Samsung S (I9000). Same goes for the current HEAD.

So I can't test it.

But I have to say that other images than the ones cached by the tt-rss server are shown correctly in ttrss-reader. Therefore, except if I missed your point, I don't think my issue can be solved by your above PR.

Futhermore, I confirm that my web server receives a request without basic HTTP auth data when I check items containing cached images with ttrss-reader-fork. Therefore, in opposition to what @mdewilde stated above, tt-rrs server sends correctly the cached image URL instead of the URL to the original site.

pmaziere commented 7 years ago

Now that the crash is fixed, I tested 1.91 which includes @mdewilde patch and images cached by my http auth enabled tt-rss server are still not displayed by ttrss-reader-fork

nilsbraden commented 7 years ago

If you want to check what exactly the app is accessing you can debug into this line: https://github.com/nilsbraden/ttrss-reader-fork/blob/master/ttrssreader/src/main/java/org/ttrssreader/gui/fragments/ArticleFragment.java#L435 -> There is the last place where content of the webview is manipulated.

It might be that at this place the javascript in the header is called and replaces the url with a local reference to the image cache but in this case you shouldn't see a call to your web server of course. In this case the array "cachedImages" contains the right URL to the local files.

Next step is to check what happens in ArticleWebViewClient.shouldInterceptRequest(...): Can you check if this is called and what is returned? It should check if the image-url points to your ttrss-instance and provide a connection with correct HTTP-Auth data.

If you cannot check these things you can also send me an email with credentials for a test account on your server.

pmaziere commented 7 years ago

There is definitely a call to my webserver.

ArticleWebViewClient.shouldInterceptRequest is called, but it seems there is a problem with android.webkit.WebResourceRequest.getUrl

I/ActivityManager( 1783): START u0 {cmp=org.ttrssreader/.gui.FeedHeadlineActivity (has extras)} from pid 6646
I/ActivityManager( 1783): Displayed org.ttrssreader/.gui.FeedHeadlineActivity: +407ms (total +2s209ms)
E/BufferQueue(  780): [org.ttrssreader/org.ttrssreader.gui.CategoryActivity] drainQueueLocked: BufferQueue has been abandoned!
I/dalvikvm( 6646): Could not find method android.webkit.WebResourceRequest.getUrl, referenced from method org.ttrssreader.gui.view.ArticleWebViewClient.shouldInterceptRequest
I/dalvikvm( 6646): Could not find method android.webkit.WebResourceRequest.getUrl, referenced from method org.ttrssreader.gui.view.ArticleWebViewClient.shouldInterceptRequest

could that be it ?

nilsbraden commented 7 years ago

Guess the "could not find method" part was the missing key, please try this build: https://github.com/nilsbraden/ttrss-reader-fork/releases/tag/Test_Issue_%23325

pmaziere commented 7 years ago

it still prints the same "could not find method" message

nilsbraden commented 7 years ago

Well the getUrl-Part is just an information log message so we can ignore this because the method only is called on devices with api level >= 21. As far as I can see the system Replicant 4.2 is a jelly bean rom with api level 16.

Please try to retrieve the HTML code which is set in ArticleFragment.java Line 435 and send it to me by mail (ttrss@nilsbraden.de).

pmaziere commented 7 years ago

my bad ! I must have done something wrong 1 hour ago since after adding a line to dump the html code and recompiling, it worked as expected. The HTML code use the right url and the image is displayed correctly.

again thanks for your work.