gobuffalo / buffalo

Rapid Web Development w/ Go
http://gobuffalo.io
MIT License
8.08k stars 579 forks source link

bug: buffalo.Options.Prefix does not work #2356

Closed qdongxu closed 1 year ago

qdongxu commented 1 year ago

Description

Description

Buffalo version is: v0.18.12

I want the buffalo server as a backend of nginx, and follow the instruction from #2022 , adding a Options.Prefix: '/prefix', then all request returns 404 no matter a '/prefix' is prepended in the request or not.

I found this useful guideline to configure with the nginx https://gobuffalo.io/documentation/deploy/proxy/ , but the rule inside my organization requires a prefix route config.

Expected Behavior

Expected to configure a reverse proxy route with prefix .

Actual Behavior

web requests are broken after the prefix is configured. API requests should append a '/' char at the end of the path.

To Reproduce

  1. Add a prefix in actions/App.go
func App() *buffalo.App {
    appOnce.Do(func() {
        app = buffalo.New(buffalo.Options{
            Env:         ENV,
            SessionName: "_tmon_session",
            Prefix:      "/prefix/",
        })
}
  1. run buffalo routes to verify:

before adding prefix:

  % buffalo routes
METHOD | HOST                  | PATH                    | ALIASES | NAME                   | HANDLER
------ | ----                  | ----                    | ------- | ----                   | -------
GET    | http://127.0.0.1:3000 | /                       |         | rootPath               | proj/actions.HomeHandler
POST   | http://127.0.0.1:3000 | /api/v1/user/users/     |         | apiV1UserUsersPath     | proj/actions/user.CreateUserHandler

after adding the prefix:

% buffalo routes
METHOD | HOST                  | PATH                           | ALIASES | NAME                         | HANDLER
------ | ----                  | ----                           | ------- | ----                         | -------
GET    | http://127.0.0.1:3000 | /prefix/                       |         | prefixPath                   | proj/actions.HomeHandler
POST   | http://127.0.0.1:3000 | /prefix/api/v1/user/users/     |         | prefixAPIV1UserUsersPath     | proj/actions/user.CreateUserHandler
  1. check the welcome page, the returned links do not contain the '/prefix':
    
    % curl -L  http://127.0.0.1:3000/prefix
    <pre>
    <a href="assets/">assets/</a>
    <a href="robots.txt">robots.txt</a>
    </pre>

but http://127.0.0.1:3000/prefix/ , with an ending '/' , returns 404.


3.  http://127.0.0.1:3000/prefix/assets/ redirected with 301 code  to http://127.0.0.1:3000/prefix/assets/assests then 404 code.

4. http://127.0.0.1:3000/prefix/robots.txt works. 

6. The API works only postpended with '/'.  

This causes a 404 code :

curl -XPOST -L http://127.0.0.1:3000/prefix/api/v1/user/users


this works :

curl -XPOST -L http://127.0.0.1:3000/prefix/api/v1/user/users/


### Additional Context

<!-- Additional Context Here -->

Details

<details>

Paste the output of buffalo info here!



</details>
sio4 commented 1 year ago

Thank you @qdongxu for raising this issue. I am not fully sure if I understood your issue correctly, but I would like to leave comments on what I understood.

I want the buffalo server as a backend of nginx, and follow the instruction from #2022 , adding a Options.Prefix: '/prefix', then all request returns 404 no matter a '/prefix' is prepended in the request or not.

So it means the issue is not related to the Prefix. Is my understanding correct? With the other descriptions below, I don't think this is an issue with the prefix but an issue with the suffixed/trailing slash.

Actual Behavior

web requests are broken after the prefix is configured. API requests should append a '/' char at the end of the path.

Could you please test it with bare configuration without the Prefix? I don't think "after the prefix is configured" is not a part of the issue. If my understanding is correct, this issue is related to the following issues and PRs:

[1] #1168 [2] #2013

The only interesting part of this report is the following. I need to try to understand the below more correctly later.

  1. check the welcome page, the returned links do not contain the '/prefix':
% curl -L  http://127.0.0.1:3000/prefix
<pre>
<a href="assets/">assets/</a>
<a href="robots.txt">robots.txt</a>
</pre>

# but http://127.0.0.1:3000/prefix/ , with an ending '/' , returns 404.

I guess this part is working as static file serving which is not related to the named routes but I need to check more in details. Could you please provide your full actions/app.go file?

  1. The API works only postpended with '/'.

Yes, this behavior was introduced by #1168.

qdongxu commented 1 year ago

@sio4 Sorry for being late. I created a demo project and reproduce the problem exactly.

Here's the actions/app.go file:

func App() *buffalo.App {
    appOnce.Do(func() {
        app = buffalo.New(buffalo.Options{
            Env:         ENV,
            SessionName: "_prefix_test_session",
            Prefix:      "/prefix", 
        })

        // Automatically redirect to SSL
        app.Use(forceSSL())

        // Log request parameters (filters apply).
        app.Use(paramlogger.ParameterLogger)

        // Protect against CSRF attacks. https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)
        // Remove to disable this.
        app.Use(csrf.New)

        // Setup and use translations:
        app.Use(translations())

        app.GET("/", HomeHandler)

        // Add API for about info
        app.GET("/about/", About)

        app.ServeFiles("/", http.FS(public.FS())) // serve files from the public directory
    })

    return app
}
sio4 commented 1 year ago

Hi @qdongxu

Could you please confirm if the point of this issue is a trailing slash or prefix? Do you see any issue with using the prefix itself regardless of the trailing slash issue?

qdongxu commented 1 year ago

The point is prefix. It's certainly ok if the rule requires a trailing slash. but the prefix is a blocking issue.

It returns below after adding a prefix, instead of the page index.plush.html rendered by actions.HomeHandler.

% curl -L  http://127.0.0.1:3000/prefix
<pre>
<a href="assets/">assets/</a>
<a href="robots.txt">robots.txt</a>
</pre>
qdongxu commented 1 year ago

The issue described excessive observed phenomenons. Let's simplify it according to @sio4 's clarification:

  1. A request URL should end with a trailing slash '/' for route lookup.
  2. The named route did not handle the root path with a prefix properly. PR #2367 addresses this issue.
sio4 commented 1 year ago

Ah, now I think I understand what you mean (for the second point regarding the prefix support). You mean the generated route name for the root of the prefixed app is currently prefixPath but you expected it should be rootPath as the same as the app without a prefix. Is my understanding correct?

Please let me know if I understand correctly.

Actually, this is not simple since we support a prefix app, a multi-homed app, groups, and resources which are basically a grouped route. Could you please correct me if I still misunderstood your issue?

qdongxu commented 1 year ago

You mean the generated route name for the root of the prefixed app is currently prefixPath but you expected it should be rootPath as the same as the app without a prefix. Is my understanding correct?

Yes, Exactly!

In PR #2367 I have updated the code and verified with rootPath, VirtualHost Path, Group paths, and Resource paths.

for VirtualHost, there may be identical paths under different VHost. and for Resource, there are identical paths with different HTTP methods, but the route name depends on the path only. The naming rule should be updated. This is not caused by introducing prefix, which should be considered in a separate issue.

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment. Otherwise, this will be closed in 7 days.

github-actions[bot] commented 1 year ago

This issue was closed because it has been stalled for 30+7 days with no activity.