Skip to content

Pass Client down the well known resolution #451

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

Closed
wants to merge 1 commit into from
+23 −10
Closed

Pass Client down the well known resolution #451

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion fclient/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@
// If the cache returned nothing then we'll have no results here,
// so go and hit the network.
if len(resolutionResults) == 0 {
resolutionResults, err = ResolveServer(r.Context(), serverName)
resolutionResults, err = ResolveServer(r.Context(), &NewClient(WithTransport(f.getTransport(string(serverName), f.dialer))).client, serverName)

Check warning on line 349 in fclient/client.go

View check run for this annotation

Codecov / codecov/patch

fclient/client.go#L349 

Added line #L349 was not covered by tests
if err != nil {
return nil, err
}
Expand Down
11 changes: 6 additions & 5 deletions fclient/resolve.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"context"
"fmt"
"net"
"net/http"
"strconv"

"github.com/matrix-org/gomatrixserverlib/spec"
Expand All @@ -37,14 +38,14 @@ type ResolutionResult struct {
// Returns a slice of ResolutionResult that can be used to send a federation
// request to the server using a given server name.
// Returns an error if the server name isn't valid.
func ResolveServer(ctx context.Context, serverName spec.ServerName) (results []ResolutionResult, err error) {
return resolveServer(ctx, serverName, true)
func ResolveServer(ctx context.Context, client *http.Client, serverName spec.ServerName) (results []ResolutionResult, err error) {
return resolveServer(ctx, client, serverName, true)
}

// resolveServer does the same thing as ResolveServer, except it also requires
// the checkWellKnown parameter, which indicates whether a .well-known file
// should be looked up.
func resolveServer(ctx context.Context, serverName spec.ServerName, checkWellKnown bool) (results []ResolutionResult, err error) {
func resolveServer(ctx context.Context, client *http.Client, serverName spec.ServerName, checkWellKnown bool) (results []ResolutionResult, err error) {
host, port, valid := spec.ParseAndValidateServerName(serverName)
if !valid {
err = fmt.Errorf("Invalid server name")
Expand Down Expand Up @@ -94,10 +95,10 @@ func resolveServer(ctx context.Context, serverName spec.ServerName, checkWellKno
if checkWellKnown {
// 3. If the hostname is not an IP literal
var result *WellKnownResult
result, err = LookupWellKnown(ctx, serverName)
result, err = LookupWellKnown(ctx, client, serverName)
if err == nil {
// We don't want to check .well-known on the result
return resolveServer(ctx, result.NewAddress, false)
return resolveServer(ctx, client, result.NewAddress, false)
}
}

Expand Down
17 changes: 15 additions & 2 deletions fclient/resolve_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"net"
"net/http"
"reflect"
"testing"

Expand All @@ -27,8 +28,8 @@ func assertCritical(t *testing.T, val, expected interface{}) {
// and expected certificate name.
// If one of them doesn't match, or the resolution function returned with an
// error, it aborts the current test.
func testResolve(t *testing.T, serverName spec.ServerName, destination, host, certName string) {
res, err := ResolveServer(context.Background(), serverName)
func testResolve(t *testing.T, client *http.Client, serverName spec.ServerName, destination, host, certName string) {
res, err := ResolveServer(context.Background(), client, serverName)
assertCritical(t, err, nil)
assertCritical(t, len(res), 1)
assertCritical(t, res[0].Destination, destination)
Expand All @@ -40,6 +41,7 @@ func testResolve(t *testing.T, serverName spec.ServerName, destination, host, ce
func TestResolutionIPLiteral(t *testing.T) {
testResolve(
t,
http.DefaultClient,
spec.ServerName("42.42.42.42"), // The server name is an IP literal without a port
"42.42.42.42:8448", // Destination must be the IP address + port 8448
"42.42.42.42", // Host must be the IP address
Expand All @@ -51,6 +53,7 @@ func TestResolutionIPLiteral(t *testing.T) {
func TestResolutionIPv6Literal(t *testing.T) {
testResolve(
t,
http.DefaultClient,
spec.ServerName("[42:42::42]"), // The server name is an IP literal without a port
"[42:42::42]:8448", // Destination must be the IP address + port 8448
"[42:42::42]", // Host must be the IP address
Expand All @@ -62,6 +65,7 @@ func TestResolutionIPv6Literal(t *testing.T) {
func TestResolutionIPLiteralWithPort(t *testing.T) {
testResolve(
t,
http.DefaultClient,
spec.ServerName("42.42.42.42:443"), // The server name is an IP literal with a port
"42.42.42.42:443", // Destination must be the IP address + port
"42.42.42.42:443", // Host must be the IP address + port
Expand All @@ -73,6 +77,7 @@ func TestResolutionIPLiteralWithPort(t *testing.T) {
func TestResolutionIPv6LiteralWithPort(t *testing.T) {
testResolve(
t,
http.DefaultClient,
spec.ServerName("[42:42::42]:443"), // The server name is an IP literal with a port
"[42:42::42]:443", // Destination must be the IP address + port
"[42:42::42]:443", // Host must be the IP address + port
Expand All @@ -84,6 +89,7 @@ func TestResolutionIPv6LiteralWithPort(t *testing.T) {
func TestResolutionHostnameAndPort(t *testing.T) {
testResolve(
t,
http.DefaultClient,
spec.ServerName("example.com:4242"), // The server name is not an IP literal and includes an explicit port
"example.com:4242", // Destination must be the hostname + port
"example.com:4242", // Host must be the hostname + port
Expand All @@ -102,6 +108,7 @@ func TestResolutionHostnameWellKnownWithIPLiteral(t *testing.T) {

testResolve(
t,
http.DefaultClient,
spec.ServerName("example.com"), // The server name is a domain hosting a .well-known file which specifies an IP literal without a port
"42.42.42.42:8448", // Destination must be the IP literal + port 8448
"42.42.42.42", // Host must be the IP literal
Expand All @@ -120,6 +127,7 @@ func TestResolutionHostnameWellKnownWithIPLiteralAndPort(t *testing.T) {

testResolve(
t,
http.DefaultClient,
spec.ServerName("example.com"), // The server name is a domain hosting a .well-known file which specifies an IP literal with a port
"42.42.42.42:443", // Destination must be the IP literal + port
"42.42.42.42:443", // Host must be the IP literal + port
Expand All @@ -138,6 +146,7 @@ func TestResolutionHostnameWellKnownWithHostnameAndPort(t *testing.T) {

testResolve(
t,
http.DefaultClient,
spec.ServerName("example.com"), // The server name is a domain hosting a .well-known file which specifies a hostname that's not an IP literal and has a port
"matrix.example.com:4242", // Destination must be the hostname + port
"matrix.example.com:4242", // Host must be the hostname + port
Expand All @@ -159,6 +168,7 @@ func TestResolutionHostnameWellKnownWithHostnameSRV(t *testing.T) {

testResolve(
t,
http.DefaultClient,
spec.ServerName("example.com"), // The server name is a domain hosting a .well-known file which specifies a hostname that's not an IP literal, has no port and for which a SRV record with a non-0 exists
"matrix.otherexample.com:4242", // Destination must be the hostname + port from the SRV record
"matrix.example.com", // Host must be the delegated hostname
Expand All @@ -180,6 +190,7 @@ func TestResolutionHostnameWellKnownWithHostnameNoSRV(t *testing.T) {

testResolve(
t,
http.DefaultClient,
spec.ServerName("example.com"), // The server name is a domain hosting a .well-known file which specifies a hostname that's not an IP literal, has no port and for which no SRV record exists
"matrix.example.com:8448", // Destination must be the delegated hostname + port 8448
"matrix.example.com", // Host must be the delegated hostname
Expand All @@ -194,6 +205,7 @@ func TestResolutionHostnameWithSRV(t *testing.T) {

testResolve(
t,
http.DefaultClient,
spec.ServerName("example.com"), // The server name is a domain for which a SRV record exists with a non-0 port
"matrix.otherexample.com:4242", // Destination must be the hostname + port
"example.com", // Host must be the server name
Expand All @@ -214,6 +226,7 @@ func TestResolutionHostnameWithNoWellKnownNorSRV(t *testing.T) {

testResolve(
t,
http.DefaultClient,
spec.ServerName("example.com"), // The server name is a domain for no .well-known file nor SRV record exist
"example.com:8448", // Destination must be the hostname + 8448
"example.com", // Host must be the server name
Expand Down
3 changes: 1 addition & 2 deletions fclient/well_known.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ type WellKnownResult struct {

// LookupWellKnown looks up a well-known record for a matrix server. If one if
// found, it returns the server to redirect to.
func LookupWellKnown(ctx context.Context, serverNameType spec.ServerName) (*WellKnownResult, error) {
func LookupWellKnown(ctx context.Context, client *http.Client, serverNameType spec.ServerName) (*WellKnownResult, error) {
serverName := string(serverNameType)

// Handle ending "/"
Expand All @@ -43,7 +43,6 @@ func LookupWellKnown(ctx context.Context, serverNameType spec.ServerName) (*Well
return nil, err
}
// Given well-known should be quite small and fast to fetch, timeout the request after 30s.
client := http.Client{Timeout: time.Second * 30}
resp, err := client.Do(req)
if err != nil {
return nil, err
Expand Down
Loading