Re: [elixir-core:10038] [Proposal] Add `URI.encode_query/2` to allow RFC3986 space encoding

2021-01-29 Thread Floris Huetink
On Friday, January 29, 2021 at 8:23:04 PM UTC+1 José Valim wrote: > Yes, please go ahead with the PR, it will be very appreciated! > Great! I've just submitted a PR here: https://github.com/elixir-lang/elixir/pull/10680 -- You received this message because you are subscribed to the Google

Re: [elixir-core:10036] [Proposal] Add `URI.encode_query/2` to allow RFC3986 space encoding

2021-01-29 Thread José Valim
> Great! I'd like to take this up myself it that's OK. (This would be my first Elixir core contribution, so feedback is very much appreciated.) Yes, please go ahead with the PR, it will be very appreciated! On Fri, Jan 29, 2021 at 8:17 PM Floris Huetink wrote: > Oh wait, now that I read this:

Re: [elixir-core:10036] [Proposal] Add `URI.encode_query/2` to allow RFC3986 space encoding

2021-01-29 Thread Floris Huetink
Oh wait, now that I read this: https://github.com/elixir-lang/elixir#proposing-new-features Should I first wait for an issue to be added to the issue tracker before opening up a PR? Eager to get started, but let's not rush things :) On Friday, January 29, 2021 at 8:15:00 PM UTC+1 Floris

Re: [elixir-core:10035] [Proposal] Add `URI.encode_query/2` to allow RFC3986 space encoding

2021-01-29 Thread Floris Huetink
On Friday, January 29, 2021 at 8:06:28 PM UTC+1 José Valim wrote: > > - Implement encode_query/2: say via encode_query(_, :www_form) and > > encode_query(_, > :rfc3986); reimplement encode_query/1 to use :www_form > > I like this. A PR is welcome. :) > > Great! I'd like to take this up myself

Re: [elixir-core:10034] [Proposal] Add `URI.encode_query/2` to allow RFC3986 space encoding

2021-01-29 Thread David Bernheisel
Heads up, the ExAws library also had a problem with this, and there was a PR to adjust the behavior to match S3. I'm not suggesting S3 is doing it correctly, but we've had to deal with this in the community already. https://github.com/ex-aws/ex_aws/pull/660 On Friday, January 29, 2021 at

Re: [elixir-core:10032] [Proposal] Add `URI.encode_query/2` to allow RFC3986 space encoding

2021-01-29 Thread José Valim
> - Implement encode_query/2: say via encode_query(_, :www_form) and > encode_query(_, :rfc3986); reimplement encode_query/1 to use :www_form I like this. A PR is welcome. :) On Fri, Jan 29, 2021 at 7:57 PM Christopher Keele wrote: > *> For example, someone could use a query string where the

Re: [elixir-core:10032] [Proposal] Add `URI.encode_query/2` to allow RFC3986 space encoding

2021-01-29 Thread Christopher Keele
*> For example, someone could use a query string where the meaning of & and = are replaced and that's fine.* I've had to work with an API that used subtly different querystring encoding semantics, and had to pretty much throw out all of Tesla to get it to work, since it made these assumptions

Re: [elixir-core:10030] [Proposal] Add `URI.encode_query/2` to allow RFC3986 space encoding

2021-01-29 Thread José Valim
Ok, sorry about the initial confusion. I thought we emitted %20 and you were proposing to emit +. So I was basically arguing in favor of your change except I didn't know it. :) Here is my hopefully correct e-reply: encode_www_form (which is what is used by encode_query) is not specified by

Re: [elixir-core:10030] [Proposal] Add `URI.encode_query/2` to allow RFC3986 space encoding

2021-01-29 Thread Floris Huetink
Ah, thanks! I was already getting quite lost here, seriously doubting my own understanding of the situation :) Looking forward to your next reply! On Friday, January 29, 2021 at 4:27:20 PM UTC+1 José Valim wrote: > Gah, I am so sorry. I have been working on the wrong assumption that >

Re: [elixir-core:10028] [Proposal] Add `URI.encode_query/2` to allow RFC3986 space encoding

2021-01-29 Thread José Valim
Gah, I am so sorry. I have been working on the wrong assumption that URI.encode_query was escaping space to %20 but it is encoding it to +, which was your point all along. Yes, escaping it to + is not in accordance to RFC3986. I will re-read your original e-mail and address it accordingly now.

Re: [elixir-core:10027] [Proposal] Add `URI.encode_query/2` to allow RFC3986 space encoding

2021-01-29 Thread José Valim
> If I read this correctly, than given what you write, the current `URI.encode_query/1` implementation _is_ in violation of RFC3986. Example: You can’t compare the result of URI.encode with URI.encode_query because they are meant to escape different parts of an URI and different parts use

Re: [elixir-core:10027] [Proposal] Add `URI.encode_query/2` to allow RFC3986 space encoding

2021-01-29 Thread Floris Huetink
Hi José, Thanks for the quick reply! To be sure, generally speaking, I certainly see what you say about "a bit more complicated than expected". A couple of sentences are a bit confusing to me, and make me wonder whether we've understood each other correctly. I'll explain in context of your

Re: [elixir-core:10025] [Proposal] Add `URI.encode_query/2` to allow RFC3986 space encoding

2021-01-29 Thread José Valim
Hi Floris, thanks for the proposal! Unfortunately this is a bit more complicated than expected. First of all, the current implementation is not in violation of RFC3986. %20 is a valid escaping of spaces in query strings. As far as I know, RFC3986 does not explicitly mention that + is equivalent

[elixir-core:10025] [Proposal] Add `URI.encode_query/2` to allow RFC3986 space encoding

2021-01-29 Thread Floris Huetink
Hi, I've been using `URI.encode_query/1` to convert a key/value map to a query string to be appended to a URL (as GET parameters). As it turns out, `URI.encode_query/1` encodes differently than `URI.encode/2`, particularly in the case of spaces ("+" instead of "%20"). For more context, please