oetiker / mojolicious-plugin-reverseproxy

A Reverse Proxy Helper for Mojolicious
5 stars 5 forks source link

Commit 8657014 has broken path handling with no mount points. #6

Open tobez opened 5 years ago

tobez commented 5 years ago

Hello,

After the commit in question, the URL path is first set to the destination URL path. Then it is set to the request path, but only if mount point is non-empty, which results in insanity like this:

[2019-03-15 14:30:08.05202] [30650] [debug] GET "/app.js" (98378be2)
Mojolicious::Plugin::ReverseProxy request /app.js to http://localhost:3033/
[2019-03-15 14:30:08.75624] [30650] [debug] GET "/static/favicon-192x192.png" (1c66f132)
Mojolicious::Plugin::ReverseProxy request /static/favicon-192x192.png to http://localhost:3033/

These logs are produced from the following code:

    app->plugin('Mojolicious::Plugin::ReverseProxy', {
        destination_url => 'http://localhost:3033',
        req_processor => sub {
            my ($c,$req) = @_;
            print STDERR "Mojolicious::Plugin::ReverseProxy request " . $c->req->url . " to " . $req->url . "\n";
        },
    });

I am not entirely sure how the proper fix would look. I'd probably just remove the line

$url->path($dest_url->path);

as obviously non-sensical.

oetiker commented 5 years ago

wouldn't in your case the mount point be / ?

tobez commented 5 years ago

One would expect this, yes, if not for

my $mount_point = $conf->{mount_point} || '';
$mount_point =~ s{/$}{};

in the caller.

On 15 Mar 2019, at 15:06, Tobias Oetiker notifications@github.com wrote:

wouldn't in your case the mount point be / ?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/oetiker/mojolicious-plugin-reverseproxy/issues/6#issuecomment-473298902, or mute the thread https://github.com/notifications/unsubscribe-auth/AAD4v1s8cKMgdecZ27TU5BTIpqCrJ5m2ks5vW6jagaJpZM4b2fWH.

tobez commented 3 years ago

Ping on this one. The bug is still there, and I am still unsure of the "proper" fix. In the meantime, my workaround is to specify

    mount_point => '//'

Then the last slash gets stripped, and the mount point really becomes /, and all is dandy.