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

Remove tablecloth library as it's no longer needed. #373

Merged
merged 5 commits into from
Jul 27, 2023
Merged

Conversation

sengi
Copy link
Contributor

@sengi sengi commented Jul 26, 2023

Now that we're finally containerised, we no longer need this library.

Which is lucky for us, because https://github.com/alext/tablecloth/ was archived in February and the last commit was 4 years ago.

Recommend reviewing commit-by-commit since a chunk of this is just generated updates from go mod.

Now that we're finally containerised, we no longer need this library.

Which is lucky for us, because https://github.com/alext/tablecloth/ was
archived in February and the last commit was 4 years ago.
@sengi sengi requested a review from dj-maisy July 26, 2023 15:09
@sengi
Copy link
Contributor Author

sengi commented Jul 26, 2023

cc @chao-xian

@sengi
Copy link
Contributor Author

sengi commented Jul 26, 2023

Ooh nice, the gosec linter doesn't like us using http.ListenAndServe. Could just make it a TODO but since it's come up now and it should be an easy fix, I'll try to get it in this PR.

(It's not a new problem; it's just that Tablecloth was obscuring the problem from the linter before.)

edit: done!

This fixes a potential DoS vulnerability; see
https://blog.cloudflare.com/the-complete-guide-to-golang-net-http-timeouts/#httplistenandserveisdoingitwrong

While we're at it, parse the environment variables in main.go and pass
time.Duration around internally.
Avoids some of the modal popups from the macOS firewall. Annoyingly
there's still one more coming from somewhere, maybe a test library
that's still binding to the zero address? Oh well.
http.ListenAndServe should never be used in production code, so let's
avoid using it in our examples.

Also clean up some old links and fix a tiny spelling/l10n issue :)
Copy link
Contributor

@theseanything theseanything left a comment

Choose a reason for hiding this comment

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

Nice one!

Base automatically changed from sengi/lint to main July 27, 2023 15:21
@sengi sengi merged commit 4c948d8 into main Jul 27, 2023
2 checks passed
@sengi sengi deleted the sengi/rm-tablecloth branch July 27, 2023 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants