diff --git a/.gitattributes b/.gitattributes index f099faa4..f42d752c 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,7 +1,14 @@ -*.bash text eol=lf whitespace=blank-at-eol,space-before-tab,tab-in-indent,trailing-space,tabwidth=4 -*.sh text eol=lf whitespace=blank-at-eol,space-before-tab,tab-in-indent,trailing-space,tabwidth=4 +# shell scripts should not use tabs to indent! +*.bash text eol=lf core.whitespace whitespace=tab-in-indent,trailing-space,tabwidth=2 +*.sh text eol=lf core.whitespace whitespace=tab-in-indent,trailing-space,tabwidth=2 -# files for systemd -*.path text eol=lf whitespace=blank-at-eol,space-before-tab,tab-in-indent,trailing-space,tabwidth=4 -*.service text eol=lf whitespace=blank-at-eol,space-before-tab,tab-in-indent,trailing-space,tabwidth=4 -*.timer text eol=lf whitespace=blank-at-eol,space-before-tab,tab-in-indent,trailing-space,tabwidth=4 +# files for systemd (shell-similar) +*.path text eol=lf core.whitespace whitespace=tab-in-indent,trailing-space,tabwidth=2 +*.service text eol=lf core.whitespace whitespace=tab-in-indent,trailing-space,tabwidth=2 +*.timer text eol=lf core.whitespace whitespace=tab-in-indent,trailing-space,tabwidth=2 + +# go fmt will enforce this, but in case a user has not called "go fmt" allow GIT to catch this: +*.go text eol=lf core.whitespace whitespace=indent-with-non-tab,trailing-space,tabwidth=4 + +*.yml text eol=lf core.whitespace whitespace=tab-in-indent,trailing-space,tabwidth=2 +.git* text eol=auto core.whitespace whitespace=trailing-space diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 204f70cf..73a4e10f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -20,10 +20,10 @@ anything about Web development ### Bug reports -First, please [search this repository](https://github.com/mholt/caddy/search?q=&type=Issues&utf8=%E2%9C%93) +Please [search this repository](https://github.com/mholt/caddy/search?q=&type=Issues&utf8=%E2%9C%93) with a variety of keywords to ensure your bug is not already reported. -If not, [open an issue](https://github.com/mholt/caddy/issues) and answer the +If unique, [open an issue](https://github.com/mholt/caddy/issues) and answer the questions so we can understand and reproduce the problematic behavior. The burden is on you to convince us that it is actually a bug in Caddy. This is @@ -39,12 +39,16 @@ getting free help. If we helped you, please consider ### Minor improvements and new tests -Submit [pull requests](https://github.com/mholt/caddy/pulls) at any time. Make -sure to write tests to assert your change is working properly and is thoroughly -covered. We'll ask most pull requests to be +Submit [pull requests](https://github.com/mholt/caddy/pulls) at any time for +minor changes or new tests. Make sure to write tests to assert your change is +working properly and is thoroughly covered. We'll ask most pull requests to be [squashed](http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html), especially with small commits. +Your pull request may be thoroughly reviewed. This is because if we accept the +PR, we also assume responsibility for it, although we would prefer you to +help maintain your code after it gets merged. + ### Proposals, suggestions, ideas, new features @@ -54,17 +58,23 @@ with a variety of keywords to ensure your suggestion/proposal is new. If so, you may open either an issue or a pull request for discussion and feedback. -The advantage of issues is that you don't have to spend time actually -implementing your idea, but you should still describe it thoroughly. The -advantage of a pull request is that we can immediately see the impact the change -will have on the project, what the code will look like, and how to improve it. -The disadvantage of pull requests is that they are unlikely to get accepted -without significant changes, or it may be rejected entirely. Don't worry, that -won't happen without an open discussion first. +The advantage of issues is that you don't have to spend time implementing your +idea, but you should still describe it thoroughly as if someone reading it would +implement the whole thing starting from scratch. -If you are going to spend significant time implementing code for a pull request, -best to open an issue first and "claim" it and get feedback before you invest -a lot of time. +The advantage of pull requests is that we can immediately see the impact the +change will have on the project, what the code will look like, and how to +improve it. The disadvantage of pull requests is that they are unlikely to get +accepted without significant changes first, or it may be rejected entirely. +Don't worry, that won't happen without an open discussion first. + +If you are going to spend significant time writing code for a new pull request, +best to open an issue to "claim" it and get feedback before you invest a lot of +time. + +Remember: pull requests should always be thoroughly documented both via godoc +and with at least a rough draft of documentation that might go on the website +for users to read. ### Collaborator status @@ -75,6 +85,18 @@ push to the repository and merge other pull requests. We hope that you will stay involved by reviewing pull requests, submitting more of your own, and resolving issues as you are able to. Thanks for making Caddy amazing! +We ask that collaborators will conduct thorough code reviews and be nice to +new contributors. Before merging a PR, it's best to get the approval of +at least one or two other collaborators and/or the project owner. We prefer +squashed commits instead of many little, semantically-unimportant commits. Also, +CI and other post-commit hooks must pass before being merged except in certain +unusual circumstances. + +Collaborator status may be removed for inactive users from time to time as +we see fit; this is not an insult, just a basic security precaution in case +the account becomes inactive or abandoned. Privileges can always be restored +later. + ### Vulnerabilities diff --git a/ISSUE_TEMPLATE b/ISSUE_TEMPLATE index f9d55a2d..433d3f14 100644 --- a/ISSUE_TEMPLATE +++ b/ISSUE_TEMPLATE @@ -18,3 +18,7 @@ #### 6. What did you see instead (give full error messages and/or log)? + + +#### 7. How can someone who is starting from scratch reproduce this behavior as minimally as possible? + diff --git a/build.bash b/build.bash index 7c3bce07..ec8a67d5 100755 --- a/build.bash +++ b/build.bash @@ -17,6 +17,7 @@ set -euo pipefail : ${output_filename:="ecaddy"} : ${git_repo:="${2:-}"} +: ${git_repo:="."} pkg=main ldflags=() diff --git a/caddy/https/crypto_test.go b/caddy/https/crypto_test.go index f61669d7..efa45c65 100644 --- a/caddy/https/crypto_test.go +++ b/caddy/https/crypto_test.go @@ -8,6 +8,7 @@ import ( "crypto/rand" "crypto/rsa" "crypto/x509" + "errors" "os" "runtime" "testing" @@ -95,17 +96,25 @@ func TestSaveAndLoadECCPrivateKey(t *testing.T) { // PrivateKeysSame compares the bytes of a and b and returns true if they are the same. func PrivateKeysSame(a, b crypto.PrivateKey) bool { - return bytes.Equal(PrivateKeyBytes(a), PrivateKeyBytes(b)) + var abytes, bbytes []byte + var err error + + if abytes, err = PrivateKeyBytes(a); err != nil { + return false + } + if bbytes, err = PrivateKeyBytes(b); err != nil { + return false + } + return bytes.Equal(abytes, bbytes) } // PrivateKeyBytes returns the bytes of DER-encoded key. -func PrivateKeyBytes(key crypto.PrivateKey) []byte { - var keyBytes []byte +func PrivateKeyBytes(key crypto.PrivateKey) ([]byte, error) { switch key := key.(type) { case *rsa.PrivateKey: - keyBytes = x509.MarshalPKCS1PrivateKey(key) + return x509.MarshalPKCS1PrivateKey(key), nil case *ecdsa.PrivateKey: - keyBytes, _ = x509.MarshalECPrivateKey(key) + return x509.MarshalECPrivateKey(key) } - return keyBytes + return nil, errors.New("Unknown private key type") } diff --git a/caddy/https/maintain.go b/caddy/https/maintain.go index 28fa2fe6..a0fb0557 100644 --- a/caddy/https/maintain.go +++ b/caddy/https/maintain.go @@ -112,12 +112,21 @@ func renewManagedCertificates(allowPrompts bool) (err error) { // Apply changes to the cache for _, cert := range renewed { + if cert.Names[len(cert.Names)-1] == "" { + // Special case: This is the default certificate, so we must + // ensure it gets updated as well, otherwise the renewal + // routine will find it and think it still needs to be renewed, + // even though we already renewed it... + certCacheMu.Lock() + delete(certCache, "") + certCacheMu.Unlock() + } _, err := cacheManagedCertificate(cert.Names[0], cert.OnDemand) if err != nil { if client.AllowPrompts { return err // operator is present, so report error immediately } - log.Printf("[ERROR] %v", err) + log.Printf("[ERROR] Caching renewed certificate: %v", err) } } for _, cert := range deleted { @@ -178,7 +187,7 @@ func updateOCSPStaples() { if err != nil { if cert.OCSP != nil { // if it was no staple before, that's fine, otherwise we should log the error - log.Printf("[ERROR] Checking OCSP for %s: %v", name, err) + log.Printf("[ERROR] Checking OCSP for %v: %v", cert.Names, err) } continue } diff --git a/caddy/restart.go b/caddy/restart.go index 28b79d7a..717bd3a3 100644 --- a/caddy/restart.go +++ b/caddy/restart.go @@ -11,7 +11,7 @@ import ( "net" "os" "os/exec" - "path" + "path/filepath" "sync/atomic" "github.com/mholt/caddy/caddy/https" @@ -138,7 +138,7 @@ func Restart(newCaddyfile Input) error { func getCertsForNewCaddyfile(newCaddyfile Input) error { // parse the new caddyfile only up to (and including) TLS // so we can know what we need to get certs for. - configs, _, _, err := loadConfigsUpToIncludingTLS(path.Base(newCaddyfile.Path()), bytes.NewReader(newCaddyfile.Body())) + configs, _, _, err := loadConfigsUpToIncludingTLS(filepath.Base(newCaddyfile.Path()), bytes.NewReader(newCaddyfile.Body())) if err != nil { return errors.New("loading Caddyfile: " + err.Error()) } diff --git a/caddy/setup/browse.go b/caddy/setup/browse.go index 28cb2582..6039480b 100644 --- a/caddy/setup/browse.go +++ b/caddy/setup/browse.go @@ -3,6 +3,7 @@ package setup import ( "fmt" "io/ioutil" + "net/http" "text/template" "github.com/mholt/caddy/middleware" @@ -17,7 +18,6 @@ func Browse(c *Controller) (middleware.Middleware, error) { } browse := browse.Browse{ - Root: c.Root, Configs: configs, IgnoreIndexes: false, } @@ -50,6 +50,16 @@ func browseParse(c *Controller) ([]browse.Config, error) { } else { bc.PathScope = "/" } + bc.Root = http.Dir(c.Root) + theRoot, err := bc.Root.Open("/") // catch a missing path early + if err != nil { + return configs, err + } + defer theRoot.Close() + _, err = theRoot.Readdir(-1) + if err != nil { + return configs, err + } // Second argument would be the template file to use var tplText string @@ -85,7 +95,6 @@ const defaultTemplate = `