Skip to content

Fix middleware duplicated creation of transactions#1196

Draft
jptosso wants to merge 2 commits into
mainfrom
fix(middleware)-duplicated-transaction-creation
Draft

Fix middleware duplicated creation of transactions#1196
jptosso wants to merge 2 commits into
mainfrom
fix(middleware)-duplicated-transaction-creation

Conversation

@jptosso

@jptosso jptosso commented Nov 5, 2024

Copy link
Copy Markdown
Member

Thank you for contributing to Coraza WAF, your effort is greatly appreciated
Before submitting check if what you want to add to coraza list meets quality standards before sending pull request. Thanks!

Make sure that you've checked the boxes below before you submit PR:

Thanks for your contribution ❤️

@jptosso jptosso requested a review from a team as a code owner November 5, 2024 15:22
@jptosso jptosso marked this pull request as draft November 5, 2024 15:22
@fzipi

fzipi commented Dec 29, 2024

Copy link
Copy Markdown
Member

@jptosso Can you follow up on this one?

@fzipi

fzipi commented Apr 3, 2026

Copy link
Copy Markdown
Member

@copilot Is this still valid?

Copilot AI commented Apr 3, 2026

Copy link
Copy Markdown
Contributor

@copilot Is this still valid?

No, this PR is no longer valid. The previous change introduced compilation errors in http/middleware.go:

  1. newTX was used with = (assignment) before being declared in scope
  2. newTX in the else block was declared with := creating a local variable that was never used
  3. newTX was undefined at the call site inside fn

The original pattern in the codebase (which is still present in main) is correct — declare newTX once with the default, then optionally replace it if the WAF implements experimental.WAFWithOptions. There was no actual "duplicated creation of transactions" issue in the original code.

I've reverted http/middleware.go to the correct pattern in commit 8a5604e.

Copilot finished work on behalf of fzipi April 3, 2026 20:42
Copilot AI requested a review from fzipi April 3, 2026 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants