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

wrong response after returning error at middleware #3718

Closed
mkruczek opened this issue Sep 7, 2023 · 3 comments
Closed

wrong response after returning error at middleware #3718

mkruczek opened this issue Sep 7, 2023 · 3 comments

Comments

@mkruczek
Copy link

mkruczek commented Sep 7, 2023

Description

i have my own middelware which takes data from queryParama and adds it to the context (in my example it's name)
if name is not given, an error is returned and the request is not further processed with a response code of 400.
The problem is that if I call a non-existent URL, e.g. /xxxhello , without the name parameter, I also get a 400, and I would expect a 404.
I can get a 404 if I add the ?name=value parameter, but that doesn't make much sense

How to reproduce

package main

import (
	"context"
	"errors"
	"github.com/gin-gonic/gin"
	"log"
	"net/http"
)

func main() {
	g := gin.Default()
	g.Use(myMiddleware())
	g.GET("/hello", func(c *gin.Context) {
		c.String(200, "Hello %s", c.Request.Context().Value("name"))
	})
	g.Run(":9000")
}

func myMiddleware() gin.HandlerFunc {
	return func(c *gin.Context) {
		req := c.Request
		nameParam := req.URL.Query().Get("name")
		if nameParam == "" {
			log.Println("param is empty")
			c.AbortWithError(http.StatusBadRequest, errors.New("param is empty"))
			return
		}
		req = req.WithContext(context.WithValue(req.Context(), "name", nameParam))
		c.Request = req
		c.Next()
	}
}

Expectations

$ curl http://localhost:9000/xxxhello
404 - not found

Actual result

$ curl -i http://localhost:9000/xxxhello
400 - bad request

Environment

  • go version: 1.21.0
  • gin version (or commit ref): v1.9.0
  • operating system: Linux/Windows
@AuroraTea
Copy link

if !c.Writer.Written() {
    // TODO: 404
}

@VarusHsu
Copy link

VarusHsu commented Sep 11, 2023

You should group your router at first.
Let the middleware only affect a group instead of the global location.

package main

import (
	"context"
	"errors"
	"github.com/gin-gonic/gin"
	"log"
	"net/http"
)

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

	group := g.Group("")
	group.Use(myMiddleware())
	{
		group.GET("/hello1", func(c *gin.Context) {
			c.String(200, "Hello1 %s", c.Request.Context().Value("name"))
		})
		group.GET("/hello2", func(c *gin.Context) {
			c.String(200, "Hello2 %s", c.Request.Context().Value("name"))
		})
		group.GET("/hello3", func(c *gin.Context) {
			c.String(200, "Hello3 %s", c.Request.Context().Value("name"))
		})
	}
	g.GET("/hello", func(c *gin.Context) {
		c.String(200, "Hello %s", c.Request.Context().Value("name"))
	})
	g.Run(":9000")
}

func myMiddleware() gin.HandlerFunc {
	return func(c *gin.Context) {
		req := c.Request
		nameParam := req.URL.Query().Get("name")
		if nameParam == "" {
			log.Println("param is empty")
			c.AbortWithError(http.StatusBadRequest, errors.New("param is empty"))
			return
		}
		req = req.WithContext(context.WithValue(req.Context(), "name", nameParam))
		c.Request = req
		c.Next()
	}
}

@mkruczek
Copy link
Author

mkruczek commented Sep 11, 2023

@VarusHsu yours solution is good for me, thank you :)

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

3 participants