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 FormRequest.formdata with GET method duplicates same key in querystring (#2919) #3269

Closed

Conversation

iamriel
Copy link

@iamriel iamriel commented May 22, 2018

This is the fix for issue #2919 This is my first time contributing to an open source project. This PR is based on gekco's PR here. I followed the instruction in Submitting Patches for updating an existing PR. Hopefully I have done it correctly.

@iamriel iamriel changed the title Fix FormRequest.formdata with GET method duplicates same key in querystring #2919 Fix FormRequest.formdata with GET method duplicates same key in querystring (#2919) May 22, 2018
@codecov
Copy link

codecov bot commented May 22, 2018

Codecov Report

Merging #3269 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3269      +/-   ##
==========================================
+ Coverage   82.12%   82.13%   +<.01%     
==========================================
  Files         228      228              
  Lines        9599     9603       +4     
  Branches     1385     1386       +1     
==========================================
+ Hits         7883     7887       +4     
  Misses       1457     1457              
  Partials      259      259
Impacted Files Coverage Δ
scrapy/http/request/form.py 96.03% <100%> (+0.13%) ⬆️

@iamriel iamriel force-pushed the duplicate-key-in-formrequest-2919 branch from d34d640 to 095c727 Compare May 23, 2018 05:21
self._set_url(self.url + ('&' if '?' in self.url else '?') + querystr)
query = dict(parse_qsl(urlparse(self.url).query))
if query:
query.update(dict(items))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes we've to send multiple values for a single key, dict(items) will make it impossible and will result in unexpected URL.

formdata=[ ('a','1'), ('a','2'), ('b','3'), ]

@HassanQamar07
Copy link

HassanQamar07 commented Aug 11, 2018

@iamriel Can you add another test case which tests below formdata
e.g

formdata=[ ('a','1'), ('a','2'), ('b','3'), ]

@iamriel
Copy link
Author

iamriel commented Aug 12, 2018

@HassanQamar07 Thanks for checking this PR, you are right about it, I have updated the implementation and added the test in this commit. Please let me know if there is anything else. Thanks!

@iamriel
Copy link
Author

iamriel commented Aug 13, 2018

Ooops, the build failed, I'll update later today.

@maramsumanth
Copy link
Contributor

@iamriel , what happens when there is fragment in url. I think your code fails.

@iamriel
Copy link
Author

iamriel commented Jul 15, 2020

I apologize I haven't had time to attend into this. I'm closing this as @maramsumanth already had an approved PR for this. Thanks everyone!

@iamriel iamriel closed this Jul 15, 2020
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.

None yet

4 participants