template icon indicating copy to clipboard operation
template copied to clipboard

When calling Render with a layout and a nil binding: panic: assignment to entry in nil map

Open flowonyx opened this issue 4 years ago • 4 comments

The issue is that in jetVarMap it assumes that a nil binding should result in a nil jet.VarMap.

https://github.com/gofiber/template/blob/c4b2c7ea24bfc975a0c0a438b15d3fb6a244db41/jet/jet.go#L233-L237

But if a layout is specified, it is then used to set a function and panics:

https://github.com/gofiber/template/blob/c4b2c7ea24bfc975a0c0a438b15d3fb6a244db41/jet/jet.go#L219-L229

This could be solved either by replacing var bind jet.VarMap with bind := make(jet.VarMap) or by checking if bind is nil (and makeing it if not) in Render right after if len(layout) > 0 {.

// var bind jet.VarMap
bind := make(jet.VarMap)

Or in Render

bind := jetVarMap(binding) 
 if len(layout) > 0 { 
        if bind == nil {
                bind = make(jet.VarMap)
        }
 	lay, err := e.Templates.GetTemplate(layout[0]) 
 	if err != nil { 
 		return err 
 	} 
 	bind.Set(e.layout, func() { 
 		_ = tmpl.Execute(out, bind, nil) 
 	}) 
 	return lay.Execute(out, bind, nil) 
 } 

I would send a pull request, but I'm not sure if it matters to you which way it is fixed. I would think it would be much clearer to put it in jetVarMap and get rid of the other calls to make there, but I guess there was a reason to not do that in the first place.


The work around is for the caller of Render to just specify an empty jet.VarMap or fiber.Map but that is not obvious and panicing from forgetting is not great.

flowonyx avatar Aug 22 '21 13:08 flowonyx

I've found another way this blows up:

When using gofiber/fiber/v3 on the upstream and using fiber.Ctx.Render to draw the page content using Jet templates, we're forced to pass in a v3 fiber.Map to the calling function, which fails the (type) switch in jetVarMap since fiber/v3.Map is not the same thing as the fiber/v2.Map that the case statement on line 205 expects:

	switch binds := binding.(type) {
	case map[string]interface{}:
		bind = make(jet.VarMap)
		for key, value := range binds {
			bind.Set(key, value)
		}
	case fiber.Map: // <-------------------- this is (fiber/v2).Map, but we're passing in (v3).Map
		bind = make(jet.VarMap)
		for key, value := range binds {
			bind.Set(key, value)
		}
	case jet.VarMap:
		bind = binds
	}

Is there any way to upgrade gofiber/template to work with gofiber/fiber/v3?

justledbetter avatar Dec 14 '24 13:12 justledbetter

Found a way to get it to work (by adding an import to gofiber/fiber/v3 to template/jet/jet.go; The patch to jet.go looks like the following (happy to submit as a PR but holding off on doing that until I get your blessing that this is a desirable solution.... 😅)

--- a/jet/jet.go
+++ b/jet/jet.go
@@ -10,6 +10,7 @@ import (
        "strings"
 
        "github.com/gofiber/fiber/v2"
+       newfiber "github.com/gofiber/fiber/v3"
 
        "github.com/CloudyKit/jet/v6"
        "github.com/CloudyKit/jet/v6/loaders/httpfs"
@@ -207,6 +208,11 @@ func jetVarMap(binding interface{}) jet.VarMap {
                for key, value := range binds {
                        bind.Set(key, value)
                }
+       case newfiber.Map:
+               bind = make(jet.VarMap)
+               for key, value := range binds {
+                       bind.Set(key, value)
+               }
        case jet.VarMap:
                bind = binds
        }

justledbetter avatar Dec 14 '24 14:12 justledbetter

(... Of course, upgrading the gofiber/template package to v3 may be in the works already; Happy to beta test if that's the right way to do it!)

justledbetter avatar Dec 14 '24 14:12 justledbetter

I've found another way this blows up:

When using gofiber/fiber/v3 on the upstream and using fiber.Ctx.Render to draw the page content using Jet templates, we're forced to pass in a v3 fiber.Map to the calling function, which fails the (type) switch in jetVarMap since fiber/v3.Map is not the same thing as the fiber/v2.Map that the case statement on line 205 expects:

	switch binds := binding.(type) {
	case map[string]interface{}:
		bind = make(jet.VarMap)
		for key, value := range binds {
			bind.Set(key, value)
		}
	case fiber.Map: // <-------------------- this is (fiber/v2).Map, but we're passing in (v3).Map
		bind = make(jet.VarMap)
		for key, value := range binds {
			bind.Set(key, value)
		}
	case jet.VarMap:
		bind = binds
	}

Is there any way to upgrade gofiber/template to work with gofiber/fiber/v3?

Not yet, v3 is still under development. We are trying to merge as many improvements/fixes

gaby avatar Dec 14 '24 14:12 gaby