add tests for _matrix/client/v1/media/download

client/client.go

@@ -101,6 +101,19 @@ func (c *CSAPI) DownloadContent(t ct.TestLike, mxcUri string) ([]byte, string) {
 	return b, contentType
 }
 
+// DownloadContentV2 downloads media from _matrix/client/v1/media resource, returning the raw bytes and the Content-Type. Fails the test on error.
+func (c *CSAPI) DownloadContentV2(t ct.TestLike, mxcUri string) ([]byte, string) {
+	t.Helper()
+	origin, mediaId := SplitMxc(mxcUri)
+	res := c.MustDo(t, "GET", []string{"_matrix", "client", "v1", "media", "download", origin, mediaId})
+	contentType := res.Header.Get("Content-Type")
+	b, err := io.ReadAll(res.Body)
+	if err != nil {
+		ct.Errorf(t, err.Error())
+	}
+	return b, contentType
+}
+
 // MustCreateRoom creates a room with an optional HTTP request body. Fails the test on error. Returns the room ID.
 func (c *CSAPI) MustCreateRoom(t ct.TestLike, reqBody map[string]interface{}) string {
 	t.Helper()

tests/csapi/apidoc_content_test.go

@@ -27,3 +27,23 @@ func TestContent(t *testing.T) {
 		t.Fatalf("expected contentType to be %s, got %s", wantContentType, contentType)
 	}
 }
+
+// same as above but testing _matrix/client/v1/media/download
+func TestContentCSAPIMediaV1(t *testing.T) {
+	deployment := complement.Deploy(t, 2)
+	defer deployment.Destroy(t)
+
+	hs1 := deployment.Register(t, "hs1", helpers.RegistrationOpts{})
+	hs2 := deployment.Register(t, "hs2", helpers.RegistrationOpts{})
+
+	wantContentType := "img/png"
+	mxcUri := hs1.UploadContent(t, data.MatrixPng, "test.png", wantContentType)
+
+	content, contentType := hs2.DownloadContentV2(t, mxcUri)
+	if !bytes.Equal(data.MatrixPng, content) {
+		t.Fatalf("uploaded and downloaded content doesn't match: want %v\ngot\n%v", data.MatrixPng, content)
+	}
+	if contentType != wantContentType {
+		t.Fatalf("expected contentType to be \n %s, got \n %s", wantContentType, contentType)
+	}
+}

tests/csapi/media_async_uploads_test.go

@@ -70,4 +70,14 @@ func TestAsyncUpload(t *testing.T) {
 			t.Fatalf("expected contentType to be %s, got %s", wantContentType, contentType)
 		}
 	})
+
+	t.Run("Download media over _matrix/client/v1/media/download", func(t *testing.T) {
+		content, contentType := alice.DownloadContentV2(t, mxcURI)
+		if !bytes.Equal(data.MatrixPng, content) {
+			t.Fatalf("uploaded and downloaded content doesn't match: want %v\ngot\n%v", data.MatrixPng, content)
+		}
+		if contentType != wantContentType {
+			t.Fatalf("expected contentType to be %s, got %s", wantContentType, contentType)
+		}
+	})
 }

tests/media_filename_test.go

