Incorrect handling of % in URL under Ventura
Opened this issue · 1 comments
System and IINA version:
- macOS 13.6.3
- IINA 1.3.4
Expected behavior:
IINA correctly handles a %
character in a URL.
Actual behavior:
When running under macOS Ventura (and earlier releases) if a URL contains an encoded %
character (%25
) IINA will fail to open the file or stream.
This was uncovered during the investigation into issue #4861.
@xfoxfu provided the following example:
Opening this URL in Firefox:
iina://weblink?url=https://example.com/%25foo%20bar.mkv
Fails reporting the URL can not be percent encoded:
14:25:51.188 [iina][d] Parsing URL iina://weblink?url=https://example.com/%25foo%20bar.mkv
14:25:51.188 [player0][e] Cannot add percent encoding for https://example.com/%foo bar.mkv
Steps to reproduce:
-
Running under Ventura, enable logging in IINA
-
Open the URL given above in Firefox
-
Look for the message shown above in IINA's log file
-
MPV does not have this problem.
This is an issue with IINA code. I have not checked if mpv
also has this problem.
How often does this happen?
Every time.
Analysis
Adding additional logging to PlayerCore.openURLString
:
Logger.log("Percent encoding \(str)")
guard let pstr = str.addingPercentEncoding(withAllowedCharacters: .urlAllowed) else {
Logger.log("Cannot add percent encoding for \(str)", level: .error, subsystem: subsystem)
return
}
Logger.log("Percent encoded \(pstr)")
guard let url = URL(string: pstr) else {
Logger.log("Cannot parse url for \(pstr)", level: .error, subsystem: subsystem)
return
}
Shows the Cannot add percent encoding
error is bogus:
14:29:51.383 [iina][d] Parsing URL iina://weblink?url=https://example.com/%25foo%20bar.mkv
14:30:03.043 [iina][d] Percent encoding https://example.com/%foo bar.mkv
14:30:03.048 [iina][d] Percent encoded https://example.com/%foo%20bar.mkv
14:30:03.048 [player0][e] Cannot parse url for https://example.com/%foo%20bar.mkv
The addingPercentEncoding
method completed successfully. It was the URL initializer that failed because the resulting encoding is incorrect (%foo
).
Looking at the call to addingPercentEncoding
:
guard let pstr = str.addingPercentEncoding(withAllowedCharacters: .urlAllowed), let url = URL(string: pstr) else {
It uses urlAllowed
which does not require %
to be encoded:
extension CharacterSet {
static let urlAllowed: CharacterSet = {
var set = CharacterSet.urlHostAllowed
.union(.urlUserAllowed)
.union(.urlPasswordAllowed)
.union(.urlPathAllowed)
.union(.urlQueryAllowed)
.union(.urlFragmentAllowed)
set.insert(charactersIn: "%")
return set
}()
}
Percent was added to the set a long time ago in commit 84927d7.
The reason why CharacterSet provides the the different sets is that each URL component has different rules regarding what is allowed. It is concerning that IINA is trying to apply percent encoding to a full string.
Fixing
Because IINA is using the URL class which does not automatically encode characters, IINA is having to use addingPercentEncoding to encode the string before passing it to the URL initializer.
@xfoxfu pointed out that Apple has now changed this behavior. From the Xcode 15 Release Notes:
Fixed: For apps linked on or after iOS 17 and aligned OS versions, URL parsing has updated from the obsolete RFC 1738/1808 parsing to the same RFC 3986 parsing as URLComponents. This unifies the parsing behaviors of the URL and URLComponents APIs. Now, URL automatically percent- or IDNA-encodes invalid characters to help create a valid URL.
To check if a
urlString
is strictly valid according to the RFC, use the newURL(string: urlString, encodingInvalidCharacters: false)
initializer. This init leaves all characters as they are and will returnnil
ifurlString
is explicitly invalid. (93368104)
This means use of the URL
class needs to be conditionalized to handle both the old and new behaviors depending upon macOS version. Instead of continuing to use the URL
class and trying to correct IINA's percent encoding IINA should simplify the code and switch to the URLComponents which has always adhered to RFC 3986 and automatically handles encoding. This should correct the problem with handing %25
.