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

Global middlewares are not always executed #2413

Closed
kpacha opened this issue Jun 15, 2020 · 3 comments
Closed

Global middlewares are not always executed #2413

kpacha opened this issue Jun 15, 2020 · 3 comments

Comments

@kpacha
Copy link

kpacha commented Jun 15, 2020

Description

The documentation for the Use function says

Use attaches a global middleware to the router. ie. the middleware attached though Use() will be included in the handlers chain for every single request. Even 404, 405, static files... For example, this is the right place for a logger or error management middleware.

But this is not accurate since the router can return a redirect without executing any of the declared middlewares. https://github.com/gin-gonic/gin/blob/master/gin.go#L406-L434

For some middlewares it is ok. But for features such as CORS, instrumentation, rate-limiting and others, it is not.

How to reproduce

package main

import (
	"log"
	"net/http"
	"time"

	"github.com/gin-gonic/gin"
)

func main() {
	router := gin.Default()

	router.RedirectTrailingSlash = true
	router.RedirectFixedPath = true
	router.HandleMethodNotAllowed = true

	router.Use(func(c *gin.Context) {
		t := time.Now()
		log.Print(c.Request.URL.String())

		c.Next()

		log.Print(time.Since(t))
		log.Println(c.Writer.Status())
	})

	router.GET("/foo/bar", func(context *gin.Context) {
		context.JSON(http.StatusOK, gin.H{"hello": "world"})
	})

	router.Run(":8080")
}

and then

$ curl http://localhost:8080/foo//bar/

<a href="/foo/bar">Moved Permanently</a>.

Expectations

I would expect 3 custom log lines in the stdout

[GIN-debug] [WARNING] Creating an Engine instance with the Logger and Recovery middleware already attached.

[GIN-debug] [WARNING] Running in "debug" mode. Switch to "release" mode in production.
 - using env:	export GIN_MODE=release
 - using code:	gin.SetMode(gin.ReleaseMode)

[GIN-debug] GET    /foo/bar                  --> main.main.func2 (4 handlers)
[GIN-debug] Listening and serving HTTP on :8080
2020/06/15 18:21:49 /foo//bar/
2020/06/15 18:21:49 275.831µs
2020/06/15 18:21:49 301
[GIN-debug] redirecting request 301: /foo//bar/ --> /foo//bar

Actual result

The logs are not present because the middleware is not executed

[GIN-debug] [WARNING] Creating an Engine instance with the Logger and Recovery middleware already attached.

[GIN-debug] [WARNING] Running in "debug" mode. Switch to "release" mode in production.
 - using env:	export GIN_MODE=release
 - using code:	gin.SetMode(gin.ReleaseMode)

[GIN-debug] GET    /foo/bar                  --> main.main.func2 (4 handlers)
[GIN-debug] Listening and serving HTTP on :8080
[GIN-debug] redirecting request 301: /foo//bar/ --> /foo//bar

Environment

  • go version: 1.14.3
  • gin version (or commit ref): 1.6.3
  • operating system: darwin
@linvis
Copy link
Contributor

linvis commented Jun 17, 2020

you enable redirecting, so server will response a new url to client, then the client will visit new url.

use curl -L http://localhost:8080/foo//bar/ to make curl follow the new url.
and curl -iL to see all http response

@kpacha
Copy link
Author

kpacha commented Jun 17, 2020

@shlinym , please read again the issue, because the 301 is ok and the client (cURL) not following it is also ok. I did not put the request/response on the Expectations nor the Actual Result section, because they don't matter for my point. The only thing that matters is that the global middleware is not executed in this scenario in opposition of what the documentation says

you enable redirecting, so server will response a new url to client, then the client will visit new url.

if the middleware was a CORS adapter, the browser won't follow such redirect due the lack of required CORS headers (since the middleware won't ever be executed for that request).

@linvis
Copy link
Contributor

linvis commented Jun 18, 2020

@shlinym , please read again the issue, because the 301 is ok and the client (cURL) not following it is also ok. I did not put the request/response on the Expectations nor the Actual Result section, because they don't matter for my point. The only thing that matters is that the global middleware is not executed in this scenario in opposition of what the documentation says

you enable redirecting, so server will response a new url to client, then the client will visit new url.

if the middleware was a CORS adapter, the browser won't follow such redirect due the lack of required CORS headers (since the middleware won't ever be executed for that request).

sorry for my misunderstanding,
your expectation looks like pre-middleware, before router finding,
and gin, for the present, call middleware after router finding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants