Skip to content

Commit

Permalink
fix(fiber): Fix fiber returning 500 internal server error
Browse files Browse the repository at this point in the history
fde5929 caused a regression
in the fiber adapter logic as we previously would try to get the source IP from nil, which
internally would just return an empty string.

Since above commit, the RemoteAddr is not nil anymore, but is lacking the required port
that is needed in order to parse it as a *TCPAddr

This commit makes sure the port section is always added if it does not exist. Furthermore, it
adds a test that prevents this from happening again and we now have logging in place.
  • Loading branch information
marvin-w committed May 27, 2023
1 parent 06305ac commit 004ea1c
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 3 deletions.
17 changes: 14 additions & 3 deletions fiber/adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@ package fiberadapter

import (
"context"
"io/ioutil"
"fmt"
"io"
"log"
"net"
"net/http"
"strings"

"github.com/aws/aws-lambda-go/events"
"github.com/gofiber/fiber/v2"
Expand Down Expand Up @@ -103,7 +106,7 @@ func (f *FiberLambda) adaptor(w http.ResponseWriter, r *http.Request) {
defer fasthttp.ReleaseRequest(req)

// Convert net/http -> fasthttp request
body, err := ioutil.ReadAll(r.Body)
body, err := io.ReadAll(r.Body)
if err != nil {
http.Error(w, utils.StatusMessage(fiber.StatusInternalServerError), fiber.StatusInternalServerError)
return
Expand All @@ -129,8 +132,16 @@ func (f *FiberLambda) adaptor(w http.ResponseWriter, r *http.Request) {
}
}

remoteAddr, err := net.ResolveTCPAddr("tcp", r.RemoteAddr)
// We need to make sure the net.ResolveTCPAddr call works as it expects a port
addrWithPort := r.RemoteAddr
if !strings.Contains(r.RemoteAddr, ":") {
addrWithPort = r.RemoteAddr + ":80" // assuming a default port
}

remoteAddr, err := net.ResolveTCPAddr("tcp", addrWithPort)
if err != nil {
fmt.Printf("could not resolve TCP address for addr %s\n", r.RemoteAddr)
log.Println(err)
http.Error(w, utils.StatusMessage(fiber.StatusInternalServerError), fiber.StatusInternalServerError)
return
}
Expand Down
33 changes: 33 additions & 0 deletions fiber/fiberlambda_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,39 @@ var _ = Describe("FiberLambda tests", func() {
})
})

Context("RemoteAddr handling", func() {
It("Properly parses the IP address", func() {
app := fiber.New()
app.Get("/ping", func(c *fiber.Ctx) error {
// make sure the ip address is actually set properly
Expect(c.Context().RemoteAddr().String()).To(Equal("8.8.8.8:80"))
return c.SendString("pong")
})

adapter := fiberadaptor.New(app)

req := events.APIGatewayProxyRequest{
Path: "/ping",
HTTPMethod: "GET",
RequestContext: events.APIGatewayProxyRequestContext{
Identity: events.APIGatewayRequestIdentity{
SourceIP: "8.8.8.8",
},
},
}

resp, err := adapter.ProxyWithContext(context.Background(), req)

Expect(err).To(BeNil())
Expect(resp.StatusCode).To(Equal(200))

resp, err = adapter.Proxy(req)

Expect(err).To(BeNil())
Expect(resp.StatusCode).To(Equal(200))
})
})

Context("Request header", func() {
It("Check pass canonical header to fiber", func() {
app := fiber.New()
Expand Down

0 comments on commit 004ea1c

Please sign in to comment.