-
Notifications
You must be signed in to change notification settings - Fork 90
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
[backend] Fix Regular expression injection #2137
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2137 +/- ##
============================================
- Coverage 35.12% 35.10% -0.02%
Complexity 1418 1418
============================================
Files 535 535
Lines 16189 16197 +8
Branches 1037 1037
============================================
Hits 5686 5686
- Misses 10209 10217 +8
Partials 294 294 ☔ View full report in Codecov by Sentry. |
try { | ||
UUID.fromString(id); | ||
} catch (IllegalArgumentException e) { | ||
throw new IllegalArgumentException("ID invalid : " + id, e); |
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.
Will this cause HTTP 500, where it should be HTTP 400 IMO?
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.
How about throwing https://www.baeldung.com/spring-response-status-exception instead, wrapping the original exception?
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.
You are right
I can throw an ElementNotFoundException and it will be handled by the RestBehavior class
@ExceptionHandler(ElementNotFoundException.class)
public ResponseEntity handleElementNotFoundException(ElementNotFoundException ex) {
ErrorMessage message = new ErrorMessage("Element not found: " + ex.getMessage());
log.warning("ElementNotFoundException: " + ex.getMessage());
return new ResponseEntity<>(message, HttpStatus.NOT_FOUND);
}
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.
That's for HTTP 404, but perhaps there is a readymade handler for 400 as well?
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.
I can use InputValidationException
@ResponseStatus(HttpStatus.BAD_REQUEST)
@ExceptionHandler(InputValidationException.class)
public ValidationErrorBag handleInputValidationExceptions(InputValidationException ex) {
ValidationErrorBag bag = new ValidationErrorBag();
ValidationError errors = new ValidationError();
Map<String, ValidationContent> errorsBag = new HashMap<>();
errorsBag.put(ex.getField(), new ValidationContent(ex.getMessage()));
errors.setChildren(errorsBag);
bag.setErrors(errors);
return bag;
}
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.
sounds like a plan
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.
I made the changes.
Related issues