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

Test: remove part of the hardcoded network ports #206

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
67 changes: 24 additions & 43 deletions internal/sessiontest/helper_servers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@ import (
"encoding/json"
"fmt"
"io/ioutil"
"net"
"net/http"
"net/http/httptest"
"path/filepath"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -37,22 +40,18 @@ var (
jwtPrivkeyPath = filepath.Join(testdata, "jwtkeys", "sk.pem")
)

// Some urls are hardcoded in the test configuration, so we have to hardcode them here too.
const (
irmaServerPort = 48680

schemeServerURL = "http://localhost:48681"

requestorServerPort = 48682
requestorServerURL = "http://localhost:48682"

revocationServerPort = 48683
revocationServerURL = "http://localhost:48683"
)

staticSessionServerPort = 48685
staticSessionServerURL = "http://localhost:48685"

nextSessionServerPort = 48686
nextSessionServerURL = "http://localhost:48686"
// The doSession helper expects the requestor server URL to be globally defined, to support the optionReuseServer.
var (
requestorServerPort = findFreePort()
requestorServerURL = fmt.Sprintf("http://localhost:%d", requestorServerPort)
)

type IrmaServer struct {
Expand Down Expand Up @@ -87,6 +86,12 @@ func apply(
}
}

func findFreePort() int {
s := httptest.NewUnstartedServer(http.NotFoundHandler())
defer s.Close()
return s.Listener.Addr().(*net.TCPAddr).Port
}

func StartRequestorServer(t *testing.T, configuration *requestorserver.Configuration) *requestorserver.Server {
requestorServer, err := requestorserver.New(configuration)
require.NoError(t, err)
Expand All @@ -103,19 +108,19 @@ func StartIrmaServer(t *testing.T, conf *server.Configuration) *IrmaServer {
conf = IrmaServerConfiguration()
}

mux := http.NewServeMux()
httpServer := httptest.NewServer(mux)

// Make sure domain is used instead of IP address.
conf.URL = strings.Replace(httpServer.URL, "127.0.0.1", "localhost", 1)
Copy link
Member

Choose a reason for hiding this comment

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

Why is this necessary? If it is because IPv6 might be used, it would be helpful to add that to the comment above.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I didn't think of IPv6. The httptest library gives us an IP address instead of a domain. This causes the requestor name test to fail, because this one relies on the domain being localhost. I'm not sure whether we can rewrite that test in such a way that it expects an IP address. If that's not possible, we can use url.Parse to actually replace the host part with localhost. Then it should work in all cases.

Copy link
Member Author

@ivard ivard Feb 2, 2022

Choose a reason for hiding this comment

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

Switching the domain in the test-requestors scheme to an IP address does not seem to be a very smart solution, because then we hardcode the use of IPv4 even more. I'd say let's do it this way then:

conf.URL = fmt.Sprintf("http://localhost:%d", portFromTestServer(httpServer))

With portFromTestServer being:

func portFromTestServer(s *httptest.Server) int {
	return s.Listener.Addr().(*net.TCPAddr).Port
}

This function can also be used in the function findFreePort to prevent code duplication. The findFreePort function is a bit of a hack for the RequestorServer now. Ideally, that function shouldn't be needed.

irmaServer, err := irmaserver.New(conf)
require.NoError(t, err)

mux := http.NewServeMux()
mux.HandleFunc("/", irmaServer.HandlerFunc())
httpServer := &http.Server{Addr: fmt.Sprintf("localhost:%d", irmaServerPort), Handler: mux}
go func() {
_ = httpServer.ListenAndServe()
}()
return &IrmaServer{
irma: irmaServer,
conf: conf,
http: httpServer,
http: httpServer.Config,
}
}

Expand All @@ -137,7 +142,7 @@ func chainedServerHandler(t *testing.T, jwtPubKey *rsa.PublicKey) http.Handler {
request := &irma.ServiceProviderRequest{
Request: getDisclosureRequest(id),
RequestorBaseRequest: irma.RequestorBaseRequest{
NextSession: &irma.NextSessionData{URL: nextSessionServerURL + "/2"},
NextSession: &irma.NextSessionData{URL: fmt.Sprintf("http://%s/2", r.Host)},
},
}
bts, err := json.Marshal(request)
Expand Down Expand Up @@ -180,7 +185,7 @@ func chainedServerHandler(t *testing.T, jwtPubKey *rsa.PublicKey) http.Handler {
bts, err = json.Marshal(irma.IdentityProviderRequest{
Request: irma.NewIssuanceRequest([]*irma.CredentialRequest{cred}),
RequestorBaseRequest: irma.RequestorBaseRequest{
NextSession: &irma.NextSessionData{URL: nextSessionServerURL + "/3"},
NextSession: &irma.NextSessionData{URL: fmt.Sprintf("http://%s/3", r.Host)},
},
})
require.NoError(t, err)
Expand All @@ -206,20 +211,8 @@ func chainedServerHandler(t *testing.T, jwtPubKey *rsa.PublicKey) http.Handler {
return mux
}

func StartNextRequestServer(t *testing.T, jwtPubKey *rsa.PublicKey) *http.Server {
s := &http.Server{
Addr: fmt.Sprintf("localhost:%d", nextSessionServerPort),
Handler: chainedServerHandler(t, jwtPubKey),
}
go func() {
_ = s.ListenAndServe()
}()
return s
}

func IrmaServerConfiguration() *server.Configuration {
return &server.Configuration{
URL: fmt.Sprintf("http://localhost:%d", irmaServerPort),
Logger: logger,
DisableSchemesUpdate: true,
SchemesPath: filepath.Join(testdata, "irma_configuration"),
Expand All @@ -229,19 +222,7 @@ func IrmaServerConfiguration() *server.Configuration {
revKeyshareTestCred: {RevocationServerURL: revocationServerURL},
},
JwtPrivateKeyFile: jwtPrivkeyPath,
StaticSessions: map[string]interface{}{
"staticsession": irma.ServiceProviderRequest{
RequestorBaseRequest: irma.RequestorBaseRequest{
CallbackURL: staticSessionServerURL,
},
Request: &irma.DisclosureRequest{
BaseRequest: irma.BaseRequest{LDContext: irma.LDContextDisclosureRequest},
Disclose: irma.AttributeConDisCon{
{{irma.NewAttributeRequest("irma-demo.RU.studentCard.level")}},
},
},
},
},
StaticSessions: map[string]interface{}{},
ivard marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
10 changes: 6 additions & 4 deletions internal/sessiontest/redis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,14 +240,16 @@ func TestRedisRedundancy(t *testing.T) {
mr, cert := startRedis(t, true)
defer mr.Close()

ports := []int{48690, 48691, 48692}
ports := make([]int, 3)
servers := make([]*requestorserver.Server, len(ports))

for i, port := range ports {
for i := range ports {
port := findFreePort()
c := redisRequestorConfigDecorator(mr, cert, "", RequestorServerAuthConfiguration)()
c.Configuration.URL = fmt.Sprintf("http://localhost:%d/irma", port)
// Make sure URL of load balancer is being used in QRs.
c.URL = requestorServerURL
Copy link
Member

Choose a reason for hiding this comment

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

This changes the test behaviour itself, instead of just some ports. Strange that it wasn't already like this.

Copy link
Member Author

Choose a reason for hiding this comment

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

True, it was a mistake in the test I think. We overlooked that one. Luckily, it works like a charm, so it only was a testing mistake and not a functional mistake.

c.Port = port
rs := StartRequestorServer(t, c)
ports[i] = port
servers[i] = rs
}
lb := startLoadBalancer(t, ports)
Expand Down
37 changes: 25 additions & 12 deletions internal/sessiontest/session_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"io/ioutil"
"net/http"
"net/http/httptest"
"os"
"path/filepath"
"reflect"
Expand Down Expand Up @@ -380,17 +381,31 @@ func testDisclosureNewAttributeUpdateSchemeManager(t *testing.T, conf interface{
func testStaticQRSession(t *testing.T, _ interface{}, opts ...option) {
client, handler := parseStorage(t, opts...)
defer test.ClearTestStorage(t, handler.storage)
rs := StartRequestorServer(t, RequestorServerAuthConfiguration())
defer rs.Stop()

// start server to receive session result callback after the session
var received bool
mux := http.NewServeMux()
mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
received = true
})
s := &http.Server{Addr: fmt.Sprintf("localhost:%d", staticSessionServerPort), Handler: mux}
go func() { _ = s.ListenAndServe() }()
staticSessionServer := httptest.NewServer(mux)
defer staticSessionServer.Close()

config := RequestorServerAuthConfiguration()
config.StaticSessions["staticsession"] = irma.ServiceProviderRequest{
RequestorBaseRequest: irma.RequestorBaseRequest{
CallbackURL: staticSessionServer.URL,
},
Request: &irma.DisclosureRequest{
BaseRequest: irma.BaseRequest{LDContext: irma.LDContextDisclosureRequest},
Disclose: irma.AttributeConDisCon{
{{irma.NewAttributeRequest("irma-demo.RU.studentCard.level")}},
},
},
}

rs := StartRequestorServer(t, config)
defer rs.Stop()

// setup static QR and other variables
qr := &irma.Qr{
Expand All @@ -410,7 +425,6 @@ func testStaticQRSession(t *testing.T, _ interface{}, opts ...option) {

// give irma server time to post session result to the server started above, and check the call was received
time.Sleep(200 * time.Millisecond)
require.NoError(t, s.Shutdown(context.Background()))
require.True(t, received)
}

Expand Down Expand Up @@ -511,13 +525,11 @@ func testChainedSessions(t *testing.T, conf interface{}, opts ...option) {
require.IsType(t, IrmaServerConfiguration, conf)
irmaServer := StartIrmaServer(t, conf.(func() *server.Configuration)())
defer irmaServer.Stop()
nextServer := StartNextRequestServer(t, &irmaServer.conf.JwtRSAPrivateKey.PublicKey)
defer func() {
_ = nextServer.Close()
}()
nextServer := httptest.NewServer(chainedServerHandler(t, &irmaServer.conf.JwtRSAPrivateKey.PublicKey))
defer nextServer.Close()

var request irma.ServiceProviderRequest
require.NoError(t, irma.NewHTTPTransport(nextSessionServerURL, false).Get("1", &request))
require.NoError(t, irma.NewHTTPTransport(nextServer.URL, false).Get("1", &request))
doSession(t, &request, client, irmaServer, nil, nil, nil)

// check that our credential instance is new
Expand Down Expand Up @@ -947,7 +959,8 @@ func TestDisclosureNonexistingCredTypeUpdateSchemeManager(t *testing.T) {
}

func TestPOSTSizeLimit(t *testing.T) {
rs := StartRequestorServer(t, RequestorServerConfiguration())
config := RequestorServerConfiguration()
rs := StartRequestorServer(t, config)
defer rs.Stop()

server.PostSizeLimit = 1 << 10
Expand All @@ -957,7 +970,7 @@ func TestPOSTSizeLimit(t *testing.T) {

req, err := http.NewRequest(
http.MethodPost,
requestorServerURL+"/session/",
fmt.Sprintf("http://localhost:%d/session/", config.Port),
bytes.NewReader(make([]byte, server.PostSizeLimit+1, server.PostSizeLimit+1)),
)
require.NoError(t, err)
Expand Down
41 changes: 22 additions & 19 deletions internal/test/testdata.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"io/ioutil"
"net/http"
"net/http/httptest"
"os"
"path/filepath"
"testing"
Expand All @@ -28,8 +29,6 @@ func checkError(t *testing.T, err error) {
}

var schemeServer *http.Server
var badServer *http.Server
var badServerCount int
var testStorageDir = "client"

func StartSchemeManagerHttpServer() {
Expand All @@ -45,26 +44,30 @@ func StopSchemeManagerHttpServer() {
_ = schemeServer.Close()
}

// StartBadHttpServer starts an HTTP server that times out and returns 500 on the first few times.
func StartBadHttpServer(count int, timeout time.Duration, success string) {
badServer = &http.Server{Addr: "localhost:48682", Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if badServerCount >= count {
_, _ = fmt.Fprintln(w, success)
return
} else {
badServerCount++
time.Sleep(timeout)
}
})}
type BadServer struct {
count int
success string
timeout time.Duration
}

go func() {
_ = badServer.ListenAndServe()
}()
time.Sleep(100 * time.Millisecond) // Give server time to start
func (s *BadServer) ServeHTTP(w http.ResponseWriter, _ *http.Request) {
if s.count <= 0 {
_, _ = fmt.Fprintln(w, s.success)
return
} else {
s.count--
time.Sleep(s.timeout)
}
}

func StopBadHttpServer() {
_ = badServer.Close()
// StartBadHttpServer starts an HTTP server that times out and returns 500 on the first few times.
func StartBadHttpServer(count int, timeout time.Duration, success string) *httptest.Server {
s := &BadServer{
count: count,
timeout: timeout,
success: success,
}
return httptest.NewServer(s)
}

// FindTestdataFolder finds the "testdata" folder which is in . or ..
Expand Down
13 changes: 10 additions & 3 deletions irmago_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,18 @@ func TestParseInvalidIrmaConfiguration(t *testing.T) {
}

func TestRetryHTTPRequest(t *testing.T) {
test.StartBadHttpServer(2, 1*time.Second, "42")
defer test.StopBadHttpServer()
// Make sure that first 5 requests fail.
badServer := test.StartBadHttpServer(5, 1*time.Second, "42")
defer badServer.Close()

transport := NewHTTPTransport("http://localhost:48682", false)
transport := NewHTTPTransport(badServer.URL, false)
transport.client.HTTPClient.Timeout = 500 * time.Millisecond

// Retryable HTTP tries 3 times, so this attempt should fail.
_, err := transport.GetBytes("")
require.Error(t, err)

// The 6th request should succeed.
bts, err := transport.GetBytes("")
require.NoError(t, err)
require.Equal(t, "42\n", string(bts))
Expand Down
29 changes: 8 additions & 21 deletions server/api_test.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
package server

import (
"context"
"encoding/json"
"fmt"
"io"
"io/ioutil"
"net/http"
"net/http/httptest"
"testing"
"time"

Expand Down Expand Up @@ -130,12 +130,12 @@ func TestServerTimeouts(t *testing.T) {
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
// start server
s := startServer(t, test.handler, test.readTimeout)
defer stopServer(t, s)
s := startServer(test.handler, test.readTimeout)
defer s.Close()

// do request
called = false
req, err := http.NewRequest(http.MethodPost, "http://localhost:34534", test.body)
req, err := http.NewRequest(http.MethodPost, s.URL, test.body)
require.NoError(t, err)
start := time.Now()
res, err := http.DefaultClient.Do(req)
Expand All @@ -149,22 +149,9 @@ func TestServerTimeouts(t *testing.T) {
}
}

func startServer(t *testing.T, handler http.Handler, timeout time.Duration) *http.Server {
s := &http.Server{
Addr: "localhost:34534",
Handler: handler,
ReadTimeout: timeout,
}
go func() {
err := s.ListenAndServe()
require.Equal(t, http.ErrServerClosed, err)
}()
time.Sleep(50 * time.Millisecond) // give server time to start
func startServer(handler http.Handler, timeout time.Duration) *httptest.Server {
s := httptest.NewUnstartedServer(handler)
s.Config.ReadTimeout = timeout
s.Start()
return s
}

func stopServer(t *testing.T, server *http.Server) {
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
require.NoError(t, server.Shutdown(ctx))
cancel()
}