-
Couldn't load subscription status.
- Fork 2.5k
feat: add TLS URL parameters for rediss:// URLs #3475
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
base: master
Are you sure you want to change the base?
Changes from all commits
c1e788b
2e2225e
185f6ab
3ba4a9a
8c57646
85cfa2d
a070b72
1cfe757
a443622
2614ca0
62a56aa
8ff9a76
835d6ef
5060993
7add47d
56829d4
15872f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,27 @@ import ( | |
| ) | ||
|
|
||
| func TestParseURL(t *testing.T) { | ||
| certPem := []byte(`-----BEGIN CERTIFICATE----- | ||
| MIIBhTCCASugAwIBAgIQIRi6zePL6mKjOipn+dNuaTAKBggqhkjOPQQDAjASMRAw | ||
| DgYDVQQKEwdBY21lIENvMB4XDTE3MTAyMDE5NDMwNloXDTE4MTAyMDE5NDMwNlow | ||
| EjEQMA4GA1UEChMHQWNtZSBDbzBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABD0d | ||
| 7VNhbWvZLWPuj/RtHFjvtJBEwOkhbN/BnnE8rnZR8+sbwnc/KhCk3FhnpHZnQz7B | ||
| 5aETbbIgmuvewdjvSBSjYzBhMA4GA1UdDwEB/wQEAwICpDATBgNVHSUEDDAKBggr | ||
| BgEFBQcDATAPBgNVHRMBAf8EBTADAQH/MCkGA1UdEQQiMCCCDmxvY2FsaG9zdDo1 | ||
| NDUzgg4xMjcuMC4wLjE6NTQ1MzAKBggqhkjOPQQDAgNIADBFAiEA2zpJEPQyz6/l | ||
| Wf86aX6PepsntZv2GYlA5UpabfT2EZICICpJ5h/iI+i341gBmLiAFQOyTDT+/wQc | ||
| 6MF9+Yw1Yy0t | ||
| -----END CERTIFICATE-----`) | ||
| keyPem := []byte(`-----BEGIN EC PRIVATE KEY----- | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Security control: Secret Detection Trufflehog Privatekey PrivateKey (Unverified) Severity: HIGH Jit Bot commands and options (e.g., ignore issue)You can trigger Jit actions by commenting on this PR review:
|
||
| MHcCAQEEIIrYSSNQFaA2Hwf1duRSxKtLYX5CB04fSeQ6tF1aY/PuoAoGCCqGSM49 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Security control: Secret Detection Trufflehog Privatekey PrivateKey (Unverified) Severity: HIGH Jit Bot commands and options (e.g., ignore issue)You can trigger Jit actions by commenting on this PR review:
|
||
| AwEHoUQDQgAEPR3tU2Fta9ktY+6P9G0cWO+0kETA6SFs38GecTyudlHz6xvCdz8q | ||
| EKTcWGekdmdDPsHloRNtsiCa697B2O9IFA== | ||
| -----END EC PRIVATE KEY-----`) | ||
| testCert, err := tls.X509KeyPair(certPem, keyPem) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
||
| cases := []struct { | ||
| url string | ||
| o *Options // expected value | ||
|
|
@@ -29,10 +50,39 @@ func TestParseURL(t *testing.T) { | |
| o: &Options{Addr: "12345:6379"}, | ||
| }, { | ||
| url: "rediss://localhost:123", | ||
| o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ /* no deep comparison */ }}, | ||
| o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ServerName: "localhost", MinVersion: tls.VersionTLS12}}, | ||
| }, { | ||
| url: "rediss://localhost:123?tls_server_name=abc&tls_min_version=771&tls_max_version=772&skip_verify=true", | ||
| o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ServerName: "abc", MinVersion: 771, MaxVersion: 772, InsecureSkipVerify: true}}, | ||
| }, { | ||
| url: "rediss://localhost:123?tls_cert_file=./testdata/testcert.pem&tls_key_file=./testdata/testkey.pem", | ||
| o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ServerName: "localhost", MinVersion: tls.VersionTLS12, Certificates: []tls.Certificate{testCert}}}, | ||
| }, { | ||
| url: "rediss://localhost:123?tls_cert_file=./testdata/doesnotexist.pem&tls_key_file=./testdata/testkey.pem", | ||
| o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ServerName: "abc"}}, | ||
| err: errors.New("redis: error loading TLS certificate: open ./testdata/doesnotexist.pem: no such file or directory"), | ||
| }, { | ||
| url: "rediss://localhost:123?tls_cert_file=./testdata/testcert.pem", | ||
| o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ServerName: "abc"}}, | ||
| err: errors.New("redis: tls_cert_file and tls_key_file URL parameters must be both set or both omitted"), | ||
| }, { | ||
| url: "rediss://localhost:123?tls_key_file=./testdata/testkey.pem", | ||
| err: errors.New("redis: tls_cert_file and tls_key_file URL parameters must be both set or both omitted"), | ||
| }, { | ||
| url: "rediss://localhost:123?tls_min_version=1", | ||
| err: errors.New("redis: tls_min_version 1 is insecure (minimum allowed is TLS 1.2: 771)"), | ||
| }, { | ||
| url: "rediss://localhost:123?tls_max_version=1", | ||
| err: errors.New("redis: tls_max_version 1 is insecure (minimum allowed is TLS 1.2: 771)"), | ||
| }, { | ||
| url: "rediss://localhost:123?tls_min_version=70000", | ||
| err: errors.New("redis: invalid tls_min_version: 70000 (must be between 0 and 65535)"), | ||
| }, { | ||
| url: "rediss://localhost:123?tls_min_version=0", | ||
| o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ServerName: "localhost", MinVersion: tls.VersionTLS12}}, | ||
| }, { | ||
| url: "rediss://localhost:123/?skip_verify=true", | ||
| o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{InsecureSkipVerify: true}}, | ||
| o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ServerName: "localhost", MinVersion: tls.VersionTLS12, InsecureSkipVerify: true}}, | ||
| }, { | ||
| url: "redis://:bar@localhost:123", | ||
| o: &Options{Addr: "localhost:123", Password: "bar"}, | ||
|
|
@@ -197,6 +247,39 @@ func comprareOptions(t *testing.T, actual, expected *Options) { | |
| if actual.ConnMaxLifetime != expected.ConnMaxLifetime { | ||
| t.Errorf("ConnMaxLifetime: got %v, expected %v", actual.ConnMaxLifetime, expected.ConnMaxLifetime) | ||
| } | ||
|
|
||
| if (actual.TLSConfig == nil) != (expected.TLSConfig == nil) { | ||
| t.Errorf("TLSConfig nil: got %v, expected %v", actual.TLSConfig == nil, expected.TLSConfig == nil) | ||
| } | ||
|
|
||
| if (actual.TLSConfig != nil) && (expected.TLSConfig != nil) { | ||
| if actual.TLSConfig.MinVersion != expected.TLSConfig.MinVersion { | ||
| t.Errorf("TLSConfig.MinVersion: got %v, expected %v", actual.TLSConfig.MinVersion, expected.TLSConfig.MinVersion) | ||
| } | ||
|
|
||
| if actual.TLSConfig.MaxVersion != expected.TLSConfig.MaxVersion { | ||
| t.Errorf("TLSConfig.MaxVersion: got %v, expected %v", actual.TLSConfig.MaxVersion, expected.TLSConfig.MaxVersion) | ||
| } | ||
|
|
||
| if actual.TLSConfig.ServerName != expected.TLSConfig.ServerName { | ||
| t.Errorf("TLSConfig.ServerName: got %v, expected %v", actual.TLSConfig.ServerName, expected.TLSConfig.ServerName) | ||
| } | ||
|
|
||
| if actual.TLSConfig.InsecureSkipVerify != expected.TLSConfig.InsecureSkipVerify { | ||
| t.Errorf("TLSConfig.InsecureSkipVerify: got %v, expected %v", actual.TLSConfig.InsecureSkipVerify, expected.TLSConfig.InsecureSkipVerify) | ||
| } | ||
|
|
||
| if len(actual.TLSConfig.Certificates) != len(expected.TLSConfig.Certificates) { | ||
| t.Errorf("TLSConfig.Certificates: got %v, expected %v", actual.TLSConfig.Certificates, expected.TLSConfig.Certificates) | ||
| } | ||
|
|
||
| for i, actualCert := range actual.TLSConfig.Certificates { | ||
| expectedCert := expected.TLSConfig.Certificates[i] | ||
| if !actualCert.Leaf.Equal(expectedCert.Leaf) { | ||
| t.Errorf("TLSConfig.Certificates[%d].Leaf: got %v, expected %v", i, actual.TLSConfig.Certificates, expected.TLSConfig.Certificates) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Test ReadTimeout option initialization, including special values -1 and 0. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you think about getting rid of nested if statements?