fix: Address codebase TODOs in http.go and healthcheck.go#15218
fix: Address codebase TODOs in http.go and healthcheck.go#15218vishnarac wants to merge 1 commit intolinkerd:mainfrom
Conversation
Problem There were some pending TODO comments in the codebase that represented technical debt: missing input validation for HTTP parsing and a hardcoded clock skew value that needed to be configurable. Solution - Added input validation and sanitization to ParseScheme and ParseMethod in pkg/util/http.go. - Configured the allowed clock skew as AllowedClockSkew in the Options struct and updated healthcheck.go to use this field instead of the hardcoded global constant. Validation Ran the test suite locally for pkg/util and pkg/healthcheck (go test ./pkg/util ./pkg/healthcheck), and ran go vet. All tests pass successfully. Signed-off-by: Vishnu N C <vishnuc@ibm.com>
adleong
left a comment
There was a problem hiding this comment.
Hi @vishnarac, thanks for your interest in the project.
I don't think these changes adequately address the TODOs. Adding allowed clock skew to the options struct isn't sufficient, since there is no flag to set this value. The code you added to ParseScheme and ParseMethod doesn't actually do any validation.
More importantly, though, these TODOs are quite old and aren't necessarily still applicable or high value. If you'd like to get involved with the project, I recommend looking for Github issues with the Good First Issue label or opening a new issue discussing what you'd like to work on.
Appreciate the feedback. @adleong Will close this PR. Lemme look at 'Good First Issue label' issues to get started. |
Problem
There were some pending TODO comments in the codebase that represented technical debt: missing input validation for HTTP parsing and a hardcoded clock skew value that needed to be configurable.
Solution
Validation
Ran the test suite locally for pkg/util and pkg/healthcheck (go test ./pkg/util ./pkg/healthcheck), and ran go vet. All tests pass successfully.