Skip to content
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

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

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
Contributor

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["403"] = ngx.HTTP_FORBIDDEN
M.HTTP_CODE["404"] = ngx.HTTP_NOT_FOUND
M.HTTP_CODE["405"] = ngx.HTTP_NOT_ALLOWED
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