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

Add hostname validation #42553

Merged
merged 4 commits into from Oct 6, 2022
Merged

Add hostname validation #42553

merged 4 commits into from Oct 6, 2022

Conversation

evict
Copy link
Contributor

@evict evict commented Oct 5, 2022

Add functionality that validates hostnames and IP addresses. Validate gitolite addresses.

Test plan

  • ci tests
  • manual tests

@evict evict requested review from ryanslade and vrto October 5, 2022 09:59
@evict evict self-assigned this Oct 5, 2022
@cla-bot cla-bot bot added the cla-signed label Oct 5, 2022
@sourcegraph-bot
Copy link
Contributor

sourcegraph-bot commented Oct 5, 2022

Codenotify: Notifying subscribers in CODENOTIFY files for diff a44b3b6...c89e6ea.

Notify File(s)
@indradhanush cmd/gitserver/server/list_gitolite.go
@ryanslade cmd/gitserver/server/list_gitolite.go
@sashaostrikov cmd/gitserver/server/list_gitolite.go

Copy link
Contributor

@ryanslade ryanslade left a comment

Choose a reason for hiding this comment

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

Thanks!

@evict evict requested a review from a team October 5, 2022 12:04
// ValidateRemoteAddr validates if the input is a valid IP or a valid hostname.
// It validates the hostname by attempting to resolve it.
func ValidateRemoteAddr(raddr string) bool {
host, port, err := net.SplitHostPort(raddr)
Copy link
Contributor

@ryanslade ryanslade Oct 5, 2022

Choose a reason for hiding this comment

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

What if err != nil here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then we don't use anything from it. That means that raddr is probably just an IP or just a hostname.

Copy link
Contributor

@ryanslade ryanslade left a comment

Choose a reason for hiding this comment

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

Cool, this looks good to me and the tests help with confidence.

Copy link
Contributor

@vrto vrto left a comment

Choose a reason for hiding this comment

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

Good job!

@evict evict merged commit 5f37217 into main Oct 6, 2022
25 checks passed
@evict evict deleted the vincent/validate-gitolite-hostname branch October 6, 2022 12:10
@evict evict added the SSDLC label Oct 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants