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

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

Open
flowonyx opened this issue Aug 22, 2021 · 4 comments

Comments

@flowonyx
Copy link

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

template/jet/jet.go

Lines 233 to 237 in c4b2c7e

func jetVarMap(binding interface{}) jet.VarMap {
var bind jet.VarMap
if binding == nil {
return bind
}

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

template/jet/jet.go

Lines 219 to 229 in c4b2c7e

bind := jetVarMap(binding)
if len(layout) > 0 {
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)
}

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.

@justledbetter
Copy link

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
Copy link

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
Copy link

(... 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!)

@gaby
Copy link
Member

gaby commented Dec 14, 2024

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

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