search

apeisa / Multisite

Adds support for subsites run from separate domain, but using single database (same admin site and user database).
12 stars 17 forks source link

modifyPath() potential issues #4

Open mindplay-dk opened 11 years ago

mindplay-dk commented 11 years ago

This looks potentially dangerous:

$event->return = str_replace("{$this->subdomain}/", '', $event->return);

https://github.com/apeisa/Multisite/blob/master/Multisite.module#L74

A simple str_replace() will replace all occurrences of anything that looks like the sub-domain - I believe the correct thing to do here would be something along the lines of:

$event->return = substr($event->return, strlen($this->subdomain) + 1);

Also, I wonder, is Module::init() the right place to modify the $_GET['it'] superglobal? Aren't other modules (and the ProcessWire core itself) potentially initializing based on the "old" value prior to modification?

I'm vetting this module for use in a large multi-tenant site - have you used this module in production in a real multi-site scenario?

Thanks!

somatonic commented 11 years ago

I'm currently working on a modified version of this module to support mutlilanguage setup using core language support and the new language page names support.

I think some of the methods in the current are questionable but mostly fine. I agree that the str_replace maybe not the best here and thanks for mention here mindplay!

I'm currently building a large multilanguage site that will also need multisite/multidomain setup at some point for partner sub websites.

So far I got the module working fine and it works surprizingly well and even with LangaugeSupportPageNames that also "modifies/reads" the $_GET['it']. Getting and modifying it in the init() is the only way to do it as after that ProcessPageView does remove it after processing. Of course one would think this could lead to problems but it's actually working fine (as far as my test say). It's also of course possible that it in some cases it could create problems but I see it pretty solid as we are only modifying the requested url and the path PW spits out when using $page->url. It seems dangerous but works fine so far. LanguageLocalizedUrls module was also working in the same manner and it was even working with this module too. It's almost magic as I got also worries at first and never thought this would work.

apeisa commented 11 years ago

Thanks for the discussion guys. I have this running on one site/installation with four domains. Works fine there, but remember nasty problems in some phase of development.

Never used this with multilingual sites.