Skip to content

os-nginx : Add error page file extension support #4687

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Nemric
Copy link

@Nemric Nemric commented May 8, 2025

resolve #4684
Allow users to change the extension of the error file so we can now set it to i.e. json to make it well read for app that require json
These changes were made on a running Opnsense so perhaps I did miss to report some modifications here, so, please, review it carefully ^^
As I don't know Opnsense code at all I did a "just working" change
see you :)

@Nemric Nemric changed the title Add error page file extension support os-nginx : Add error page file extension support May 8, 2025
@fichtner fichtner requested a review from kulikov-a May 13, 2025 17:55
@kulikov-a
Copy link
Member

kulikov-a commented May 15, 2025

Hi!
maybe I would stick with the help style of the form (something like "Change the file extension if necessary. Response MIME type will be set according to the extension."). The rest is looking good to me

@Nemric
Copy link
Author

Nemric commented May 15, 2025

Yeah, of course help message could be changed up to you ^^

Co-authored-by: Franco Fichtner <[email protected]>
@@ -1841,6 +1841,10 @@
</check001>
</Constraints>
</pagecontent>
<extension type="TextField">
<default>html</default>
Copy link
Member

Choose a reason for hiding this comment

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

although I'm doubting we need a lot of free-text options here?

Copy link
Author

Choose a reason for hiding this comment

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

do you mean that a drop down list would be better ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

os-nginx : Allow changing error page file extension
3 participants