http
http copied to clipboard
feat: Add feature to parse multiple set-cookies
Hi amazing developers,
Perhaps this is already on the agenda before, but the http
library combines multiple set-cookie
s in the response header into a comma-separated list that is set in the headers
field. This is a rather complicated specification and not one that developers can easily handle.
Therefore, I have added the feature to parse set-cookie
s contained in headers
to make it easier to handle multiple set-cookie
s. This modification is not disruptive and the headers
field will continue to function as before.
However, in the future you need to reintegrate cookies
field when you make the headers
field an object, as was written in the BaseResponse
TODO.
Also it was necessary to add universal_io
to the dependencies to use Cookie
object.
Thank you.
Thanks for the PR! Some thoughts:
- this is a highly used package, so we generally want to be very conservative about adding a new package dependency. You're adding
package:universal_io
- I assume to get the definition of theCookie
class? We'll likely have to remove the dep. It looks like package:universal_io exports the dart:ioCookie
class in a dart:io context, and defines it's own class in other contexts. I don't know if that makes sense to do in this library, but fyi - adding @natebosch for review (and @brianquinlan for fyi, in case he has comments on the PR)
Thanks for your feedback @devoncarew !
This is exactly what I was thinking too.
I used to get an error in static analysis when I uploaded a package I was developing that used dart:io
to Pub.dev
, and this is the reason for adding universal_io
but now it seems I misunderstood the cause of that error.
And now I have scrutinized the universal_io
implementation and found no reason to use universal_io
instead of dart:io
in this fix. I'm going to refactor my pull request.
Thank you.
FYI this is related to https://github.com/dart-lang/http/issues/24 which proposes a different approach
up ,there's still an issue about parsing multiple set-cookies now , all set-cookies will be covered by last one
hope this can be merged asap