Skip to content

Commit ed6f2e8

Browse files
sixcolorssixcolors
committed
feat(migrate): add session Release() migration with scope-aware tracking
Implements migration to add defer sess.Release() calls for v3 session Store Pattern usage. In v3, sessions obtained via store.Get() and store.GetByID() must be manually released back to the pool. Key Features: - Parses imports to find v3 session package (skips v2) - Tracks session.NewStore() variables specifically - Scope-aware: verifies store variable is from session.NewStore() in current function scope to prevent false positives - Handles closures accessing parent scope variables - Adds defer Release() after error checks or immediately if no check - Prevents duplicates by checking for existing Release() calls - Safe with nil (Release() has nil guard) Edge Cases Handled: ✅ No error checking (sess, _ := store.Get(c)) ✅ Already has defer (no duplicates) ✅ Multiline error blocks ✅ Middleware pattern (correctly excluded - middleware manages lifecycle) ✅ False positives (cache.Get, Ent ORM, CSRF - correctly excluded) ✅ Various store variable names (store, sessionStore, myStore) ✅ V2 imports (correctly skipped - migration runs after v2→v3 upgrade) ✅ Cross-function variable name collision (store in session vs cache) ✅ Closures/anonymous functions accessing parent scope ✅ Real-world examples from gofiber/recipes verified Test Coverage: - 13 comprehensive tests covering all edge cases - Includes real-world patterns from csrf-with-session and ent-mysql - 0 linting issues Fixes gofiber/recipes#3841
1 parent 14ad351 commit ed6f2e8

File tree

2 files changed

+774
-76
lines changed

2 files changed

+774
-76
lines changed

‎cmd/internal/migrations/v3/session_release.go‎

