General comment: shouldn't we encode/decode cookies with URLEncode by default? I think this is a standard, I can see even on this github page "tz=Europe%2FCopenhagen; _octo=GH1.1.1424226204.1506678902" = $1

Or even we could do it bright and shine, leaving the user what coder/decoder to use. Please check the following API, we might spread it to the other parts of Drab later:

set_cookie name, value, encode: Drab.Coder.Encrypt
get_cookie name, decode: Drab.Coder.URL

where every Drab.Coder must have encode and decode functions (it might implement Drab.Coder behaviour)

• Drab.Coder.Encrypt - encrypt + base64 - takes any erlang term
• Drab.Coder.Base64 - base 64 only - takes any erlang term
• Drab.Coder.URL - urlencoder - takes only string

So later we might easly add new coders etc.

For those coders, we should follow the pattern from Jason or Poison: encode, decode, encode!, decode!, so all the functions which should encode/decode data might take not only our coders, but also Jason:

do_something_with_string string, encode: Drab.Coder.URL
do_something_with_string string, encode: Jason

Also, we might have a default encoder (configurable in config.exs), which normally should be encrypt + base64 (for safety). Then the user might just do:

set_cookie cookie, value, encode: true

Also (sorry for a comment spam ;) we should have Drab.Coder.encode and Drab.Coder.decode, which delegates to the default coder. So in your controller, you could call them without specifying the one.

Ebert has finished reviewing this Pull Request and has found:

• 7 possible new issues (including those that may have been commented here).
• 7 fixed issues! 🎉

You can see more details about this review at https://ebertapp.io/github/grych/drab/pulls/136.

General comment: shouldn't we encode/decode cookies with URLEncode by default?

I'm generally in favor to default encoding something when the format could be critical in term of errors between the storing and retrieving of the value. But in this case I think it should be left to the developer decision, as cookies aren't an opaque storage, but they are directly visible also to the final user, so the encoding format matter, for example when the gdpr privacy and beyond problematics are taken in account.

Furthermore, when we extract a cookie, we cannot know in advance if it was encoded and how (the programmer might want to retrive a cookie written by others), so we cannot apply any default decode criteria.

A case where default encoding maybe could be useful is when complex values have to be transferred from an .html.drab template to the backend by mean the third value of a Drab event handler call.

For example, instead to having to explicitly encoded it in the template

<% encoded_value = Drab.Utils.encode_param %{question: "foo", answer: 42} %>
<button  drab='click:check_answer("<%= encoded_value %>")'>Check answer</button>

and then having to remember to decode it in the commander

defhandler check_answer(socket, sender, value) do
  decoded_value = Drab.Utils.decode_value(value)
        
  question = decoded_value["question"]
  answer = decoded_value["answer"]
end

we maybe could just have an helper that takes the name of the event, the name of the handler and the parameter with the complex value to pass on and some optional option, that implicitly encode the value, mark it as encoded someway and then in Drab.Core recognizing that flag in the parameter, and transparently decode the value without an action by the programmer, something like

<button drab=<%=drab_handler(:click, :check_answer, %{question: "foo", answer: "42"}, encrypted: true) %> >Check answer</button>

defhandler check_answer(socket, sender, value) do        
  question = value["question"]
  answer = value["answer"]
end

Furthermore, if we give transparently encoding by default, we probably should give also security by design, for example encrypting by default all the communications between browser and the backend.

Or even we could do it bright and shine, leaving the user what coder/decoder to use.
[...] where every Drab.Coder must have encode and decode functions (it might implement Drab.Coder behaviour)
[...] we should follow the pattern from Jason or Poison: encode, decode, encode!, decode!, so all the functions which should encode/decode data might take not only our coders, but also Jason
Also, we might have a default encoder (configurable in config.exs), which normally should be encrypt + base64 (for safety)

Absolutely! This is the right way to go. Elegant and consistent!
it is certainly good to manage the encode actions, but as stated before I don't know if it is good to encode by default the cookies.

In brief, my (yet mumbling) view is:

• for cookies, default is plain cookies, programmer choose if/when/how encoded them;
• for drab_handler calls, default is to transparently encode/decode the passed value; programmer can set a default in the config and/or specify the encode preferred method for every call

for cookies, default is plain cookies, programmer choose if/when/how encoded them;

set_cookie cookie, value # no encoding
set_cookie cookie, value, encode: true # default &Drab.Coder.encode/1
set_cookie cookie, value, encode: Drab.Coder.URL # calling Drab.Coder.URL.encode
set_cookie cookie, value, encode: Any.Other.Module # calling Any.Other.Module.encode

The similar for decoding.

for drab_handler calls, default is to transparently encode/decode the passed value; programmer can set a default in the config and/or specify the encode preferred method for every call

Sorry, I don't get, could you please elaborate?

I am going to create this Drab.Coder now, please hold on.

for drab_handler calls, default is to transparently encode/decode the passed value; programmer can set a default in the config and/or specify the encode preferred method for every call

Sorry, I don't get, could you please elaborate?

it is not about cookies, it was referring to my previous comment:

A case where default encoding maybe could be useful is when complex values have to be transferred from an .html.drab template to the backend by mean the third value of a Drab event handler call. [...]

where I proposed to have a sort of a new helper to call Drab from templates, in which the value of the parameter is encoded by default (currently instead, is passed as a plain string, so encoded is manually needed every time the parameter is i.e. a struct)

Got it.

With the new API you would be able to do:

<% encoded_value = Drab.Coder.encode %{question: "foo", answer: 42} %>
<button  drab='click:check_answer("<%= encoded_value %>")'>Check answer</button>

and

defhandler check_answer(socket, sender, value) do
  decoded_value = Drab.Coder.decode(value)
        
  question = decoded_value[:question]
  answer = decoded_value[:answer]
end

Or of course you could choose the other encoder than default (which will be cipher+base64)

I am now working on it, should finish soon :)

Yes, but what I mean is a suggestion to write an helper and to modify somewhere the Drab.Core, to avoid the necessity to call by hand Drab.Coder.encode/x, as per the example in my previous message:

# helper in the template
<button drab=<%=drab_handler(:click, :check_answer, %{question: "foo", answer: "42"}, encrypted: true) %> >Check answer</button>

# in the Commander handler, automatic decoder of the incoming value
defhandler check_answer(socket, sender, value) do        
  question = value["question"]
  answer = value["answer"]
end

It does make sense for you?

Hmm I believe it is overcomplicating.

IMO in this case explicitness is better, also because in 99% case you don't need the encoding.

Automatic decoder in handler would also be problematic, as we need to differentiate between encoded and non-encoded values.

I have created coders, #137

Hmm I believe it is overcomplicating.
IMO in this case explicitness is better, also because in 99% case you don't need the encoding.

I understand your point of view. Probably mine is influenced by the fact that I often need to pass more than just one value to a Drab call, and then I would like to pass transparently any kind of value as I were calling a standard function, but, as you said, probably this is not the need of most of the audience.