iina/iina

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 new URL(string: urlString, encodingInvalidCharacters: false) initializer. This init leaves all characters as they are and will return nil if urlString 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.