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

Add an option to disable or change URI encoding in HTTP package #11

Closed
DartBot opened this issue Jun 5, 2015 · 6 comments
Closed

Add an option to disable or change URI encoding in HTTP package #11

DartBot opened this issue Jun 5, 2015 · 6 comments
Labels
closed-not-planned Closed as we don't intend to take action on the reported issue P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@DartBot
Copy link

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/148256?v=3" align="left" width="96" height="96"hspace="10"> Issue by kaisellgren
Originally opened as dart-lang/sdk#10475


Currently as it stands, the HTTP package uses encodeUriComponent() from dart:uri to encode all fields.

This is not good when writing something like OAuth 1 lib for Twitter which happens to be what I/we are doing right now.

Instead, I have a fully RFC 3986 compliant percentage encoder, which Twitter OAuth API likes. The problem is that there is no way to disable or switch the encoding that is used.

Can we either add an option to disable it or to provide a callback for encoding?

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/1343914?v=3" align="left" width="48" height="48"hspace="10"> Comment by sgjesse


cc @nex3.
cc @munificent.
Added Area-Pkg, Triaged labels.

@DartBot DartBot added type-enhancement A request for a change that isn't a bug NotPlanned P3 A lower priority bug or feature request labels Jun 5, 2015
@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/188?v=3" align="left" width="48" height="48"hspace="10"> Comment by nex3


What's the practical difference between your encoder and encodeUriComponent?

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/148256?v=3" align="left" width="48" height="48"hspace="10"> Comment by kaisellgren


This is exactly what I am doing: https://dev.twitter.com/docs/auth/percent-encoding-parameters

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/188?v=3" align="left" width="48" height="48"hspace="10"> Comment by nex3


Perhaps the better solution is to follow RFC 3986 in the default encodeUriComponent implementation. Soren, what do you think?

I don't think this is something we want to make configurable in the http package. URL-escaping is only performed when using the [Request.bodyFields] attribute, which is just a wrapper over the underlying [Request.body] field. It would be pretty easy to write a [Request] subclass that uses whatever encoding mechanism you want for [bodyFields], or to write a [Client] subclass that re-encodes requests before sending them.


cc @sgjesse.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/1343914?v=3" align="left" width="48" height="48"hspace="10"> Comment by sgjesse


This is also related to the discussion in issue dart-lang/sdk#7464.

There is also the subtle difference in using URL encoding for FORM (both post and get) using the content type application/x-www-form-urlencoded. HTML 4.01 section 17.13.4 (http://www.w3.org/TR/REC-html40/interact/forms.html#h-17.13.4.1) says that space characters should be replaced by `+'.

Of cause the encoding of form post with application/x-www-form-urlencoded is not directly related to RFC 3938 except that the RFC 1738 (the RFC updated by RFC 3938) encoding is used after the space to `+' replacement has taken place.

@DartBot
Copy link
Author

DartBot commented Jun 5, 2015

<img src="https://avatars.githubusercontent.com/u/5479?v=3" align="left" width="48" height="48"hspace="10"> Comment by sethladd


Doesn't sound like the http package will add this option. Please reopen if otherwise.


Removed Type-Defect, Priority-Medium labels.
Added Type-Enhancement, Priority-Low, Pkg-Http, NotPlanned labels.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-not-planned Closed as we don't intend to take action on the reported issue P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

1 participant