CASSSIDECAR-461: Validate snapshot name during list snapshot#348
CASSSIDECAR-461: Validate snapshot name during list snapshot#348frankgh wants to merge 3 commits into
Conversation
| */ | ||
| String validateSnapshotName(@NotNull String snapshotName); | ||
| default String validateSnapshotName(@NotNull String snapshotName) | ||
| { |
There was a problem hiding this comment.
moved implementation to default implementation in the interface. RegexBasedCassandraInputValidator didn't seem the appropriate place for a non-regex validation
| return SnapshotRequestParam.builder() | ||
| .qualifiedTableName(qualifiedTableName(context)) | ||
| .snapshotName(context.pathParam("snapshot")) | ||
| .snapshotName(validator.validateSnapshotName(context.pathParam("snapshot"))) |
There was a problem hiding this comment.
I realized this might not be a real issue because we can't have / or \0 (null character) in the path parameter. I tried coming up with a test, but the request fails before reaching the snapshot handler with a 404 (because the route is incorrect). I think we should still do the validation here in case in the future the snapshot name validation changes.
There was a problem hiding this comment.
try %2e%2e/%2e%2e (escaped encoding of ../..) for the snapshot name?
There was a problem hiding this comment.
I did, it is still a 404
There was a problem hiding this comment.
I found a case that relies on the validator to reject, 400 error instead of 404. The pathParam value is ..%2F.., i.e. ../..
Without the validator, the snapshot name is ../.. and it is passed to handler to process.
Patch by Francisco Guerrero; reviewed by TBD for CASSSIDECAR-461
3 participants
Patch by Francisco Guerrero; reviewed by TBD for CASSSIDECAR-461