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 an issue where empty Encodable Parameters resulted in a non-nil query string. #2818

Merged
merged 1 commit into from May 6, 2019

Conversation

@DavidBarry
Copy link
Contributor

commented May 3, 2019

Goals ⚽️

This fixes an issue where a Parameters instance passed to URLEncodedFormParameterEncoder that produced an empty query string would result in that empty string being set to the percentEncodedQuery instead of nil. This would result in a URL of https://httpbin.org/get? instead of https://httpbin.org/get and differs from the behavior of the existing URLEncoding.

Implementation Details 🚧

This adds a check for an empty string when setting the percentEncodedQuery and sets nil instead in that case.

Testing Details 🔍

A new test was added to cover this case using an empty dictionary as the parameter type. This test mirrors one that exists for URLEncoding for this situation.

@cnoon cnoon requested review from cnoon and jshier May 3, 2019

@cnoon cnoon added this to the 5.0.0-beta.7 milestone May 3, 2019

@cnoon

cnoon approved these changes May 3, 2019

Copy link
Member

left a comment

Looks good @DavidBarry! 🍻

@jshier

This comment has been minimized.

Copy link
Contributor

commented May 6, 2019

Yep, looks great! Thanks @DavidBarry!

@jshier jshier merged commit 1081318 into Alamofire:master May 6, 2019

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details

@DavidBarry DavidBarry deleted the DavidBarry:bugfix/empty-query-string branch May 6, 2019

toomasr added a commit to toomasr/Alamofire that referenced this pull request Aug 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.