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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 c.OriginalURL() changed after SendFile #1499

Closed
trim21 opened this issue Aug 20, 2021 · 6 comments 路 Fixed by #1553
Closed

馃悰 c.OriginalURL() changed after SendFile #1499

trim21 opened this issue Aug 20, 2021 · 6 comments 路 Fixed by #1553

Comments

@trim21
Copy link
Contributor

trim21 commented Aug 20, 2021

Fiber version

github.com/gofiber/fiber/v2 v2.17.0

Issue description

c.OriginalURL() changed after c.SendFile

Code snippet

package main

import (
	"fmt"
	"log"

	"github.com/gofiber/fiber/v2"
)

func main() {
	app := fiber.New()

	// Steps to reproduce
	app.Get("/index", func(c *fiber.Ctx) error {
		fmt.Println("original", c.OriginalURL()) // "/index?page=1"
		err := c.SendFile(fmt.Sprintf("files/thread/%s.html", c.Query("page", "1")))
		fmt.Println(c.OriginalURL()) // "$CWD/files/thread/1.html"
		return err
	})

	log.Fatal(app.Listen(":3000"))
}
@ReneWerner87
Copy link
Member

https://github.com/gofiber/fiber/blob/master/ctx.go#L1053-L1075

right , we change the original url so we can use the fasthttp internal functionality

probably we should change this back

maybe you can support us with a pull request and a unittest for it

@trim21

This comment has been minimized.

@trim21
Copy link
Contributor Author

trim21 commented Aug 20, 2021

this should be separated into 2 issues maybe.

@ReneWerner87
Copy link
Member

feel free to create a pull request with the solution for both things

@trim21

This comment has been minimized.

@geet
Copy link
Contributor

geet commented Feb 20, 2022

@ReneWerner87 I think we need to reopen this issue. If I try the code snippet mentioned in the comment , I can still reproduce the bug. Try hitting the /index endpoint twice.
i gave a fix for this in the PR

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

Successfully merging a pull request may close this issue.

3 participants