It is possible to create broken site or duplicate sites nodes because there is not enough strtolower(trim(domain)) used

Indeed, that's a bug. Not sure we need to fix it in all those places, but I don't think it would hurt and it would be safer. I think this is a critical bug, marking as such.

this actually works fine for me on head, but this puzzles me :

function hosting_site_allow_domain($url, $params = array()) {
  $query = "SELECT COUNT(n.nid) FROM {node} n 
    JOIN {hosting_site} h ON n.nid = h.nid
    WHERE type='site' AND title='%s' AND h.status <> %d ";
  $args[] = $url;
  $args[] = HOSTING_SITE_DELETED;

 // WTF IS THIS DOING ?  
  if (isset($params['client'])) {
    $query .= ' AND h.client <> %d';
    $args[] = $params['client'];
  }

  if (isset($params['nid'])) {
    $query .= ' AND n.nid <> %d';
    $args[] = $params['nid'];
  }
  $result = !db_result(db_query($query, $args));
  return $result;
}

The client bit is basically saying that the client can create the same site twice, as long as the site belongs to him ?
this code comes from pre-git days.

confirmed that was the error.

my test site was owned by a different client, but it was allowing you to create multiple copies of the site for the same client.

[master 842e252] Logic error allowing a client to create multiple copies of the same site. #949044
1 files changed, 0 insertions(+), 5 deletions(-)

it was also being done for aliases. there it was even stranger. the logic was allowing the user to create an alias for a site, that is the same value as the that for one of the other sites it owns, including the site proper. Meaning it was attempting to allow you to have the vhost loading to have a crapshoot about which alias was used, based on the alphabetical precedence of the parent site.

[master e90196d] incorrect logic for clients and aliases. same issue as #949044
1 files changed, 0 insertions(+), 5 deletions(-)

This seems to have come in with the old ports stuff - http://is.gd/gAeVX , which has since been changed significantly.

I believe the original intention was something along the lines of not allowing another site to create an alias or a new site running on a different port from yours.
ie: another client couldnt create clientsite.com:8080 on top of the existing site clientsite.com.

That's all irrelevant now, with the new port system.

Applied the more ambitious patch:

http://gitorious.org/aegir/hostmaster/commit/983a0ee8aeca7e079453a88543d...

Some of those are weird for me:

   $url = strtolower(trim($form_state['values']['parameters']['new_uri'])); // domain names are case-insensitive
-  if ($url != $site->title) {
+  $form_state['values']['parameters']['new_uri'] = $url; // force lowercase

Why assign the url back to the form values?! Why not just:

if (strtolower($url) != $site->title) {

?

The patches should be fixed - i think they shouldn't modify the $forms array, and the comment is wrong (the assignment doesn't turn into lowercase). I do not feel the resulting code is more readable, for me at least.

imported both commits in stable and master branches. please don't include the "BOA" prefix or "(grace)" suffix in commit logs, as then i can't cherry-pick them directly...

merged in 2.x and 1.x