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

Config to prevent reflection of user input when reporting errors #9811

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

spericas
Copy link
Member

@spericas spericas commented Feb 18, 2025

Description

New config section in listeners for error handling. Default settings prevent any entity from being returned to avoid reflecting any data from a request. Default settings can be modified to return safe messages and to log all messages. Issue #9698.

Documentation

None

@spericas spericas added webserver 4.x Version 4.x labels Feb 18, 2025
@spericas spericas added this to the 4.2.0 milestone Feb 18, 2025
@spericas spericas self-assigned this Feb 18, 2025
@spericas spericas marked this pull request as draft February 18, 2025 15:02
@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Feb 18, 2025
@spericas spericas marked this pull request as ready for review February 18, 2025 15:42
Copy link
Member

@tomas-langer tomas-langer left a comment

Choose a reason for hiding this comment

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

We are reflecting back a cleaned user input (it should not contain the actual illegal character, unless it is an OK entity to return - i.e. if [ is forbidden, we can still return it, as it is not an HTML entity, so it does not matter
Even if we had a bug that returns illegal characters, the fix is to remove the illegal character from the returned string (it should be replaced with some other character, to keep the indexes - this should be already implemented).

@spericas
Copy link
Member Author

We are reflecting back a cleaned user input (it should not contain the actual illegal character, unless it is an OK entity to return - i.e. if [ is forbidden, we can still return it, as it is not an HTML entity, so it does not matter Even if we had a bug that returns illegal characters, the fix is to remove the illegal character from the returned string (it should be replaced with some other character, to keep the indexes - this should be already implemented).

Yes, that's the current behavior, but I still think it is unnecessary to return back all those characters as we are doing now. Pointing out the invalid char in the URI should be sufficient.

Copy link
Member

@tomas-langer tomas-langer left a comment

Choose a reason for hiding this comment

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

I do not think that this is a bug - I have commented in detail on the original issue.

@spericas spericas marked this pull request as draft February 19, 2025 13:51
…prevent any entity from being returned to avoid reflecting any data from a request. Default settings can be modified to return safe messages and to log all messages.
@spericas spericas changed the title Prevents reflection of user input when reporting URI validation errors Config to prevent reflection of user input when reporting errors Feb 19, 2025
@spericas spericas force-pushed the issue-9698 branch 2 times, most recently from c3b1fda to 7f6fbc7 Compare February 19, 2025 19:31
Signed-off-by: Santiago Pericas-Geertsen <santiago.pericasgeertsen@oracle.com>
* @return optional error handling
*/
@Option.Configured
Optional<ErrorHandling> errorHandling();
Copy link
Member

Choose a reason for hiding this comment

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

This could probably return ErrorHandling instead of Optional<ErrorHandling> using @io.helidon.builder.api.Option.DefaultMethod("create"), as both options have sensible defaults.
This would make usage a bit nicer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.x Version 4.x OCA Verified All contributors have signed the Oracle Contributor Agreement. webserver
Projects
Status: Sprint Scope
Development

Successfully merging this pull request may close these issues.

2 participants