Serialization of parameters with multiple values ([] brackets)

Why the change to hashes in the first place? Keep it as strings! That's what the HTTP protocol uses, that's what servers expect, that's what programmers are used to. By switching the inner workings to a hash as well you've introduced unexpected behavior.

First, don't convert passed query strings to a hash (as per your last Ajax patch), then back to a string. We lose performance to no advantage and introduce bugs with increased complexity. (For example, your patch also overwrites '_method' and '_', instead of appending them, and--although I haven't tested this part--has the potential to mess up the passed parameter order if for some reason the server is fragile enough for it to be important.)

Rewrite the Ajax changes to convert passed hashes immediately to a query string. (I'll do it if you need em to.) The key should be used just as inputted. i.e. 'foo':[1,2] => foo=1&foo=2; 'foo[]':[1,2] => foo[]=1&foo[]=2.

Copying comment made by slusarz on #6366:

Part of the code committed in this ticket badly breaks Ajax.Request(). For example, you can no longer pass in the following parameter string:

foo1=bar1&foo2[]=bar2&foo2[]=bar3&foo2[]=bar4

This is a perfectly valid string under RFC. However, the new code attempts to convert this string from string to array and back to string, in the process mangling and losing parameters.

Since PHP uses the 'foo[]=' syntax to pass an array of data across page loads, this is a major issue.

Patch:

--- prototype.js        18 Nov 2006 04:01:12 -0000      1.3
+++ prototype.js        18 Nov 2006 06:47:42 -0000
@@ -714,8 +714,8 @@
     Object.extend(this.options, options || {});

     this.options.method = this.options.method.toLowerCase();
-    this.options.parameters = $H(typeof this.options.parameters == 'string' ?
-      this.options.parameters.toQueryParams() : this.options.parameters);
+    this.options.parameters = typeof this.options.parameters == 'string' ?
+      this.options.parameters : this.options.parameters.toQueryString();
   }
 }

@@ -732,21 +732,21 @@
   request: function(url) {
     var params = this.options.parameters;
-    if (params.any()) params['_'] = '';
+    if (params.length) params += '&_=';

     if (!['get', 'post'].include(this.options.method)) {
       // simulate other verbs over post
-      params['_method'] = this.options.method;
+      params += (params.length ? '&' : '') + '_method=' + this.options.method;
       this.options.method = 'post';
     }

     this.url = url;

     // when GET, append parameters to URL
-    if (this.options.method == 'get' && params.any())
-      this.url += (this.url.indexOf('?') >= 0 ? '&' : '?') +
-        params.toQueryString();
+    if (this.options.method == 'get' && params.length)
+      this.url += (this.url.indexOf('?') >= 0 ? '&' : '?') + params;

Replying to tomg:

Why the change to hashes in the first place? Keep it as strings! That's what the HTTP protocol uses, that's what servers expect, that's what programmers are used to.

Programmers are certainly not used to handle various parameters inside a string - they like working with associative arrays or hashes. Think about how you handle POST params in your favorite language. Why change to hashes? Because it's easier to handle, inspect, extract or change a single value; it's easier to merge with another hash; and because we really don't need a serialized query string until the very end (when the actual HTTP request is being made).

A serialized string is just transport. Semantically, these are parameters and they belong to arrays and hashes because these language types are here to hold that kind of data. You should encode your data for transport as late as possible.

Slusarz is wrong. Prototype will not mangle this string:

foo1=bar1&foo2[]=bar2&foo2[]=bar3&foo2[]=bar4

So, the only problem remains deciding how to solve the issue of having this serialized for a PHP backend:

{ foo: 'alpha', bar: ['beta', 'gamma'] }

and deciding whether to parse this:

bar[]=beta&bar[]=gamma

to key 'bar[]' or 'bar'. Hmmm...

Replying to mislav:

Slusarz is wrong. Prototype will not mangle this string:

I respectfully disagree. This parameter string:

folder=foo&uid[]=10801&uid[]=10802&uid[]=10803

worked fine in rc0 and prior. In rc1, the following POST request is sent to the server:

folder=foo&uid%5B%5D=10803&_=

As can be seen, param values 10801 and 10802 are nowhere in sight.

At the very least, this change breaks BC. But worse, the new code also makes wild assumptions about the incoming parameter string. It should be the programmers job to format the parameter string exactly the way they want it and Ajax.Request should simply pass along this string to the server. I don't want Ajax.Request to "tidy" up my string, or try to make it more efficient, or try to "figure out" what I meant to pass in. It's as if you are assuming that a programmer is not smart enough to figure out how to correctly create a valid parameter string and feel the need to be smarter than the programmer in trying to fix something (that may not be broken).

Slusarz, I am confused. Are you using the latest trunk? Because when I type:

$H("folder=foo&uid[]=10801&uid[]=10802&uid[]=10803".toQueryParams()).toQueryString()

I get:

"folder=foo&uid%5B%5D=10801&uid%5B%5D=10802&uid%5B%5D=10803"

Exactly as I should. We fixed this not so long ago - you shouldn't go around reporting errors before you check out the trunk.

Replying to mislav:

Programmers are certainly not used to handle various parameters inside a string - they like working with associative arrays or hashes.

It's not the acceptance of hashes at the front end that bothers me. I think it's a good idea. It's the internal transformation from string to hash then back to string that troubles me. Accept the passed string without modification. Convert a passed hash to a string.

Further, I would suggest *not* changing the output of Form.serialize to be a hash. First, it's not backward compatible--it would certainly break several of my applications. A different function that converts a query string to a hash would could accomplish your goal.

Replying to mislav:

Slusarz, I am confused. Are you using the latest trunk?

I do apologize - my local svn setup was borked and I was not correctly receiving updates. The issue I originally reported has indeed been fixed.

Replying to mislav:

The problem is: how to serialize foo:[1,2] - with or without the square brackets? Do we make this configurable with extra param to the function call? What will be the default then? Should "foo[]=1&foo[]=2" become foo:[1,2] also (instead of "foo[]":[1,2])?

This has received enough discussion and I think we can all agree that the problem does not exist as long as the developer is aware what backend he is dealing with. Juggling with square brackets internally is unnecessary magic which would just make JavaScript serializing mechanism different from that of browser, which is a bad thing. At first I though something needs to be done here, but now I'm convinced that the best course of action is to leave stuff as it is now.

Should one have a need to manually post a hash like foo:[1,2] to a PHP or Rails backend, they should write a method that adds square brackets for them on all keys that have array values.

Should one have a need to manually post a hash like foo:[1,2] to a PHP or Rails backend, they should write a method that adds square brackets for them on all keys that have array values.

What about multi-dimensional arrays: foo:{a:'one',b:'two'} => foo[a]=one&foo[b]=two?

Zaadjis: we haven't implemented the nested hash feature, and there is a chance we never will. Serialization process is all about forms, anyway. With a form, you could never have foo:{a:'one',b:'two'}, only this:

{'foo[a]':'one', 'foo[b]':'two'}

And that would get serialized correctly. If you want to send complex objects to the server you should better consider JSON.