Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove $feedurl from RSS links #72

Closed
wants to merge 1 commit into from
Closed

Remove $feedurl from RSS links #72

wants to merge 1 commit into from

Conversation

ahmet2mir
Copy link

Other links haven't $feedurl var. Behind a reverse proxy with a path, rss point to server/?do=rss instead of server/path/?do=rss

…nt to server/?do=rss instead of server/path/?do=rss
@nodiscc
Copy link
Member

nodiscc commented Dec 9, 2014

Hi @ahmet2mir thanks for the pull request. I'd like to make sure we are not duplicating efforts, there is a proposed solution for reverse proxies (this bug is tracked in #17), and it has been implemented in a temporary, experimental branch skip-port-detection in https://github.com/nodiscc/Shaarli (this commit nodiscc@5c3605f).

I don't have a reverse proxy at hand to test this or your solution. I've asked for testers in sebsauvage#101, but got no replies.

So, bugs currently affecting reverse proxies are:

So far, I think your Pull Request ony addresses the last point, right?

As you have a reverse proxy, can you test the mentioned patch? (We'd rather fix all reverse proxy issues with a single solution, it would be better than hacking every separate bug)

Thanks again

@nodiscc
Copy link
Member

nodiscc commented Dec 9, 2014

Does anyone else have a Shaarli on a non-standard port behind an apache proxy? What bugs are you able to reproduce? Does the skip-port-detection patch or the one proposed here fix any of your issues?

@ahmet2mir
Copy link
Author

If you have docker, you can test this https://github.com/ahmet2mir/dockerfiles/tree/master/reverseproxy in the full example I run a shaarli.
It's not a port detection issue (both on 80), it's due do "indexUrl()" who use $_SERVER["SCRIPT_NAME"].
The docker shaarli is on http://private/ and my reverse http://public/shaarli/. So indexurl take the correct host but wrong path.
There is nothing on $SERVER side to obtains the correct path :/

@nodiscc
Copy link
Member

nodiscc commented Dec 10, 2014

Ok there were some updates in sebsauvage#101. As you said the 2 bugs are unrelated. I'll test your patch to see if it doesn't unexpectedly break things, please allow some time for the review. Feedback from other users/maintainers is also welcome.

Thanks again

@nodiscc nodiscc added the bug it's broken! label Dec 10, 2014
@ahmet2mir
Copy link
Author

I think that we need to remove totally indexUrl.

@nodiscc nodiscc added in progress and removed bug it's broken! labels Dec 16, 2014
@nodiscc nodiscc added the bug it's broken! label Jan 9, 2015
@nodiscc nodiscc added this to the 0.9beta milestone Jan 9, 2015
@nodiscc
Copy link
Member

nodiscc commented Jan 26, 2015

@ahmet2mir I've just merged #98; Shaarli now uses php's SERVER_NAME instead of HTTP_HOST. Does this fix the rss bug on your reverse proxy setup? Please let me know.

We can investigate removing indexUrl later

@nodiscc
Copy link
Member

nodiscc commented Feb 7, 2015

Closing this. @ahmet2mir please test against the latest master and feel free to reopen a PR/bug if this doesn't fix your problem.

@nodiscc nodiscc closed this Feb 7, 2015
@virtualtam virtualtam modified the milestones: 0.9.0, 0.5.0 Jul 30, 2015
@virtualtam virtualtam added the proxy hosting behind reverse proxies label Aug 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug it's broken! in progress proxy hosting behind reverse proxies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants