ether / etherpad-lite

Etherpad: A modern really-real-time collaborative document editor.
http://docs.etherpad.org/
Apache License 2.0
16.48k stars 2.84k forks source link

Hardcoded path /p breaks pad exports #1947

Closed arieljlira closed 9 years ago

arieljlira commented 10 years ago

Hi, I have an etherpad-lite instance behind a reverse proxy with an alternative path to my pads, "/pads" instead of "/p". All work fine, except the export functionality. Export routes are "/null/export/$format" and the worst is that it works! because it always returns the pad named "null" ...

How to reproduce it? reverse proxy an instance with a route other than /p/ . Like

        RewriteRule ^/pads/(.*)$ http://localhost:9001/p/$1 [P]
        RewriteCond %{REQUEST_FILENAME} !-f
        RewriteRule ^/(.*)$ http://localhost:9001/$1 [P]

I think the problem is in https://github.com/ether/etherpad-lite/blob/develop/src/static/js/pad_impexp.js#L210 Replacing RegExp

var pad_root_path = new RegExp(/.*\/p\/[^\/]+/).exec(document.location.pathname);

with

var pad_root_path = new RegExp(/.*\/[^\/\?]+/).exec(document.location.pathname);

seems to fix the problem. I've tested in and with this patch, export routes works as expected with /p, /pads, /some/other/route and /some/other/route?with=params

I saw a similar report in https://bugs.launchpad.net/openstack-ci/+bug/1168455

Thanks!

JohnMcLear commented 10 years ago

Your fix will hurt instances that run on /p/

I think the best fix for this is prolly to use whatever function is getting the padId and use that to write the export url

Afiak i had a plugin that fixed this issue for a specific site a while back, your temp fix wont hurt you now but obviously if you pull you will have merge conflicts on that file so it will hurt you later.

arieljlira commented 10 years ago

Hi John, I don't know you say it will hurt instances that run on /p/ I have the same ep instance behind another proxy, with /p, and it looks fine. It also works with no proxy at all. I can send you both urls if you find it useful.

JohnMcLear commented 10 years ago

yea but anything else will break, for example if your etherpad was on /hosted/etherpad it wouldn't work

arieljlira commented 10 years ago

I've tried several routes with 2 or more levels and worked ok. For example In apache2 with mod_proxy+mod_proxy_html:

        RewriteRule ^/hosted/etherpad/pads/(.*)$ http://localhost:9001/p/$1 [P,L]
        RewriteRule ^/hosted/etherpad/(.*)$ http://localhost:9001/$1 [P]

Perhaps I'm missing something else, however regexp /.*\/[^\/\?]+/ always seems to get the correct request-uri for any uri.

I guess it would be better to a unique function to get the padId and use relative paths. But I can't help you with that because I know almost nothing about epl nor nodejs. Regards

JohnMcLear commented 10 years ago

Try hitup /frontend/tests when hosting EP on different endpoints with and without proxy see if anything explodes, if not send through a PR with your patch

JohnMcLear commented 9 years ago

I just test this using this apache config and export is working fine..

    <VirtualHost etherpadtest:80>
        ServerAdmin support@example.org
        ServerName etherpadtest
        ServerSignature Off

      Redirect /pad /pad/

    LoadModule  proxy_module         /usr/lib/apache2/modules/mod_proxy.so
    LoadModule  proxy_http_module    /usr/lib/apache2/modules/mod_proxy_http.so
    LoadModule  headers_module       /usr/lib/apache2/modules/mod_headers.so
    LoadModule  deflate_module       /usr/lib/apache2/modules/mod_deflate.so

    ProxyVia On
    ProxyRequests Off
    ProxyPass /pad/ http://localhost:9001/
    ProxyPassReverse /pad/ http://localhost:9001/
    ProxyPreserveHost on
    <Proxy *>
        Options FollowSymLinks MultiViews
        AllowOverride All
        Order allow,deny
        allow from all
    </Proxy>
    </VirtualHost>

Just don't rewrite the /p/ to something else and you will be fine..

Someone might want to go ahead and update the docs on this.

Yaco commented 9 years ago

I am using Nginx with this config file: https://github.com/ether/etherpad-lite/wiki/How-to-put-Etherpad-Lite-behind-a-reverse-Proxy#hosted-at--with-rewrite-rules-to-allow-padname

Export is broken if I access without "/p" in the URL. Any fix?

JohnMcLear commented 9 years ago

Have you looked at your nginx config?

JohnMcLear commented 9 years ago

See https://github.com/MozillaFoundation/mofo-lightsaber/issues/12#issuecomment-72710349 & https://github.com/MozillaFoundation/mofo-lightsaber/issues/11#issuecomment-73773049

Looks like adding something like rewrite ^/(.*)/export/(.*) /p/$1/export/$2 break; will fix this. Please test and let me know @Yaco

Also please fix your own reverse proxy configs, it's really not up to the foundation to maintain third party software for you!

JohnMcLear commented 9 years ago

I put a fix in for export URLS, please pull latest develop and test.