[GH-ISSUE #401] Double encoded callback url causes issues #258

Closed
opened 2026-03-03 16:47:08 +03:00 by kerem · 6 comments
Owner

Originally created by @iby on GitHub (Aug 14, 2017).
Original GitHub issue: https://github.com/OAuthSwift/OAuthSwift/issues/401

Description:

I've been using the framework successfully for awhile, but recently it stoped working with Imgur with The redirect URI provided is missing or does not match error. The issue doesn't seem to be related to any particular changes in the framework, but rather to Imgur backend updates. However, oauth works with Postman and Paw, which leads to certain conclusions.

Digging deeper the issue seems to be related to double encoded callback url in OAuth2Swift::authorize method. Removing double encoding solves the issue. I believe services take this into account and un-encode the url, which is good and bad, as it makes it easier to work with, but introduces uncertainty.

queryString composed in OAuth2Swift::authorize combines every parameter as is with exception for callback url, which gets encoded. The composed query then gets encoded, which causes url to be double encoded. If query would be composed with every parameter encoded immediately and without re-encoding everything, it would produce a valid query and eliminate the issue.

P. S. I see that logic has changed recently, however, the problem occurs in the following urlQueryEncoded invocation.

OAuth Provider (Twitter, Github, ..):

  • Imgur

OAuth Version:

  • Version 2

OS (Please fill the version) :

  • OSX :

Installation method:

  • Carthage

Library version:

  • v1.1.2

Xcode version:

  • 8.0 (Swift 3.0)
Originally created by @iby on GitHub (Aug 14, 2017). Original GitHub issue: https://github.com/OAuthSwift/OAuthSwift/issues/401 ### Description: I've been using the framework successfully for awhile, but recently it stoped working with Imgur with `The redirect URI provided is missing or does not match` error. The issue doesn't seem to be related to any particular changes in the framework, but rather to Imgur backend updates. However, oauth works with Postman and Paw, which leads to certain conclusions. Digging deeper the issue seems to be related to double encoded callback url in `OAuth2Swift::authorize` method. Removing double encoding solves the issue. I believe services take this into account and un-encode the url, which is good and bad, as it makes it easier to work with, but introduces uncertainty. [`queryString` composed in `OAuth2Swift::authorize`](https://github.com/OAuthSwift/OAuthSwift/blob/b50109fd752f93c93151de4b7695bd710aa26bed/Sources/OAuth2Swift.swift#L121-L143) combines every parameter as is with exception for callback url, which gets encoded. The composed query then gets encoded, which causes url to be double encoded. If query would be composed with every parameter encoded immediately and without re-encoding everything, it would produce a valid query and eliminate the issue. P. S. I see that logic [has changed recently](https://github.com/OAuthSwift/OAuthSwift/blob/b50109fd752f93c93151de4b7695bd710aa26bed/Sources/OAuth2Swift.swift#L122-L126), however, the problem occurs in the following `urlQueryEncoded` invocation. ### OAuth Provider (Twitter, Github, ..): - [x] Imgur ### OAuth Version: - [x] Version 2 ### OS (Please fill the version) : - [x] OSX : ### Installation method: - [x] Carthage ### Library version: - [x] v1.1.2 ### Xcode version: - [x] 8.0 (Swift 3.0)
kerem closed this issue 2026-03-03 16:47:08 +03:00
Author
Owner

@phimage commented on GitHub (Aug 14, 2017):

someask ask for callback url encoding PR #325 e56fafadc8
I make it optional using encodeCallbackURL boolean, issue #339

by default there is no encoding of callback url

<!-- gh-comment-id:322295691 --> @phimage commented on GitHub (Aug 14, 2017): someask ask for callback url encoding PR #325 e56fafadc8a0db781ab51e45eb0522d70058026c I make it optional using encodeCallbackURL boolean, issue #339 by default there is no encoding of callback url
Author
Owner

@iby commented on GitHub (Aug 14, 2017):

I know. I've seen those. This is great, but not the case. If it's set to true url gets encoded twice, first here, then here. Frankly, if url is not encoded, it doesn't get encoded the second time. Like #339 describes – slashes ( / ) are being replaced with %252F – when encoded, they should become %2F, but % gets encoded twice, producing %252F.

<!-- gh-comment-id:322299084 --> @iby commented on GitHub (Aug 14, 2017): I know. I've seen those. This is great, but not the case. If it's set to true url gets encoded twice, first [here](https://github.com/OAuthSwift/OAuthSwift/blob/b50109fd752f93c93151de4b7695bd710aa26bed/Sources/OAuth2Swift.swift#L123), then [here](https://github.com/OAuthSwift/OAuthSwift/blob/b50109fd752f93c93151de4b7695bd710aa26bed/Sources/OAuth2Swift.swift#L141). Frankly, if url is not encoded, it doesn't get encoded the second time. Like #339 describes – `slashes ( / ) are being replaced with %252F` – when encoded, they should become %2F, but % gets encoded twice, producing %252F.
Author
Owner

@iby commented on GitHub (Aug 14, 2017):

Happy to make a PR.

<!-- gh-comment-id:322299506 --> @iby commented on GitHub (Aug 14, 2017): Happy to make a PR.
Author
Owner

@phimage commented on GitHub (Aug 14, 2017):

If you do not encode twice, how the server could decode and know to what url query parameters belong to

If we take this query
x=5&callbackurl=http://toto?y=8&z=4
the z is for callbackurl or main url

If we encode the first time using encodeCallbackURL =true
x=5&callbackurl=http%3A%2F%2Ftoto%3Fy%3D8%26z%3D4

then reencode (callback is encoded two times)

and then the server decode - I think server could know that z is for callbackurl
some server expect callbackurl as last query parameters...

It's difficult to make compatible with a lot of providers because of weird choice...
If you are able to provide a code that work for your provider, please PR.
But If we can keep the current two workflow...encodeCallbackURL=true and encodeCallbackURL=false
Maybe have an another dirty boolean, or make an enum with 3 cases (keep the boolean as compatiblity with deprecated and remapped to the enum)

Maybe I miss something
I must code, make unit test with URL to fully understand, try with some providers to validate that all are compatibles

<!-- gh-comment-id:322303719 --> @phimage commented on GitHub (Aug 14, 2017): If you do not encode twice, how the server could decode and know to what url query parameters belong to If we take this query x=5&callbackurl=http://toto?y=8&z=4 the z is for callbackurl or main url If we encode the first time using encodeCallbackURL =true x=5&callbackurl=http%3A%2F%2Ftoto%3Fy%3D8%26z%3D4 then reencode (callback is encoded two times) and then the server decode - I think server could know that z is for callbackurl some server expect callbackurl as last query parameters... It's difficult to make compatible with a lot of providers because of weird choice... If you are able to provide a code that work for your provider, please PR. But If we can keep the current two workflow...encodeCallbackURL=true and encodeCallbackURL=false Maybe have an another dirty boolean, or make an enum with 3 cases (keep the boolean as compatiblity with deprecated and remapped to the enum) Maybe I miss something I must code, make unit test with URL to fully understand, try with some providers to validate that all are compatibles
Author
Owner

@iby commented on GitHub (Aug 14, 2017):

No, you are right. I thought of it in the first place, but lost it out of sight with other stuff in mind. But like you say, if we don't encode it at all, the same thing happens – query parameters might get mixed. I'm mostly bummed out by the fact that it used to work… I'll look into this.

<!-- gh-comment-id:322305599 --> @iby commented on GitHub (Aug 14, 2017): No, you are right. I thought of it in the first place, but lost it out of sight with other stuff in mind. But like you say, if we don't encode it at all, the same thing happens – query parameters might get mixed. I'm mostly bummed out by the fact that it used to work… I'll look into this.
Author
Owner

@iby commented on GitHub (Aug 15, 2017):

Also tested with my project, everything seems to work! 👌

<!-- gh-comment-id:322437698 --> @iby commented on GitHub (Aug 15, 2017): Also tested with my project, everything seems to work! 👌
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/OAuthSwift#258
No description provided.