microdotblog / sunlit

Publish photos to your own blog — hosted by Micro.blog or compatible blogs using WordPress or Micropub — and discover beautiful photos from other users.
https://sunlit.io
MIT License
50 stars 7 forks source link

Uploads to external micropub sites should use unique filenames not "image.jpg" and "movie.mov" #131

Closed lildude closed 4 years ago

lildude commented 4 years ago

I noticed that uploading images and videos to an external micropub endpoints always use the same filenames: "image.jpg" and "movie.mov" respectively:

https://github.com/microdotblog/sunlit/blob/65567259f39b4739e1e80e40b3195aeaf426f9a4/Libraries/Snippets/Snippets%2BMicropub.swift#L203

https://github.com/microdotblog/sunlit/blob/65567259f39b4739e1e80e40b3195aeaf426f9a4/Libraries/Snippets/Snippets%2BMicropub.swift#L241

The XMLRPC uploads use a unique name based on a UUID:

https://github.com/microdotblog/sunlit/blob/65567259f39b4739e1e80e40b3195aeaf426f9a4/Libraries/Snippets/Snippets%2BXMLRPC.swift#L158

https://github.com/microdotblog/sunlit/blob/65567259f39b4739e1e80e40b3195aeaf426f9a4/Libraries/Snippets/Snippets%2BXMLRPC.swift#L205

All uploads should really upload unique filenames, regardless of destination.

I have no experience with Swift or iOS development, but I think it might be an easy change like this:

diff --git Libraries/Snippets/Snippets+Micropub.swift Libraries/Snippets/Snippets+Micropub.swift
index 957772e..9354d8c 100644
--- Libraries/Snippets/Snippets+Micropub.swift
+++ Libraries/Snippets/Snippets+Micropub.swift
@@ -207,6 +207,7 @@ extension Snippets {
             var formData : Data = Data()
             let imageName = "file"
             let boundary = ProcessInfo.processInfo.globallyUniqueString
+            let filename = UUID().uuidString.replacingOccurrences(of: "-", with: "") + ".jpg"

             if let blogUid = identity.micropubUid {
                 if blogUid.count > 0 {
@@ -217,7 +218,7 @@ extension Snippets {
             }

             formData.append(String("--\(boundary)\r\n").data(using: String.Encoding.utf8)!)
-            formData.append(String("Content-Disposition: form-data; name=\"\(imageName)\"; filename=\"image.jpg\"\r\n").data(using: String.Encoding.utf8)!)
+            formData.append(String("Content-Disposition: form-data; name=\"\(imageName)\"; filename=\"\(filename)\r\n").data(using: String.Encoding.utf8)!)
             formData.append(String("Content-Type: image/jpeg\r\n\r\n").data(using: String.Encoding.utf8)!)
             formData.append(imageData)
             formData.append(String("\r\n").data(using: String.Encoding.utf8)!)
@@ -245,6 +246,7 @@ extension Snippets {
             var formData : Data = Data()
             let imageName = "file"
             let boundary = ProcessInfo.processInfo.globallyUniqueString
+            let filename = UUID().uuidString.replacingOccurrences(of: "-", with: "") + ".mov"

             if let blogUid = identity.micropubUid {
                 if blogUid.count > 0 {
@@ -255,7 +257,7 @@ extension Snippets {
             }

             formData.append(String("--\(boundary)\r\n").data(using: String.Encoding.utf8)!)
-            formData.append(String("Content-Disposition: form-data; name=\"\(imageName)\"; filename=\"video.mov\"\r\n").data(using: String.Encoding.utf8)!)
+            formData.append(String("Content-Disposition: form-data; name=\"\(imageName)\"; filename=\"\(filename)\"\r\n").data(using: String.Encoding.utf8)!)
             formData.append(String("Content-Type: video/mov\r\n\r\n").data(using: String.Encoding.utf8)!)
             formData.append(data)
             formData.append(String("\r\n").data(using: String.Encoding.utf8)!)

I'm happy to open a PR if my understanding is correct.

cheesemaker commented 4 years ago

I believe that would be fine. Go ahead and do a PR and I can test it and merge it.

manton commented 4 years ago

I think this change makes sense too. A little background: Micro.blog ignores the filename, so it would only matter for external Micropub servers that did something with the filename, and part of me wondered if it was a privacy issue to expose the filename (like if someone would be surprised by that).

lildude commented 4 years ago

part of me wondered if it was a privacy issue to expose the filename

I don't think there is. The media endpoint is going to return a filename, whether it is one it received or one it produced itself, so a filename will be known at some point.

The only possible privacy issue I can think of is from the UUID, but as I understand it, pretty much everything defaults to v4 which doesn't include potentially identifying info like domain names or MAC addresses. A quick search suggests Swift uses v4 by default too.

manton commented 4 years ago

Oh, to be clear, what I meant by privacy is if clients used a photo's real filename, if there is one. Definitely no problem with using a UUID.

cheesemaker commented 4 years ago

All merged in! This will be in beta build 382