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.

Sign up for GitHub

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

Jump to bottom

fix(test): wrong url used in expired certificate test #242

Merged
merged 4 commits into from
May 29, 2024
Merged

fix(test): wrong url used in expired certificate test #242

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8dc8ce7
fix(test): wrong url used in expired certificate test
jyyi1 May 28, 2024
4af83f7
add back revoke test case
jyyi1 May 29, 2024
94a8957
Skip revocation case in Linux
jyyi1 May 29, 2024
26194b7
skip revocation test in Windows as well
jyyi1 May 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions transport/tls/stream_dialer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package tls
import (
"context"
"crypto/x509"
"runtime"
"testing"

"github.com/Jigsaw-Code/outline-sdk/transport"
Expand Down Expand Up @@ -44,15 +45,29 @@ func TestUntrustedRoot(t *testing.T) {
require.ErrorAs(t, err, &certErr)
}

func TestRevoked(t *testing.T) {
func TestExpired(t *testing.T) {
sd, err := NewStreamDialer(&transport.TCPDialer{})
require.NoError(t, err)
_, err = sd.DialStream(context.Background(), "revoked.badssl.com:443")
Copy link
Contributor

Choose a reason for hiding this comment

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

I explicitly wanted to test for revoked here. I don't think the issue is with the original test. It seems like revoked.badssl.com no longer serves an expired cert, so we don't return an error. Chrome is considering it safe.
image

Can you instead comment it out or use t.Skip("Skipping until revoked.badssl.com is fixed") in addition to adding the expired test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed offline, will keep the revoke test, but skip two platforms that won't throw errors.

_, err = sd.DialStream(context.Background(), "expired.badssl.com:443")
var certErr x509.CertificateInvalidError
require.ErrorAs(t, err, &certErr)
require.Equal(t, x509.Expired, certErr.Reason)
}

func TestRevoked(t *testing.T) {
if runtime.GOOS == "linux" || runtime.GOOS == "windows" {
t.Skip("Certificate revocation list is not up-to-date in Linux and Windows")
}

sd, err := NewStreamDialer(&transport.TCPDialer{})
require.NoError(t, err)
conn, err := sd.DialStream(context.Background(), "revoked.badssl.com:443")

// Revocation error is thrown by the system API and is not strongly typed
require.ErrorContains(t, err, "certificate is revoked")
require.Nil(t, conn)
}

func TestIP(t *testing.T) {
sd, err := NewStreamDialer(&transport.TCPDialer{})
require.NoError(t, err)
Expand Down
Loading