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

Fix Share Sheet + image upload for images with copyright symbol from Photos app #17779

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions RELEASE-NOTES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* [**] Reader post details Comments snippet: added ability to manage conversation subscription and notifications. [#17749]
* [**] Accessibility: VoiceOver improvements on Activity Log and Schedule Post calendars [#17756, #17761]
* [*] Weekly Roundup: Fix a crash which was preventing weekly roundup notifications from appearing [#17765]
* [*] Share Sheet from Photos: Fix an issue where certain filenames would not upload or render in Post [#16773]

19.0
-----
Expand Down
13 changes: 13 additions & 0 deletions WordPress/Classes/Extensions/String+Files.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,17 @@ extension String {
fileExtension as CFString,
nil)?.takeUnretainedValue() as String?
}

/// Lightweight swift version of wordpress formatting.php sanitize_file_name function
///
/// - Returns: sanitized filename to match api
func sanitizeFileName() -> String {
var fileName = folding(options: .diacriticInsensitive, locale: .current)

let specialChars: Set<Character> = ["?", "[", "]", "/", "\\", "=", "<", ">", ":", ";", ",", "'", "\"", "&", "$", "#", "*", "(", ")", "|", "~", "`", "!", "{", "}", "%", "+", "’", "«", "»", "”", "“"]
fileName.removeAll(where: { specialChars.contains($0) })

return fileName
}

}
8 changes: 6 additions & 2 deletions WordPress/WordPress.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -2268,6 +2268,8 @@
DC3B9B2C27739760003F7249 /* TimeZoneSelectorViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = DC3B9B2B27739760003F7249 /* TimeZoneSelectorViewModel.swift */; };
DC3B9B2D27739760003F7249 /* TimeZoneSelectorViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = DC3B9B2B27739760003F7249 /* TimeZoneSelectorViewModel.swift */; };
DC3B9B2F27739887003F7249 /* TimeZoneSelectorViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = DC3B9B2E27739887003F7249 /* TimeZoneSelectorViewModelTests.swift */; };
DC5FE07F2795C6A4006D5779 /* String+Files.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7360018E20A265C7001E5E31 /* String+Files.swift */; };
DC5FE0802795C6A5006D5779 /* String+Files.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7360018E20A265C7001E5E31 /* String+Files.swift */; };
DC76668326FD9AC9009254DD /* TimeZoneRow.swift in Sources */ = {isa = PBXBuildFile; fileRef = DC76668126FD9AC8009254DD /* TimeZoneRow.swift */; };
DC76668426FD9AC9009254DD /* TimeZoneRow.swift in Sources */ = {isa = PBXBuildFile; fileRef = DC76668126FD9AC8009254DD /* TimeZoneRow.swift */; };
DC76668526FD9AC9009254DD /* TimeZoneTableViewCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = DC76668226FD9AC9009254DD /* TimeZoneTableViewCell.swift */; };
Expand Down Expand Up @@ -8508,7 +8510,7 @@
path = Classes;
sourceTree = "<group>";
};
29B97314FDCFA39411CA2CEA = {
29B97314FDCFA39411CA2CEA /* CustomTemplate */ = {
isa = PBXGroup;
children = (
3F20FDF3276BF21000DA3CAD /* Packages */,
Expand Down Expand Up @@ -15299,7 +15301,7 @@
bg,
sk,
);
mainGroup = 29B97314FDCFA39411CA2CEA;
mainGroup = 29B97314FDCFA39411CA2CEA /* CustomTemplate */;
packageReferences = (
3FF1442E266F3C2400138163 /* XCRemoteSwiftPackageReference "ScreenObject" */,
3FC2C33B26C4CF0A00C6D98F /* XCRemoteSwiftPackageReference "XCUITestHelpers" */,
Expand Down Expand Up @@ -18723,6 +18725,7 @@
74021A14202E1393006CC39F /* ShareExtensionEditorViewController.swift in Sources */,
CBF6201426E8FB8A0061A1F8 /* RemotePost+ShareData.swift in Sources */,
CE39E17320CB117B00CABA05 /* RemoteBlog+Capabilities.swift in Sources */,
DC5FE0802795C6A5006D5779 /* String+Files.swift in Sources */,
74021A0F202E1370006CC39F /* ExtensionTransitioningManager.swift in Sources */,
74E44AD92031ED2300556205 /* WordPressDraft-Lumberjack.m in Sources */,
741AF3A5202F3E2A00C771A5 /* Tracks+DraftAction.swift in Sources */,
Expand Down Expand Up @@ -18809,6 +18812,7 @@
170BEC872391530D0017AEC1 /* FeatureFlagOverrideStore.swift in Sources */,
CBF6201326E8FB520061A1F8 /* RemotePost+ShareData.swift in Sources */,
74448F542044BC7600BD4CDA /* CategoryTree.swift in Sources */,
DC5FE07F2795C6A4006D5779 /* String+Files.swift in Sources */,
74402F2A200528F200A1D4A2 /* ExtensionPresentationController.swift in Sources */,
B5BEA55F1C7CE6D100C8035B /* Constants.m in Sources */,
CE39E17220CB117B00CABA05 /* RemoteBlog+Capabilities.swift in Sources */,
Expand Down
12 changes: 11 additions & 1 deletion WordPress/WordPressShareExtension/AppExtensionsService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,12 @@ extension AppExtensionsService {
mediaUploadOp.height = height
}

ShareMediaFileManager.shared.removeFromUploadDirectory(fileName: localFileName)
// localURL lastPathComponent is the original unsanitized filename
if let localURL = mediaUploadOp.localURL, let url = URL(string: localURL) {
ShareMediaFileManager.shared.removeFromUploadDirectory(fileName: url.lastPathComponent)
} else {
ShareMediaFileManager.shared.removeFromUploadDirectory(fileName: localFileName)
}
}
}
})
Expand Down Expand Up @@ -366,7 +371,12 @@ fileprivate extension AppExtensionsService {
var allRemoteMedia = [RemoteMedia]()
localMediaFileURLs.forEach { tempFilePath in
let remoteMedia = RemoteMedia()

// Use a percent encoded + sanitized filename as the remoteMedia file.
remoteMedia.file = tempFilePath.lastPathComponent
.addingPercentEncoding(withAllowedCharacters: .urlPathAllowed)?
.sanitizeFileName()

remoteMedia.mimeType = Constants.mimeType
remoteMedia.localURL = tempFilePath
allRemoteMedia.append(remoteMedia)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -989,7 +989,12 @@ extension ShareExtensionEditorViewController {
let attachment = richTextView.replaceWithImage(at: self.richTextView.selectedRange, sourceURL: url, placeHolderImage: Assets.defaultMissingImage)

attachment.size = .full
attachment.uploadID = url.lastPathComponent // Use the filename as the uploadID here.

// Use a percent encoded + sanitized filename as the uploadID here.
attachment.uploadID = url.lastPathComponent
.addingPercentEncoding(withAllowedCharacters: .urlPathAllowed)?
.sanitizeFileName()

richTextView.refresh(attachment)
}
}
Expand Down
26 changes: 26 additions & 0 deletions WordPress/WordPressTest/Extensions/StringExtensionsTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,30 @@ class StringExtensionsTests: XCTestCase {
func testLinkifyingPlainText() {
XCTAssertEqual(text.stringWithAnchoredLinks(), text, "Oh noes!")
}

func testSanitizeFileName() {
// https://github.com/wordpress-mobile/WordPress-iOS/issues/16773
// Given Percent encoded string
let fileNameRaw = "crown-house_%C2%A9-francesco-russo_websize_002-1-1"

// When sanitizing
// Then sanitizeFileName returns strings without %
XCTAssertEqual(fileNameRaw.sanitizeFileName(), "crown-house_C2A9-francesco-russo_websize_002-1-1")


// Given a filename with accents
let fileNameWithAccents = "Français"

// When sanitizing
// Then sanitizeFileName returns fileName without accents
XCTAssertEqual(fileNameWithAccents.sanitizeFileName(), "Francais")


// Given a filename with only special characters
let fileNameWithOnlySpecialChars = "?[]/\\=<>:;,'\"&$#*()|~`!{}%+’«»”“"

// When sanitizing
// Then sanitizeFileName returns empty string
XCTAssertEqual(fileNameWithOnlySpecialChars.sanitizeFileName(), "")
}
}