-
-
Notifications
You must be signed in to change notification settings - Fork 219
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
feat(middleware): v4 experimental middleware #986
base: main
Are you sure you want to change the base?
Conversation
…ooks to the middleware. This is specially useful when you want to use transaction data to populate things like logs or spans in a trace enriching the experience and connecting the dots with observability.
//go:embed error_template.html error_template.html | ||
var embededTemplates embed.FS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So... are these going to be html/template
s or just html code? At first read I thought they were not static html files only.
In summary:
- are these meant to be static html?
- wouldn't be more interesting to use
html/template
templates that can be filled with details about the transaction? E.g. "Some error found. Contact your admin with this code: { < the transaction ID> }"`
If we use static html, maybe removing the "_template" in the name will make more sense, and use "default_error.html" and "default_interruption.html" instead?
"net/http" | ||
|
||
"github.com/corazawaf/coraza/v3" | ||
) | ||
|
||
//go:embed error_template.html error_template.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
//go:embed error_template.html error_template.html | |
//go:embed error_template.html interruption_template.html |
That's just to fix this. But I suggest changing to "default_error.html" and "default_interruption.html", pending on the next comment.
errorTemplate, err = embededTemplates.ReadFile("error_template.html") | ||
if err != nil { | ||
panic(err) | ||
} | ||
interruptionTemplate, err = embededTemplates.ReadFile("interruption_template.html") | ||
if err != nil { | ||
panic(err) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we avoid error handling at the init
level? Can we have a readTemplates
method instead that will be called by the middleware?
// Interruption template supports variables in macro expansion format: %{var} | ||
// Variables are: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Macro expansion as in SecRule macro expansion?
@jptosso Care to take a look at comments? |
And provide a description about what this PR does? |
No description provided.