Skip to content
This repository has been archived by the owner on Feb 24, 2024. It is now read-only.

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

Closed
qdongxu opened this issue Dec 26, 2022 · 9 comments
Closed

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

qdongxu opened this issue Dec 26, 2022 · 9 comments
Assignees
Labels
s: closed s: triage Some tests need to be run to confirm the issue stale

Comments

@qdongxu
Copy link

qdongxu commented Dec 26, 2022

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.
  1. 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.

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

  3. 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

Details

Paste the output of `buffalo info` here!
@sio4 sio4 added the s: triage Some tests need to be run to confirm the issue label Jan 6, 2023
@sio4
Copy link
Member

sio4 commented Jan 22, 2023

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.

@sio4 sio4 self-assigned this Jan 22, 2023
@qdongxu
Copy link
Author

qdongxu commented Feb 17, 2023

@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
Copy link
Member

sio4 commented Feb 18, 2023

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
Copy link
Author

qdongxu commented Feb 19, 2023

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>

This was referenced Feb 19, 2023
@qdongxu
Copy link
Author

qdongxu commented Feb 19, 2023

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 Fix URL prefix #2367 addresses this issue.

@sio4
Copy link
Member

sio4 commented Feb 23, 2023

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
Copy link
Author

qdongxu commented Feb 28, 2023

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
Copy link

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 github-actions bot added the stale label Mar 31, 2023
@github-actions
Copy link

github-actions bot commented Apr 8, 2023

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

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
s: closed s: triage Some tests need to be run to confirm the issue stale
Projects
None yet
Development

No branches or pull requests

2 participants