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

Fix Let's Encrypt handler #5524

Closed
wants to merge 2 commits into from
Closed

Fix Let's Encrypt handler #5524

wants to merge 2 commits into from

Conversation

gregkare
Copy link
Contributor

@gregkare gregkare commented Dec 11, 2018

The issue was that the listen address was wrong (it looked like .0.0.0:443:80).

Also handle errors in the HTTP server go routine, return a fatal error when something goes wrong.

Thanks to @gbl08ma for finding the actual bug

Here is an example of the error handling:

2018/12/11 14:23:07 [....io/gitea/cmd/web.go:87 func1()] [E] Failed to
start the Let's Encrypt handler on port 30: listen tcp 0.0.0.0:30: bind:
permission denied

Closes #5280

Edit: This is my first Go pull request so I am open to suggestions about the error handling

Also handle errors in the HTTP server go routine, return a fatal error
when something goes wrong.

Thanks to @gbl08ma for finding the actual bug

Here is an example of the error handling:

    2018/12/11 14:23:07 [....io/gitea/cmd/web.go:87 func1()] [E] Failed to
    start the Let's Encrypt handler on port 30: listen tcp 0.0.0.0:30: bind:
    permission denied

Closes #5280
@codecov-io
Copy link

Codecov Report

Merging #5524 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5524   +/-   ##
=======================================
  Coverage   37.58%   37.58%           
=======================================
  Files         318      318           
  Lines       46928    46928           
=======================================
  Hits        17638    17638           
  Misses      26778    26778           
  Partials     2512     2512

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9681c83...7b43d6a. Read the comment docs.

@bkcsoft bkcsoft added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 11, 2018
@gregkare
Copy link
Contributor Author

Replacing this PR right now, I created it with the wrong organization

@gregkare gregkare closed this Dec 11, 2018
@gregkare gregkare deleted the bugfix/5280-fix_letsencrypt branch December 11, 2018 13:43
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Server only listening to HTTP_PORT with TLS when Let's Encrypt is enabled
3 participants