Lines changed: 201 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -16,87 +16,221 @@ const releaseComment = "// Important: Manual cleanup required"
1616
// MigrateSessionRelease adds defer sess.Release() after store.Get() calls
1717
// when using the Store Pattern (legacy pattern).
1818
// This is required in v3 for manual session lifecycle management.
19+
//
20+
// Only the following Store methods return *Session from the pool and require Release():
21+
// - store.Get(c fiber.Ctx) (*Session, error)
22+
// - store.GetByID(ctx context.Context, id string) (*Session, error)
23+
//
24+
// Middleware handlers do NOT require Release() as the middleware manages the lifecycle.
1925
func MigrateSessionRelease(cmd *cobra.Command, cwd string, _, _ *semver.Version) error {
20-
// Match patterns like:
21-
// sess, err := store.Get(c)
22-
// sess, err := store.GetByID(ctx, sessionID)
23-
// session, err := myStore.Get(c)
24-
// Capture: variable name, store variable name, method call
25-
reStoreGet := regexp.MustCompile(`(?m)^(\s*)(\w+),\s*(\w+)\s*:=\s*(\w+)\.(Get(?:ByID)?)\(`)
26-
2726
changed, err := internal.ChangeFileContent(cwd, func(content string) string {
2827
lines := strings.Split(content, "\n")
29-
result := make([]string, 0, len(lines))
3028

31-
for i := 0; i < len(lines); i++ {
32-
line := lines[i]
33-
result = append(result, line)
29+
// Step 1: Find session package import and its alias
30+
sessionPkgAlias := findSessionPackageAlias(lines)
31+
if sessionPkgAlias == "" {
32+
// No session package imported, skip this file
33+
return content
34+
}
3435

35-
// Check if this line matches a store.Get() call
36-
matches := reStoreGet.FindStringSubmatch(line)
37-
if len(matches) < 6 {
38-
continue
39-
}
36+
// Step 2: Find all Store variable names created from session package
37+
storeVars := findSessionStoreVariables(lines, sessionPkgAlias)
38+
if len(storeVars) == 0 {
39+
// No session stores found, skip this file
40+
return content
41+
}
4042

41-
indent := matches[1]
42-
sessVar := matches[2]
43-
errVar := matches[3]
43+
// Step 3: Process the file and add Release() calls where needed
44+
return addReleaseCalls(lines, storeVars)
45+
})
46+
if err != nil {
47+
return fmt.Errorf("failed to add session Release() calls: %w", err)
48+
}
49+
if !changed {
50+
return nil
51+
}
4452

45-
// Look for the error check pattern after this line
46-
// Common patterns:
47-
// if err != nil {
48-
// if err != nil { return ... }
49-
nextLineIdx := i + 1
50-
if nextLineIdx >= len(lines) {
51-
continue
52-
}
53+
cmd.Println("Adding defer sess.Release() for Store Pattern usage")
54+
return nil
55+
}
5356

54-
nextLine := strings.TrimSpace(lines[nextLineIdx])
57+
// findSessionPackageAlias finds the alias used for the session package import.
58+
// Returns the alias (e.g., "session", "sshadow") or empty string if not found.
59+
// Note: This migration runs AFTER MigrateContribPackages, so imports are already v3.
60+
func findSessionPackageAlias(lines []string) string {
61+
// Match: import "github.com/gofiber/fiber/v3/middleware/session"
62+
// Or: import sessionAlias "github.com/gofiber/fiber/v3/middleware/session"
63+
reSessionImport := regexp.MustCompile(`^\s*(?:(\w+)\s+)?"github\.com/gofiber/fiber/v3/middleware/session"`)
5564

56-
// Check if the next line starts an error check
57-
if !strings.HasPrefix(nextLine, "if "+errVar+" != nil") {
58-
continue
65+
for _, line := range lines {
66+
matches := reSessionImport.FindStringSubmatch(line)
67+
if len(matches) > 0 {
68+
if matches[1] != "" {
69+
// Custom alias
70+
return matches[1]
5971
}
72+
// Default alias is the package name
73+
return "session"
74+
}
75+
}
76+
return ""
77+
}
6078

61-
// Find where the error block ends
62-
blockEnd := findErrorBlockEnd(lines, nextLineIdx)
79+
// findSessionStoreVariables finds all variable names that are session.NewStore().
80+
// Returns a map of variable names that are session stores.
81+
// Note: This migration runs AFTER MigrateSessionStore, so session.New() has already
82+
// been converted to session.NewStore().
83+
func findSessionStoreVariables(lines []string, sessionPkgAlias string) map[string]bool {
84+
storeVars := make(map[string]bool)
6385

64-
// Insert defer after the error block
65-
if blockEnd < 0 || blockEnd >= len(lines) {
66-
continue
67-
}
86+
// Match patterns like:
87+
// store := session.NewStore()
88+
// var store = session.NewStore()
89+
// var store *session.Store
90+
// myStore := session.NewStore(config)
91+
reStoreNewStore := regexp.MustCompile(fmt.Sprintf(`^\s*(?:var\s+)?(\w+)\s*(?::=|=)\s*%s\.NewStore\(`, regexp.QuoteMeta(sessionPkgAlias)))
92+
reStoreType := regexp.MustCompile(fmt.Sprintf(`^\s*(?:var\s+)?(\w+)\s+\*?%s\.Store`, regexp.QuoteMeta(sessionPkgAlias)))
93+
94+
for _, line := range lines {
95+
// Check for NewStore() calls
96+
if matches := reStoreNewStore.FindStringSubmatch(line); len(matches) > 1 {
97+
storeVars[matches[1]] = true
98+
continue
99+
}
100+
101+
// Check for *Store type declarations
102+
if matches := reStoreType.FindStringSubmatch(line); len(matches) > 1 {
103+
storeVars[matches[1]] = true
104+
}
105+
}
106+
107+
return storeVars
108+
}
109+
110+
// isSessionStoreInScope verifies that a store variable is actually from session.NewStore()
111+
// within the current function scope by looking backwards from the Get() call.
112+
// This prevents false positives when the same variable name is reused in different functions.
113+
func isSessionStoreInScope(lines []string, getLineIdx int, storeVar string, storeVars map[string]bool) bool {
114+
// The store variable name must be in our tracked list
115+
if !storeVars[storeVar] {
116+
return false
117+
}
118+
119+
// Look backwards to find where this store variable was assigned
120+
// Track depth to handle nested scopes (closures can access parent scope)
121+
braceDepth := 0
122+
123+
for i := getLineIdx - 1; i >= 0; i-- {
124+
line := lines[i]
125+
trimmed := strings.TrimSpace(line)
126+
127+
// Count braces to track nesting depth
128+
braceDepth += strings.Count(line, "{")
129+
braceDepth -= strings.Count(line, "}")
130+
131+
// Check if this line assigns the store variable from session.NewStore()
132+
storeAssignPattern := regexp.MustCompile(fmt.Sprintf(`^\s*(?:var\s+)?%s\s*(?::=|=)\s*\w+\.NewStore\(`, regexp.QuoteMeta(storeVar)))
133+
if storeAssignPattern.MatchString(line) {
134+
// Found the assignment - verify it's from session.NewStore()
135+
sessionPattern := regexp.MustCompile(`\bsession\.NewStore\(`)
136+
return sessionPattern.MatchString(line)
137+
}
138+
139+
// Stop if we've reached a named function declaration (not a closure)
140+
// Named functions start with "func FuncName(" not "func("
141+
if strings.HasPrefix(trimmed, "func ") && !strings.HasPrefix(trimmed, "func(") && !strings.HasPrefix(trimmed, "func (") {
142+
// We've hit a different named function, stop
143+
return false
144+
}
145+
}
146+
147+
return false
148+
}
149+
150+
// addReleaseCalls processes lines and adds defer Release() calls after store.Get()/GetByID() calls.
151+
func addReleaseCalls(lines []string, storeVars map[string]bool) string {
152+
// Build regex pattern that only matches our known store variables
153+
storeNames := make([]string, 0, len(storeVars))
154+
for name := range storeVars {
155+
storeNames = append(storeNames, regexp.QuoteMeta(name))
156+
}
157+
158+
if len(storeNames) == 0 {
159+
return strings.Join(lines, "\n")
160+
}
161+
162+
// Match: sessVar, errVar := (store|sessionStore|myStore).Get(...) or .GetByID(...)
163+
storePattern := strings.Join(storeNames, "|")
164+
reStoreGet := regexp.MustCompile(fmt.Sprintf(`(?m)^(\s*)(\w+),\s*(\w+)\s*:=\s*(%s)\.(Get(?:ByID)?)\(`, storePattern))
165+
166+
result := make([]string, 0, len(lines))
167+
168+
for i := 0; i < len(lines); i++ {
169+
line := lines[i]
170+
result = append(result, line)
68171

69-
// Check if there's already a defer sess.Release() after the error block
70-
hasRelease := false
71-
searchEnd := blockEnd + 20
72-
if searchEnd > len(lines) {
73-
searchEnd = len(lines)
172+
// Check if this line matches a store.Get() call
173+
matches := reStoreGet.FindStringSubmatch(line)
174+
if len(matches) < 6 {
175+
continue
176+
}
177+
178+
indent := matches[1]
179+
sessVar := matches[2]
180+
errVar := matches[3]
181+
storeVar := matches[4]
182+
183+
// CRITICAL: Verify this store variable is actually from session.NewStore()
184+
// in the current function scope to avoid false positives across functions
185+
if !isSessionStoreInScope(lines, i, storeVar, storeVars) {
186+
continue
187+
}
188+
189+
// Check if Release() is already present for this session variable
190+
// Search from right after the Get() line
191+
hasRelease := false
192+
searchEnd := i + 30 // Look ahead up to 30 lines
193+
if searchEnd > len(lines) {
194+
searchEnd = len(lines)
195+
}
196+
for j := i + 1; j < searchEnd; j++ {
197+
if strings.Contains(lines[j], sessVar+".Release()") {
198+
hasRelease = true
199+
break
74200
}
75-
for j := blockEnd + 1; j < searchEnd; j++ {
76-
if strings.Contains(lines[j], sessVar+".Release()") {
77-
hasRelease = true
78-
break
79-
}
80-
// Stop searching if we hit a closing brace at the same or lower indent level
81-
// Only stop on lines that are purely closing braces (possibly with trailing comments)
82-
trimmed := strings.TrimSpace(lines[j])
83-
if strings.HasPrefix(trimmed, "}") && !strings.Contains(trimmed, "{") && !strings.Contains(trimmed, "else") {
84-
break
85-
}
201+
// Stop searching if we hit a closing brace at root function level
202+
// (avoid searching beyond the current function scope)
203+
trimmed := strings.TrimSpace(lines[j])
204+
if trimmed == "}" && len(indent) == 0 {
205+
break
86206
}
207+
}
87208

88-
if hasRelease {
89-
// Skip ahead to avoid re-processing these lines
90-
for i < blockEnd {
91-
i++
92-
if i < len(lines) {
93-
result = append(result, lines[i])
94-
}
95-
}
209+
if hasRelease {
210+
continue
211+
}
212+
213+
// Look for the error check pattern after this line
214+
nextLineIdx := i + 1
215+
if nextLineIdx >= len(lines) {
216+
// End of file - add defer right after the Get() call
217+
deferLine := indent + "defer " + sessVar + ".Release() " + releaseComment
218+
result = append(result, deferLine)
219+
continue
220+
}
221+
222+
nextLine := strings.TrimSpace(lines[nextLineIdx])
223+
224+
// Check if the next line starts an error check
225+
if strings.HasPrefix(nextLine, "if "+errVar+" != nil") {
226+
// Find where the error block ends
227+
blockEnd := findErrorBlockEnd(lines, nextLineIdx)
228+
229+
if blockEnd < 0 || blockEnd >= len(lines) {
96230
continue
97231
}
98232

99-
// Insert the defer statement after the error block
233+
// Insert defer after the error block
100234
deferLine := indent + "defer " + sessVar + ".Release() " + releaseComment
101235

102236
// Skip ahead in the loop to include all lines up to blockEnd
@@ -109,19 +243,14 @@ func MigrateSessionRelease(cmd *cobra.Command, cwd string, _, _ *semver.Version)
109243

110244
// Now insert the defer line
111245
result = append(result, deferLine)
246+
} else {
247+
// No error check - add defer immediately after the Get() call
248+
deferLine := indent + "defer " + sessVar + ".Release() " + releaseComment
249+
result = append(result, deferLine)
112250
}
113-
114-
return strings.Join(result, "\n")
115-
})
116-
if err != nil {
117-
return fmt.Errorf("failed to add session Release() calls: %w", err)
118-
}
119-
if !changed {
120-
return nil
121251
}
122252

123-
cmd.Println("Adding defer sess.Release() for Store Pattern usage")
124-
return nil
253+
return strings.Join(result, "\n")
125254
}
126255

127256
// findErrorBlockEnd finds the end of an error handling block

0 commit comments

Comments
 (0)