Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

modifyPath() potential issues #4

Open
mindplay-dk opened this issue Jul 15, 2013 · 2 comments
Open

modifyPath() potential issues #4

mindplay-dk opened this issue Jul 15, 2013 · 2 comments

Comments

@mindplay-dk
Copy link

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
Copy link

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
Copy link
Owner

apeisa commented Sep 12, 2013

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants