Merge pull request #309 from gofiber/fix-django-template-invalid-keys…

…-efficiency

fix: django template invalid keys efficiency

django/README.md

@@ -186,4 +186,12 @@ Hello, World!<br /><br />Greetings from Fiber Team
 
 When working with Pongo2 and this template engine, it's crucial to understand the specific rules for data binding. Only keys that match the following regular expression are supported: `^[a-zA-Z0-9_]+$`.
 
-This means that keys with special characters or punctuation, such as `my-key` or `my.key`, are not compatible and will not be bound to the template. This is a restriction imposed by the underlying Pongo2 template engine. Please ensure your keys adhere to these rules to avoid any binding issues.
+This means that keys with special characters or punctuation, such as `my-key` or `my.key`, are not compatible and will not be bound to the template. This is a restriction imposed by the underlying Pongo2 template engine. Please ensure your keys adhere to these rules to avoid any binding issues.
+
+If you need to access a value in the template that doesn't adhere to the key naming restrictions imposed by the Pongo2 template engine, you can bind the value to a new field when calling `fiber.Render`. Here's an example:
+
+```go
+c.Render("index", fiber.Map{
+    "Fiber": "Hello, World!\n\nGreetings from Fiber Team",
+    "MyKey": c.Locals("my-key"),
+})

django/django.go

@@ -7,7 +7,6 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
-	"regexp"
 	"strings"
 
 	"github.com/gofiber/fiber/v2"
@@ -17,8 +16,6 @@ import (
 	"github.com/gofiber/utils"
 )
 
-var reIdentifiers = regexp.MustCompile("^[a-zA-Z0-9_]+$")
-
 // Engine struct
 type Engine struct {
 	core.Engine
@@ -171,30 +168,43 @@ func getPongoBinding(binding interface{}) pongo2.Context {
 	if binding == nil {
 		return nil
 	}
+	var bind pongo2.Context
 	switch binds := binding.(type) {
 	case pongo2.Context:
-		return createBindFromMap(binds)
+		bind = binds
 	case map[string]interface{}:
-		return createBindFromMap(binds)
+		bind = binds
 	case fiber.Map:
-		return createBindFromMap(binds)
+		bind = make(pongo2.Context)
+		for key, value := range binds {
+			// only add valid keys
+			if isValidKey(key) {
+				bind[key] = value
+			}
+		}
+		return bind
 	}
 
-	return nil
+	// Remove invalid keys
+	for key := range bind {
+		if !isValidKey(key) {
+			delete(bind, key)
+		}
+	}
+
+	return bind
 }
 
-// createBindFromMap creates a pongo2.Context containing
-// only valid identifiers from a map.
-func createBindFromMap(binds map[string]interface{}) pongo2.Context {
-	bind := make(pongo2.Context)
-	for key, value := range binds {
-		if !reIdentifiers.MatchString(key) {
-			// Skip invalid identifiers
-			continue
+// isValidKey checks if the key is valid
+//
+// Valid keys match the following regex: [a-zA-Z0-9_]+
+func isValidKey(key string) bool {
+	for _, ch := range key {
+		if !((ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z') || (ch >= '0' && ch <= '9') || ch == '_') {
+			return false
 		}
-		bind[key] = value
 	}
-	return bind
+	return true
 }
 
 // Render will render the template by name

django/django_test.go

@@ -4,12 +4,17 @@ package django
 import (
 	"bytes"
 	"embed"
+	"math/rand"
 	"net/http"
 	"os"
 	"regexp"
 	"strings"
 	"testing"
+	"unicode"
 
+	"github.com/flosch/pongo2/v6"
+	"github.com/gofiber/fiber/v2"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -100,6 +105,68 @@ func Test_Invalid_Identifiers(t *testing.T) {
 	require.Equal(t, expect, result)
 }
 
+func Test_GetPongoBinding(t *testing.T) {
+	// Test with pongo2.Context
+	ctx := pongo2.Context{"key1": "value1"}
+	result := getPongoBinding(ctx)
+	assert.Equal(t, ctx, result, "Expected the same context")
+
+	// Test with map[string]interface{}
+	mapBinding := map[string]interface{}{"key2": "value2"}
+	result = getPongoBinding(mapBinding)
+	assert.Equal(t, pongo2.Context(mapBinding), result, "Expected the same context")
+
+	// Test with fiber.Map
+	fiberMap := fiber.Map{"key3": "value3"}
+	result = getPongoBinding(fiberMap)
+	assert.Equal(t, pongo2.Context(fiberMap), result, "Expected the same context")
+
+	// Test with unsupported type
+	result = getPongoBinding("unsupported")
+	assert.Nil(t, result, "Expected nil for unsupported type")
+
+	// Test with invalid key
+	invalidCtx := pongo2.Context{"key1": "value1", "invalid.key": "value2"}
+	result = getPongoBinding(invalidCtx)
+	assert.Equal(t, pongo2.Context{"key1": "value1"}, result, "Expected the same context")
+}
+
+func Test_IsValidKey(t *testing.T) {
+	assert.True(t, isValidKey("key1"), "Expected true for valid key")
+	assert.False(t, isValidKey("invalid.key"), "Expected false for invalid key")
+	assert.False(t, isValidKey("invalid-key"), "Expected false for invalid key")
+	assert.False(t, isValidKey("key1\n"), "Expected false for invalid key")
+	assert.False(t, isValidKey("key1 "), "Expected false for invalid key")
+	assert.False(t, isValidKey("👍"), "Expected false for invalid key")
+	assert.False(t, isValidKey("你好"), "Expected false for invalid key")
+
+	// do sume fuzzing where we generate 1000 random strings and check if they are valid keys
+	// valid keys match the following regex: [a-zA-Z0-9_]+
+	reValidkeys := regexp.MustCompile(`^[a-zA-Z0-9_]+$`)
+	for i := 0; i < 1000; i++ {
+		key := generateRandomString(10)
+		assert.Equal(t, reValidkeys.MatchString(key), isValidKey(key), "Expected the same result for key")
+	}
+}
+
+// generateRandomString generates a random string of length n
+// with printable, non-whitespace characters
+//
+// helper function for Test_IsValidKey
+func generateRandomString(n int) string {
+	b := make([]rune, n)
+	for i := range b {
+		for {
+			c := rune(rand.Intn(0x10FFFF))                 // generate a random rune
+			if !unicode.IsSpace(c) && unicode.IsPrint(c) { // check if it's a printable, non-whitespace character
+				b[i] = c
+				break
+			}
+		}
+	}
+	return string(b)
+}
+
 func Test_FileSystem(t *testing.T) {
 	engine := NewFileSystem(http.Dir("./views"), ".django")
 	engine.Debug(true)
@@ -226,6 +293,24 @@ func Test_AddFuncMap(t *testing.T) {
 	require.True(t, ok)
 }
 
+func Test_Invalid_Template(t *testing.T) {
+	engine := New("./views", ".django")
+	require.NoError(t, engine.Load())
+
+	var buf bytes.Buffer
+	err := engine.Render(&buf, "invalid", nil)
+	require.Error(t, err)
+}
+
+func Test_Invalid_Layout(t *testing.T) {
+	engine := New("./views", ".django")
+	require.NoError(t, engine.Load())
+
+	var buf bytes.Buffer
+	err := engine.Render(&buf, "index", nil, "invalid")
+	require.Error(t, err)
+}
+
 func Benchmark_Django(b *testing.B) {
 	expectSimple := `<h1>Hello, World!</h1>`
 	expectExtended := `<!DOCTYPE html><html><head><title>Main</title></head><body><h2>Header</h2><h1>Hello, Admin!</h1><h2>Footer</h2></body></html>`
@@ -264,4 +349,34 @@ func Benchmark_Django(b *testing.B) {
 		require.NoError(b, err)
 		require.Equal(b, expectExtended, trim(buf.String()))
 	})
+
+	b.Run("simple_with_invalid_binding_keys", func(bb *testing.B) {
+		bb.ReportAllocs()
+		bb.ResetTimer()
+		for i := 0; i < bb.N; i++ {
+			buf.Reset()
+			err = engine.Render(&buf, "simple", map[string]interface{}{
+				"Title":       "Hello, World!",
+				"Invalid_Key": "Don't return error from checkForValidIdentifiers!",
+			})
+		}
+
+		require.NoError(b, err)
+		require.Equal(b, expectSimple, trim(buf.String()))
+	})
+
+	b.Run("extended_with_invalid_binding_keys", func(bb *testing.B) {
+		bb.ReportAllocs()
+		bb.ResetTimer()
+		for i := 0; i < bb.N; i++ {
+			buf.Reset()
+			err = engine.Render(&buf, "extended", map[string]interface{}{
+				"User":        admin,
+				"Invalid_Key": "Don't return error from checkForValidIdentifiers!",
+			}, "layouts/main")
+		}
+
+		require.NoError(b, err)
+		require.Equal(b, expectExtended, trim(buf.String()))
+	})
 }