Fix comma separated list in query parameters

[lmazuel]

Fix Azure/azure-sdk-for-python#6370

[codecov-io]

Codecov Report

Merging #186 into master will decrease coverage by 0.82%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #186      +/-   ##
==========================================
- Coverage   87.89%   87.06%   -0.83%     
==========================================
  Files          25       25              
  Lines        2593     2597       +4     
==========================================
- Hits         2279     2261      -18     
- Misses        314      336      +22

Impacted Files	Coverage Δ
msrest/serialization.py	89.88% <100%> (-1.25%)	⬇️
msrest/paging.py	87.71% <0%> (-3.51%)	⬇️
msrest/exceptions.py	95.94% <0%> (-2.71%)	⬇️
msrest/pipeline/universal.py	93.13% <0%> (-1.97%)	⬇️
msrest/pipeline/__init__.py	89.6% <0%> (-1.61%)	⬇️
msrest/universal_http/__init__.py	74.09% <0%> (-1.04%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4a0a44a...2fcb2a9. Read the comment docs.

[johanste]

I assume we are not removing the capability to have separators other than ',' by calling serialize_iter?

[johanste]

Ah, never mind, I see we have tests for that. I would probably have called the parameter sep or separator rather than div. I was wondering what HTML had to do with this method when I first saw it :)

[lmazuel]

legacy :)

[johanste]

Ah, never mind, I see we have tests for that. I would probably have called the parameter sep or separator rather than div. I was wondering what HTML had to do with this method when I first saw it :)

[jhgoodwin]

Did anyone happen to notice that while this passes tests and seems like an obvious fix, the resulting query parameter is encoded incorrectly? array/list query parameters are expected to be like this:
https://myhost.com/api/myendpoint?friend=Robert+Downey%2C+Jr.&friend=Michael+Jordan
I validated this using URLSearchParams in a developer console of a browser window:

z = new URLSearchParams()
URLSearchParams {  }

z.append('friend', 'Robert Downey, Jr.')
undefined
z.append('friend', 'Michael Jordan')
undefined
z.getAll('friend')
Array [ "Robert Downey, Jr.", "Michael Jordan" ]
z.toString()
"friend=Robert+Downey%2C+Jr.&friend=Michael+Jordan"

Additionally, note that putting a comma in the query string results in the same behavior as the encoded version:

x = new URLSearchParams('friend=Robert+Downey,+Jr.&friend=Michael+Jordan')
URLSearchParams {  }

x.getAll('friend')
Array [ "Robert Downey, Jr.", "Michael Jordan" ]

[jhgoodwin]

A coworker pointed out that this may be relevant to the implementation of this feature: https://swagger.io/docs/specification/serialization/

(Or for 2.0 spec: https://swagger.io/docs/specification/2-0/describing-parameters/#array )

[jhgoodwin]

Looking closely, apparently 2.0 spec and 3.0 spec have different defaults, too. One is csv, the other is form, respectively. Fun times.