@@ -45,7 +45,20 @@ func TestMediaFilenames(t *testing.T) {
 
 					mxcUri := alice.UploadContent(t, data.MatrixPng, filename, "image/png")
 
-					name, _ := downloadForFilename(t, alice, mxcUri, "")
+					name, _ := downloadForFilename(t, alice, mxcUri, "", false)
+
+					// filename is not required, but if it's an attachment then check it matches
+					if name != filename {
+						t.Fatalf("Incorrect filename '%s', expected '%s'", name, filename)
+					}
+				})
+
+				t.Run(fmt.Sprintf("Can download file '%s' over /_matrix/client/v1/media/download", filename), func(t *testing.T) {
+					t.Parallel()
+
+					mxcUri := alice.UploadContent(t, data.MatrixPng, filename, "image/png")
+
+					name, _ := downloadForFilename(t, alice, mxcUri, "", true)
 
 					// filename is not required, but if it's an attachment then check it matches
 					if name != filename {
@@ -61,12 +74,25 @@ func TestMediaFilenames(t *testing.T) {
 				mxcUri := alice.UploadContent(t, data.MatrixPng, asciiFileName, "image/png")
 
 				const altName = "file.png"
-				filename, _ := downloadForFilename(t, alice, mxcUri, altName)
+				filename, _ := downloadForFilename(t, alice, mxcUri, altName, false)
 
 				if filename != altName {
 					t.Fatalf("filename did not match, expected '%s', got '%s'", altName, filename)
 				}
 			})
+			t.Run("Can download specifying a different ASCII file name over _matrix/client/v1/media/download", func(t *testing.T) {
+				t.Parallel()
+
+				mxcUri := alice.UploadContent(t, data.MatrixPng, asciiFileName, "image/png")
+
+				const altName = "file.png"
+				filename, _ := downloadForFilename(t, alice, mxcUri, altName, true)
+
+				if filename != altName {
+					t.Fatalf("filename did not match, expected '%s', got '%s'", altName, filename)
+				}
+			})
+
 		})
 
 		t.Run("Unicode", func(t *testing.T) {
@@ -87,7 +113,21 @@ func TestMediaFilenames(t *testing.T) {
 
 				const diffUnicodeFilename = "\u2615" // coffee emoji
 
-				filename, _ := downloadForFilename(t, alice, mxcUri, diffUnicodeFilename)
+				filename, _ := downloadForFilename(t, alice, mxcUri, diffUnicodeFilename, false)
+
+				if filename != diffUnicodeFilename {
+					t.Fatalf("filename did not match, expected '%s', got '%s'", diffUnicodeFilename, filename)
+				}
+			})
+
+			t.Run("Can download specifying a different Unicode file name over _matrix/client/v1/media/download", func(t *testing.T) {
+				t.Parallel()
+
+				mxcUri := alice.UploadContent(t, data.MatrixPng, unicodeFileName, "image/png")
+
+				const diffUnicodeFilename = "\u2615" // coffee emoji
+
+				filename, _ := downloadForFilename(t, alice, mxcUri, diffUnicodeFilename, true)
 
 				if filename != diffUnicodeFilename {
 					t.Fatalf("filename did not match, expected '%s', got '%s'", diffUnicodeFilename, filename)
@@ -100,7 +140,19 @@ func TestMediaFilenames(t *testing.T) {
 
 				mxcUri := alice.UploadContent(t, data.MatrixPng, unicodeFileName, "image/png")
 
-				filename, _ := downloadForFilename(t, alice, mxcUri, "")
+				filename, _ := downloadForFilename(t, alice, mxcUri, "", false)
+
+				if filename != unicodeFileName {
+					t.Fatalf("filename did not match, expected '%s', got '%s'", unicodeFileName, filename)
+				}
+			})
+
+			t.Run("Can download with Unicode file name locally over _matrix/client/v1/media/download", func(t *testing.T) {
+				t.Parallel()
+
+				mxcUri := alice.UploadContent(t, data.MatrixPng, unicodeFileName, "image/png")
+
+				filename, _ := downloadForFilename(t, alice, mxcUri, "", true)
 
 				if filename != unicodeFileName {
 					t.Fatalf("filename did not match, expected '%s', got '%s'", unicodeFileName, filename)
@@ -113,7 +165,19 @@ func TestMediaFilenames(t *testing.T) {
 
 				mxcUri := alice.UploadContent(t, data.MatrixPng, unicodeFileName, "image/png")
 
-				filename, _ := downloadForFilename(t, bob, mxcUri, "")
+				filename, _ := downloadForFilename(t, bob, mxcUri, "", false)
+
+				if filename != unicodeFileName {
+					t.Fatalf("filename did not match, expected '%s', got '%s'", unicodeFileName, filename)
+				}
+			})
+
+			t.Run("Can download with Unicode file name over federation via _matrix/client/v1/media/download", func(t *testing.T) {
+				t.Parallel()
+
+				mxcUri := alice.UploadContent(t, data.MatrixPng, unicodeFileName, "image/png")
+
+				filename, _ := downloadForFilename(t, bob, mxcUri, "", true)
 
 				if filename != unicodeFileName {
 					t.Fatalf("filename did not match, expected '%s', got '%s'", unicodeFileName, filename)
@@ -131,7 +195,25 @@ func TestMediaFilenames(t *testing.T) {
 
 				mxcUri := alice.UploadContent(t, data.MatrixPng, "", "image/png")
 
-				_, isAttachment := downloadForFilename(t, bob, mxcUri, "")
+				_, isAttachment := downloadForFilename(t, bob, mxcUri, "", false)
+
+				if isAttachment {
+					t.Fatal("Expected file to be served as inline")
+				}
+			})
+
+			t.Run("Will serve safe media types as inline via _matrix/client/v1/media/download", func(t *testing.T) {
+				if runtime.Homeserver != runtime.Synapse && runtime.Homeserver != runtime.Conduwuit {
+					// We need to check that this security behaviour is being correctly run in
+					// Synapse or conduwuit, but since this is not part of the Matrix spec we do not assume
+					// other homeservers are doing so.
+					t.Skip("Skipping test of Content-Disposition header requirements on non-Synapse and non-conduwuit homeserver")
+				}
+				t.Parallel()
+
+				mxcUri := alice.UploadContent(t, data.MatrixPng, "", "image/png")
+
+				_, isAttachment := downloadForFilename(t, bob, mxcUri, "", true)
 
 				if isAttachment {
 					t.Fatal("Expected file to be served as inline")
@@ -150,7 +232,26 @@ func TestMediaFilenames(t *testing.T) {
 				// Add parameters and upper-case, which should be parsed as text/plain.
 				mxcUri := alice.UploadContent(t, data.MatrixPng, "", "Text/Plain; charset=utf-8")
 
-				_, isAttachment := downloadForFilename(t, bob, mxcUri, "")
+				_, isAttachment := downloadForFilename(t, bob, mxcUri, "", false)
+
+				if isAttachment {
+					t.Fatal("Expected file to be served as inline")
+				}
+			})
+
+			t.Run("Will serve safe media types with parameters as inline via _matrix/client/v1/media/download", func(t *testing.T) {
+				if runtime.Homeserver != runtime.Synapse && runtime.Homeserver != runtime.Conduwuit {
+					// We need to check that this security behaviour is being correctly run in
+					// Synapse or conduwuit, but since this is not part of the Matrix spec we do not assume
+					// other homeservers are doing so.
+					t.Skip("Skipping test of Content-Disposition header requirements on non-Synapse and non-conduwuit homeserver")
+				}
+				t.Parallel()
+
+				// Add parameters and upper-case, which should be parsed as text/plain.
+				mxcUri := alice.UploadContent(t, data.MatrixPng, "", "Text/Plain; charset=utf-8")
+
+				_, isAttachment := downloadForFilename(t, bob, mxcUri, "", true)
 
 				if isAttachment {
 					t.Fatal("Expected file to be served as inline")
@@ -168,7 +269,25 @@ func TestMediaFilenames(t *testing.T) {
 
 				mxcUri := alice.UploadContent(t, data.MatrixSvg, "", "image/svg")
 
-				_, isAttachment := downloadForFilename(t, bob, mxcUri, "")
+				_, isAttachment := downloadForFilename(t, bob, mxcUri, "", false)
+
+				if !isAttachment {
+					t.Fatal("Expected file to be served as an attachment")
+				}
+			})
+
+			t.Run("Will serve unsafe media types as attachments via _matrix/client/v1/media/download", func(t *testing.T) {
+				if runtime.Homeserver != runtime.Synapse && runtime.Homeserver != runtime.Conduwuit {
+					// We need to check that this security behaviour is being correctly run in
+					// Synapse or conduwuit, but since this is not part of the Matrix spec we do not assume
+					// other homeservers are doing so.
+					t.Skip("Skipping test of Content-Disposition header requirements on non-Synapse and non-conduwuit homeserver")
+				}
+				t.Parallel()
+
+				mxcUri := alice.UploadContent(t, data.MatrixSvg, "", "image/svg")
+
+				_, isAttachment := downloadForFilename(t, bob, mxcUri, "", true)
 
 				if !isAttachment {
 					t.Fatal("Expected file to be served as an attachment")
@@ -179,17 +298,25 @@ func TestMediaFilenames(t *testing.T) {
 }
 
 // Returns content disposition information like (filename, isAttachment)
-func downloadForFilename(t *testing.T, c *client.CSAPI, mxcUri string, diffName string) (filename string, isAttachment bool) {
+func downloadForFilename(t *testing.T, c *client.CSAPI, mxcUri string, diffName string, newPath bool) (filename string, isAttachment bool) {
 	t.Helper()
 
 	origin, mediaId := client.SplitMxc(mxcUri)
 
 	var path []string
 
 	if diffName != "" {
-		path = []string{"_matrix", "media", "v3", "download", origin, mediaId, diffName}
+		if newPath {
+			path = []string{"_matrix", "client", "v1", "media", "download", origin, mediaId, diffName}
+		} else {
+			path = []string{"_matrix", "media", "v3", "download", origin, mediaId, diffName}
+		}
 	} else {
-		path = []string{"_matrix", "media", "v3", "download", origin, mediaId}
+		if newPath {
+			path = []string{"_matrix", "client", "v1", "media", "download", origin, mediaId}
+		} else {
+			path = []string{"_matrix", "media", "v3", "download", origin, mediaId}
+		}
 	}
 
 	res := c.MustDo(t, "GET", path)

tests/media_nofilename_test.go

@@ -7,8 +7,8 @@ import (
 	"testing"
 
 	"github.com/matrix-org/complement"
-	"github.com/matrix-org/complement/helpers"
 	"github.com/matrix-org/complement/federation"
+	"github.com/matrix-org/complement/helpers"
 	"github.com/matrix-org/complement/must"
 )
 
@@ -96,3 +96,77 @@ func TestMediaWithoutFileName(t *testing.T) {
 		})
 	})
 }
+
+// same test as above, but for the new _matrix/client/v1/media endpoint
+func TestMediaWithoutFileNameCSMediaV1(t *testing.T) {
+	deployment := complement.Deploy(t, 2)
+	defer deployment.Destroy(t)
+
+	hs1 := deployment.Register(t, "hs1", helpers.RegistrationOpts{})
+	hs2 := deployment.Register(t, "hs2", helpers.RegistrationOpts{})
+
+	file := []byte("Hello World!")
+	fileName := ""
+	contentType := "text/plain"
+
+	t.Run("parallel", func(t *testing.T) {
+		// sytest: Can upload without a file name
+		t.Run("Can upload without a file name", func(t *testing.T) {
+			t.Parallel()
+			mxc := hs1.UploadContent(t, file, fileName, contentType)
+			must.NotEqual(t, mxc, "", "did not return an MXC URI")
+			must.StartWithStr(t, mxc, "mxc://", "returned invalid MXC URI")
+		})
+		// sytest: Can download without a file name locally
+		t.Run("Can download without a file name locally", func(t *testing.T) {
+			t.Parallel()
+			mxc := hs1.UploadContent(t, file, fileName, contentType)
+			must.NotEqual(t, mxc, "", "did not return an MXC URI")
+			must.StartWithStr(t, mxc, "mxc://", "returned invalid MXC URI")
+
+			b, ct := hs1.DownloadContentV2(t, mxc)
+
+			// Check the Content-Type response header.
+			// NOTSPEC: There is ambiguity over whether the homeserver is allowed to change the
+			// Content-Type header here from what is sent by the client during upload. Synapse does
+			// so, and there are benefits to doing so, but the spec needs to pick a side.
+			// For now, we're operating under the assumption that homeservers are free to add other
+			// directives. All we're going to check is the mime-type.
+			mimeType := strings.Split(ct, ";")[0]
+			must.Equal(
+				t, mimeType, contentType,
+				fmt.Sprintf(
+					"Wrong mime-type returned in Content-Type returned. got Content-Type '%s', extracted mime-type '%s', expected mime-type: '%s'",
+					ct, mimeType, contentType,
+				),
+			)
+			must.Equal(t, string(b), string(file), "wrong file content returned")
+		})
+		// sytest: Can download without a file name over federation
+		t.Run("Can download without a file name over federation", func(t *testing.T) {
+			t.Parallel()
+
+			mxc := hs1.UploadContent(t, file, fileName, contentType)
+			must.NotEqual(t, mxc, "", "did not return an MXC URI")
+			must.StartWithStr(t, mxc, "mxc://", "returned invalid MXC URI")
+
+			b, ct := hs2.DownloadContentV2(t, mxc)
+
+			// Check the Content-Type response header.
+			// NOTSPEC: There is ambiguity over whether the homeserver is allowed to change the
+			// Content-Type header here from what is sent by the client during upload. Synapse does
+			// so, and there are benefits to doing so, but the spec needs to pick a side.
+			// For now, we're operating under the assumption that homeservers are free to add other
+			// directives. All we're going to check is the mime-type.
+			mimeType := strings.Split(ct, ";")[0]
+			must.Equal(
+				t, mimeType, contentType,
+				fmt.Sprintf(
+					"Wrong mime-type returned in Content-Type returned. got Content-Type '%s', extracted mime-type '%s', expected mime-type: '%s'",
+					ct, mimeType, contentType,
+				),
+			)
+			must.Equal(t, string(b), string(file), "wrong file content returned")
+		})
+	})
+}