Skip to content

Comments

Enhancement: ability to set the content type on served ban template.#68

Open
b3cft wants to merge 1 commit intocrowdsecurity:mainfrom
b3cft:enhancement/ban_content_type
Open

Enhancement: ability to set the content type on served ban template.#68
b3cft wants to merge 1 commit intocrowdsecurity:mainfrom
b3cft:enhancement/ban_content_type

Conversation

@b3cft
Copy link
Contributor

@b3cft b3cft commented Apr 14, 2024

Add a new config to allow setting the content type for the banned response.
Should you wish to, for instance sending a plain/text response or just an image.

Set a valid default of text/html so that any existing configs will not be broken by the change.

Validate the content types against a list of permitted types.


Additionally removed duplicated 401 status definition.
Some whitespace changes introduced by editor (even though I tried to stop it).

@b3cft b3cft changed the title New feature: ability to set the content type on served ban template. Enhancement: ability to set the content type on served ban template. Apr 14, 2024
@LaurenceJJones LaurenceJJones self-requested a review August 15, 2024 08:33
end
end
if content_type_ok == false then
ngx.log(ngx.ERR, "RET_CONTENT_TYPE '" .. content_type .. "' is not supported, using default content_type " .. M.content_type)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for such a long wait time, should this be BAN_CONTENT_TYPE instead of RET_CONTENT_TYPE? so they know which property in configuration is wrong?

M.HTTP_CODE["406"] = ngx.HTTP_NOT_ACCEPTABLE
M.HTTP_CODE["500"] = ngx.HTTP_INTERNAL_SERVER_ERROR

M.CONTENT_TYPE = {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm very sorry for the very late review. Is there any reason not to allow users to set content-type directly? I'm afraid the mapping is more error-prone than anything.

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

Successfully merging this pull request may close these issues.

3 participants