Page 1 of 1

"action" parameter considered harmful

Posted: Mon Sep 15, 2014 9:27 pm
by cmb
Hello Community,

actually the title of this topic is exaggerated, but indeed, the "action" parameter commonly used for the plugin administration can cause problems. Consider the following form:

Code: Select all

<form action="./?&pluginname" method="post">
  <input type="hidden" name="admin" value="plugin_main">
  <input type="hidden" name="action" value="plugin_save">
  <!-- more form controls -->
  <input type="submit">
</form>
Now consider you want to modify the action attribute of the form element, say in the onsubmit handler, to add a further parameter. In an ideal world the following would suffice:

Code: Select all

function onSubmit() {
    this.action += "&param=value";
}
However, this will result in the form being submitted to http://example.com/[object%20HTMLInputElement]&param=value. :shock: (For an explanation see http://pointedears.de/scripts/faq/cljs/names/.)

I would suggest to rename the "action" parameter, but that would break most plugins. Simply adding an alias (e.g. xh_action) also doesn't solve the problem cleanly, because a plugin that would use the alias only, would not be backward compatible, or it would have to jump through hoops by using the proper name conditionally (based on which condition?).

Any ideas how to solve the issue?

Christoph

Re: "action" parameter considered harmful

Posted: Wed Sep 17, 2014 7:29 am
by manu
..and would break all admin forms either.
But what's the results difference between a form method="get" and a hidden "action" input field and the manipulation of the form action property('s query string)? Why not manipulate the value of a hidden field? If you want to manipulate the form target itself then why not take document.url?
For purity and conformity I would follow your suggestion to invent a xh_action and replace the action field (1.7 - 2.0?) with a well shout out announcement to the plugin developer.
In the meantime IMHO it suffice a notice in the Wiki's Developer JS section.
just my 2 cents.
manu

Re: "action" parameter considered harmful

Posted: Sat Sep 20, 2014 10:20 am
by cmb
manu wrote:But what's the results difference between a form method="get" and a hidden "action" input field and the manipulation of the form action property('s query string)? Why not manipulate the value of a hidden field?
Of course, manipulating the value of a hidden field has the same effect (unless one needs a parameter without value, such as hypothetically changing "pagemanager" to something else). And one can use getAttribute() or at least getAttributeNode() to get the value of the form's action attribute. And indeed, the use case brought up by me might be completely hypothetical -- at least I never had this issue...
manu wrote:For purity and conformity I would follow your suggestion to invent a xh_action and replace the action field (1.7 - 2.0?) with a well shout out announcement to the plugin developer.
... but exactly purity and conformity were my reasons to mention this issue, and that we should try to avoid to make it harder to use JavaScript than it already is, i.e. to prevent yet another WTF? moment.

However, I'm still not comfortable making such a change for XH 2.0, let alone 1.7 -- at least unless there will be other more important changes that would require modifications to existing plugins. We should not forget about unmaintained plugins (published and private), and that such changes might hinder the adoption of new XH versions.
manu wrote:just my 2 cents.
Thanks. :)

Re: "action" parameter considered harmful

Posted: Tue Sep 23, 2014 7:40 pm
by svasti
cmb wrote:unmaintained plugins (published and private)
that's a problem. In an XH 1.4 for a book shop I have an old newsletter plugin from Jerry (form the time is was still free), upgrading XH would mean they need a new version of the newsletter which they will not pay. :( So I leave things as they are.

Re: "action" parameter considered harmful

Posted: Tue Sep 23, 2014 11:35 pm
by cmb
svasti wrote:So I leave things as they are.
Unfortunately, that is sometimes the only viable option. However, that may often end up bad. For instance, the provider might switch to a new PHP version, and suddenly the website doesn't work anymore... (let alone that the site falls victim to an attacker exploiting a vulnerability that has been fixed in a newer version[1]).

All in all, we have to find a good compromise between keeping backward compatibility and eliminating flaws resp. switching to modern practises. Neglecting or even ignoring the latter would make CMSimple_XH to be a dying breed.

[1] For CMSimple(_XH) sites that might not be too much of a problem, but if the site was built with WordPress for instance, I would really be worried.