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 escaping query parameters #369

Closed
wants to merge 1 commit into from
Closed

fix escaping query parameters #369

wants to merge 1 commit into from

Conversation

slizeray
Copy link
Contributor

@slizeray slizeray commented Mar 4, 2015

For escaping the query parameters I propose to use the iOS 7 API and above
stringByAddingPercentEncodingWithAllowedCharacters(NSCharacterSet.URLQueryAllowedCharacterSet())

The result is not the same as the current implementation which for instances escapes the / parameter whereas iOS does NOT

@mattt
Copy link
Sponsor Contributor

mattt commented Mar 4, 2015

stringByAddingPercentEncodingWithAllowedCharacters: suffers from excessive memory consumption. See #206.

@mattt mattt closed this Mar 4, 2015
@slizeray
Copy link
Contributor Author

slizeray commented Mar 4, 2015

@mattt OK, then you need to change the characters to escape because the current implementation escapes "/" and it should NOT

@mattt
Copy link
Sponsor Contributor

mattt commented Mar 4, 2015

@slizeray This is incorrect. According to RFC 3986 § 2.2, / is a general delimiter, and needs to be escaped.

@slizeray
Copy link
Contributor Author

slizeray commented Mar 4, 2015

@mattt This is perfectly correct. According to the RFC 3986 $ 3.4

The characters slash ("/") and question mark ("?") may represent data within the query component.
query = *( pchar / "/" / "?" )

Indeed, Apple has perfectly implemented the RFC.

@mattt
Copy link
Sponsor Contributor

mattt commented Mar 4, 2015

Beware that some older, erroneous implementations may not handle such data correctly when it is used as the base URI for relative references...

...it is sometimes better for usability to avoid percent-encoding [slash and question mark]

@slizeray That doesn't exactly inspire confidence. If it comes down to a question of usability concerns versus incorrect server-side handling, I'll err on the side of safety.

@slizeray
Copy link
Contributor Author

slizeray commented Mar 4, 2015

@mattt Well you put back some older code because of memory consumption. In between you had exactly the behaviour that now you think is wrong?
My problem is that I am using a server that does not understand the URL if I escape the legal "/" character.
Would it be possible to have an option such as we can choose between escaping or not "/"?

@mattt
Copy link
Sponsor Contributor

mattt commented Mar 4, 2015

@slizeray I wasn't aware of this specific difference in behavior previously, but knowing this now, I'm more comfortable keeping consistent default encoding behavior between Alamofire and AFNetworking. I understand how this might be inconvenient, which is why Alamofire offers two different ways to easily override this. First, you could use the Custom parameter encoding, supplying your own function to do the parameter escaping. Alternatively, you could bypass all of the URL encoding completely by passing a value of type conforming to URLRequestConvertible (NSURLRequest or a custom type) directly into request, upload and download.

@slizeray
Copy link
Contributor Author

slizeray commented Mar 4, 2015

@mattt OK Thanks I will give it try! As long as there is a solution, I am fine with that! That said, we could have an option both for Alamofire and AFNetworking....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants