iOS client not properly escaping "+" sign when using GET for queries

[winstondu]

Bug report

When strings are an input variable to a query, that input is not correctly sent to the server. In particular, "+" characters are changed into " " characters when converting into the GET query.

See: https://stackoverflow.com/a/24888789/7612797

while it's not contemplated in the aforementioned RFC, section 5.2, URL-encoded form data, of the W3C HTML spec says that application/x-www-form-urlencoded requests should also replace space characters with + characters (and includes the asterisk in the characters that should not be escaped). And, unfortunately, URLComponents won't properly percent escape this, so Apple advises that you manually percent escape it before retrieving the url property of the URLComponents object.

We had a very simple query our graphQL server was supporting.

query PhoneCheck($phone: String!) {
    checkPhone(phone: $phone)
}

Often, the $phone variable may include a "+" sign in front (e.g. "+1-555-554-0909").

When querying the web console, this input variable is correctly received by the server. But when querying from the iOS client, the server somehow does not get the "+" sign. (e.g. "+1-555-554-0909" becomes " 1-555-554-0909" ).

Versions

Please fill in the versions you're currently using:

• apollo-ios SDK version: 0.41.0
• Xcode version: 11.4
• Swift version: Latest
• Package manager: Cocoapods

Steps to reproduce

See description

Further info

Since the iOS client is using GET queries, strings should therefore be URL-encoded.

Generated code is as follows:

  public final class PhoneCheckQuery: GraphQLQuery {
    /// The raw GraphQL definition of this operation.
    public let operationDefinition: String =
      """
      query PhoneCheck($phone: String!) {
        checkPhone(phone: $phone)
      }
      """

    public let operationName: String = "PhoneCheck"

    public let operationIdentifier: String? = "dabcc241b73fedbe393a0e90c8855802a94b82ad04cd5e9825c514b359e0ce78"

    public var phone: String

    public init(phone: String) {
      self.phone = phone
    }

    public var variables: GraphQLMap? {
      return ["phone": phone]
    }

    public struct Data: GraphQLSelectionSet {
      public static let possibleTypes: [String] = ["Query"]

      public static var selections: [GraphQLSelection] {
        return [
          GraphQLField("checkPhone", arguments: ["phone": GraphQLVariable("phone")], type: .scalar(PhoneNumberCheckResult.self)),
        ]
      }

      public private(set) var resultMap: ResultMap

      public init(unsafeResultMap: ResultMap) {
        self.resultMap = unsafeResultMap
      }

      public init(checkPhone: PhoneNumberCheckResult? = nil) {
        self.init(unsafeResultMap: ["__typename": "Query", "checkPhone": checkPhone])
      }

      /// Returns a check of the phone number.
      public var checkPhone: PhoneNumberCheckResult? {
        get {
          return resultMap["checkPhone"] as? PhoneNumberCheckResult
        }
        set {
          resultMap.updateValue(newValue, forKey: "checkPhone")
        }
      }
    }
  }

[AnthonyMDev]

Thanks for bringing this to our attention. I'm working on a PR to correct this right now!

[AnthonyMDev]

@winstondu I've pushed up PR #1653 to fix this. Would you mind applying the fix locally and testing to confirm this fixes your issue? Assuming it does, we will push a new patch version with this fix in the next few days!

[winstondu]

@AnthonyMDev , it does not. I suspect there also needs to be a corresponding fix to the ApolloServer library to add decoding. The server now sees the string as a "%2b".

[AnthonyMDev]

Ahhhh, that's not great... haha. I was hoping it was just a simple client side fix. I'll have to get someone from our server team involved to see what the correct behavior for this is.

[designatednerd]

@winstondu I took another stab at it - I think maybe percent encoding the parameter before getting to the query level might do it. Can you pull that branch again and give it another shot?

[winstondu]

@designatednerd , this was actually what I tried to do as a workaround, but unfortunately the server just sees it as "%2b". It doesn't decode it at all.

[designatednerd]

wait, i thought you were saying that it was seeing %2B with @AnthonyMDev's original fix - or is it showing up as %2B in both places?

[winstondu]

@designatednerd , I will need to test your fix, but it looks substantially similar to something I tried before reporting the issue, and the server saw %2B instead of +

[designatednerd]

OK - would appreciate it if you could confirm what you're seeing when using the double-encoded property - that'll help.

[designatednerd]

Hey @winstondu were you able to confirm what you get when you're using the double-encoded property? Thank you!

[winstondu]

Hey @winstondu were you able to confirm what you get when you're using the double-encoded property? Thank you!

I will test right now.

[winstondu]

@designatednerd , yup, when I log what I receive from the resolver , the server receives %2B in the corresponding graphQL resolver instead of +.

We're using apollo-server-express at 2.17.0.

[designatednerd]

Interesting that it gets the same thing whether it's single or double encoded! I'll talk to server folk and get back to you.

[winstondu]

Thanks. I appreciate it. Again, it's only for GET queries. (POST works fine)

[designatednerd]

yeah there's something goofy going on with the URL encoding, I'm not super-sure what though

[designatednerd]

OK, so in talking to some folks and running a few things through URL encoder/decoders online:

• @AnthonyMDev's fix appears to have been correct (the double encoding comes through as %2B when I run it through URL decoders, and single encoding comes through as +), so I've reverted my changes.
• One of my colleagues pulled a small repro together here: https://codesandbox.io/s/nice-heisenberg-ywq7s?file=/src/server.ts. Once that's started you can run curl 'https://ywq7s.sse.codesandbox.io/?query=query%20MySearch(%24phone%3AString!)%20%7B%20userByPhone(phone%3A%20%24phone)%20%7B%20phone%20%7D%20%7D&variables=%7B%22phone%22%3A%20%22%2B16195552569%22%7D' at the command line, and you should get back {"data":{"userByPhone":{"phone":"+16195552569"}}}. His suggestion is to take a look at your bodyParser in your express usage if that doesn't help figure it out.

Does any of that help, @winstondu?

[designatednerd]

@winstondu Were you able to find anything on the above?

[winstondu]

@designatednerd , sorry about taking my time. Unfortunately not yet. Could you circle back to me later this week?

[designatednerd]

Yep just wanted to check in! I may wind up merging that open PR since I've verified that it works with assorted URL decoders, but I'll leave this open a bit longer.

[designatednerd]

silly merge auto-closing things, that's not what i wanted

[calvincestari]

Closing due to inactivity. If this is still an issue for you please reply in this issue - thank you.