jcerdenia / NiceFeed

Android RSS feed reader and news aggregator
MIT License
94 stars 14 forks source link

Feed URLs are converted to https, which breaks feeds that are only provided as http #1

Closed kitlith closed 4 years ago

kitlith commented 4 years ago

https://github.com/joshuacerdenia/NiceFeed/blob/5cf0ab6413f3596e4a447ad380fcf28cd67a0b40/app/src/main/java/com/joshuacerdenia/android/nicefeed/ui/fragment/AddFeedsFragment.kt#L110

I generate RSS feeds on-device with an HTTP server with no TLS support, because it's difficult to setup certificates for localhost, which is why I instantly encountered this.

This also breaks the theoretical case of a rss feed being provided over, say, ftp, or any other protocol.

jcerdenia commented 4 years ago

I appreciate the comment, I'm going to take a look at this and the other one. Please feel free to let me know if you see other issues.

kitlith commented 4 years ago

I'll do that.

jcerdenia commented 4 years ago

Just fixed this by automatically trying http:// when https:// fails, like so:

urlEditText.text.toString()
                .substringAfter("://")
                .toLowerCase(Locale.ROOT)
                .trim()
                .run {
                    viewModel.requestFeed(HTTPS + this, HTTP + this) // Second url is tried when first one fails
                }
...
private const val HTTPS = "https://"
private const val HTTP = "http://"

This is only when entering URLs manually. URLs obtained from search are still always converted to https:// -- I may have to look into this a little deeper, but in my own usage it results in too many failed requests otherwise.

kitlith commented 4 years ago

Why not just... use the scheme provided by the user by default, and fall back to https or http if it isn't provided, if you're going to perform any kind of fallback?

jcerdenia commented 4 years ago

I figured most users would skip entering the scheme, and the code is simpler. But this would give the option of entering a URL without the program altering the provided scheme:

            val url = urlEditText.text.toString().toLowerCase(Locale.ROOT).trim()
            if (url.contains("://")) {
                viewModel.requestFeed(url)
            } else viewModel.requestFeed("https://$url", "http://$url")
kitlith commented 4 years ago

I figure that most users are going to copy their feed urls from somewhere else, with the scheme already included. I admit that I am an outlier in that I end up manually typing most of my feeds, but that's because they come from a feed generator i wrote that is locally hosted. (i could probably also solve that with a small userscript or something, but that's unrelated)

That snippet looks fine for the purpose.