Smf2.1: $settings['theme_url']

Created on 14 Dec 2016  路  52Comments  路  Source: SimpleMachines/SMF2.1

Each $settings['theme_url'] and $settings['default_theme_url'] doesn't respect SSL. It still uses http after change to SSL. This causes most ajax request to be blocked by the browser. loadJavaScriptFile() also uses $settings['default_theme_url'] which prevents scripts form being loaded.

When doing the switch to SSL a small function needs to be called to change all DB stored urls to the new schema or store all local urls as as a token {localURL} and do a replacement when retrieved from the DB.

The urls can be changed from within the admin panel but its not very obvious, might be better to just do it automatically for the user when doing the change to SSL or as a routine maintenance task.

Enhancement Minor Bug

Most helpful comment

Why not register those urls as schemeless url ?

All 52 comments

why is this information in the database?

The key of my question is, why we got http(s) url in the db,
when we know they could change.

Why not register those urls as schemeless url ?

Agreed, schemeless URL's should be a milestone for Beta 4. That would fix this issue too.

It will be better to use tokens, this way you will also take into account domain changes. Tokens will also solve the issue with local urls stored on messages or other tables. It will be trivial to do a str_replace on preparsecode() to change all $scripturl links to {scripturl} or another more specific name and revert it back when parsing, this means links and urls will always be updated.

@albertlast This is a remnant from the very early days, it was done this way for performance but nowadays it doesn't matter that much.

In my eyes is // already a token,
so what would be the advantage of define another one?

I'm going to defer this to RC1.

In my eyes is // already a token,
so what would be the advantage of define another one?

Then if the value of $scripturl changes from, e.g., http://www.example.com/forum/index.php to https://example1.org/index.php, there will be no need to go through all the database and replace the old value with the new value.

The same behavior would be reached when // is used and
places where this information need hardcoded to replace at runtime the // with https:// or http://
So in my eyes is no need to define a new token, we should use the // as token.

You are correct that both methods would solve the http vs. https issue, @albertlast. If we limit the scope of our concern only to handling changes to the URL scheme, then schemeless URLs are indeed the more graceful solution.

The reason that @MissAllSunday's idea to use a {scripturl} token is better is that it handles the more general problem of changes to the URL of a forum. A change from http://www.example.com/forum/index.php to https://www.example.com/forum/index.php is a specific instance of the more general problem. If we use @MissAllSunday's idea, we'll be able to handle not only changes to the URL scheme, but any and all changes to the forum's URL.

How would be look the url with script url?
{scrupturl}/forum/index.php
or different?
you mean ALL url changes, how you handle then when the board is moved in sub dictory?
exp from https://www.example.com/forum/index.php -> https://www.example.com/index.php or https://www.example.com/forum/v2/index.php

The value of $scripturl is always the full URL of SMF's index.php. That's the whole point of @MissAllSunday's suggestion. :)

It would work like this:

Full URL | Stored string
----------|------------
http://www.example.com/forum/index.php?topic=8.0 | {scripturl}?topic=8.0
https://forum.example.com/v2/index.php?action=profile;area=statistics;u=1 | {scripturl}?action=profile;area=statistics;u=1

But what is the "right" resolve of this?
should be {scripturl}?topic=8.0 return http://www.example.com/forum/index.php?topic=8.0
or https://www.example.com/forum/index.php?topic=8.0
or //www.example.com/forum/index.php?topic=8.0

If the forum is using SSL, it'll be https. If not, it'l be http. Basically, you can always trust the value of $scripturl to give your the correct URL, including the correct URL scheme. The reason is simple: if $scripturl contains an incorrect value, the forum won't load. If the forum has loaded, then $scripturl is correct. So if we use the value of $scripturl we don't need to use schemeless URLs to refer to our own resources, because we already know the correct URL scheme.

How does that solve $settings['theme_url']?

Since themes are stored in a standard path relative to the forum's root directory, the correct value for $settings['theme_url'] is dependent on the URL of the forum root. By using a placeholder token when storing the value of $settings['theme_url'] in the database and then substituting the real URL of the forum root into the string when we retrieve it from the database, we can guarantee that the final value of the $settings['theme_url'] variable will always include the correct URL scheme, domain, and path.

In my comments above, I talked about using $scripturl for this job, but it occurs to me now that it'd be better to use $boardurl, since it is the "master" value stored in Settings.php from which $scripturl, etc., are derived.

So how would it help for those who don't have $settings['theme_url'] following the same url as $scripturl? I.e, check out simplemachines.org urls.

A simple check when saving the value to see if it starts with the same value as $boardurl would take care of that. If the user decides to use a custom value, she takes on responsibility to manage the URL herself.

One solution could be to add a note in the admin panel to add {scripturl} for the base path, but that's not really user friendly. The other solution would be to check for $scripturl inside the string and replace that automagically.

The other solution would be to check for $scripturl inside the string and replace that automagically.

Exactly. A simple str_replace() call just before saving the value would do it. And then another to restore the value when it is retrieved. That's it.

If the admin decides to change $settings['theme_url'] to a URL somewhere outside the forum's root, the str_replace() won't find any match and therefore will not change what the admin entered. In such a case, it would fall to the admin to make sure the URL scheme was correct. But by the time one is dealing with that level of customization, we're no longer dealing with something an automated system could handle correctly anyway.

That solution sounds best to me. It's simple and should fix the issues for most users.

This sounds like you want to store different values than what are displayed. A hidden layer of updates, that will not match what the admin actually entered.

I really think you want the admin to see the real values, or diagnosing issues becomes significantly more difficult.

How many places are urls referenced throughout the codebase? All would need to be changed, correct?

How do we want repair_settings.php to behave? Should it display the real urls, or the hidden token versions? Why?

If there is an issue with a theme url setting, that is fairly easily identified, and fixed in repair_settings.php today, correct?

I may be really missing something, but upon first read, this idea feels unnecessary, kinda dangerous, and like a good chunk of work...

I'd opt for something much simpler, like highlighting non-https entries in repair_settings.php.

Just a thought...

Sorry... Didn't mean to be a threadkiller...

This is the type of thing that makes me nervous about the token approach:
Installer - Paths & setting not set #3999

Going back to the original post above, I kind of like the initial approach suggested: "When doing the switch to SSL a small function needs to be called to change all DB stored urls to the new schema..."

I.e., focus on the problem, & fix the theme settings as part of enabling SSL.

i don't understand why this switch "to https" exists in my eyes is this wrong from the start.
When a user came by non https than he can go forther without and when the next one came by https than he keep on https.

I would expect when we safe the url,
we do a match "https://smf.de/long/url..." = http(s)://boardurl -> replace the url with the token = (token)/long/url...

Schemeless URLs vs tokens should be a 3.0 discussion at this point.

So I see two options: Close this, or write the code snippet that ensures https on the 6 url settings, including themes, when force_url is set to 2 in the admin panel. (Installs were handled in another issue/pr.)

My suggestion is to close. I don't like "hidden" settings changes, they take control from the admin. This issue is easily resolved via the admin panel or repair_settings.php today.

It wouldn't be a hidden setting since you will do it exactly when the admin decides to go for https (at install, at upgrade, directly on the admin page, etc). Indeed this can be solved by the admin but the admin will be doing SMF's job. If this can be done in an automated way, why deferred it to the admin?

The fact that we have 6 configurable URL types (board, smileys, avatar, custom avatar, theme & theme/images [one pair per theme]) for the admin to set means it's the admin's job. If we want it to be SMF's job, we should remove those configurations & assume SMF control. But we haven't, so I disagree with you on that point: it is not SMF's job.

My concern is that the instant we hard-code everything to https without consulting the admin, we are stepping on settings owned by that admin. We open ourselves up to, "Hey, I wanted my theme images over on xxxxx, why did SMF change that?"

There are ways around this:

  • Provide a warning when force_ssl is set to 2 and any of these 6+ are still http. Maybe even display which ones.
  • One step further: prompt & ask if the admin wants them changed & change them all only when the admin approves. (If we were to go down this path, I strongly suggest confirming a cert is active first, otherwise we open ourselves up to a lot of disappearing forums!)

These would be acceptable compromises, I think. And they will make SMF a bit friendlier during this process.

The question is whether it is really necessary, when we already have tools for the admin to address. I don't think it is - which is why my suggestion is to keep this simple & fully under admin control, i.e., just close this issue. Or - doc a recommended solution & flag it as a future enhancement request.

(I think the bigger problem on the https front is that setting force_ssl to 1 doesn't work AT ALL... #2939 )

You are mixing things up.

The reason why SMF has so many configurable vars is to allow the admin to chose to set different urls/domains for those, this is mainly used for things like setting a new domain for static content (like simplemachines.org does).

That is fine, that is good, the proposed change won't mess with that or the admin's decision to set those urls as s/he pleases.

The proposed change has to do with changing all those urls to https WHEN the admin chose to do so AND if those urls aren't custom defined ones.

If an admin decides to go for https THEN and only THEN all those urls will be changed, SMF won't do it arbitrarily or against the admin's choice.

If the admin decided to go for https and the urls are the default ones then it will be trivial for SMF to change those urls on behalf the admin, not only it will avoid possible surprises to the admin but to avoid having them to do it themselves and mess things up.

This is just really to make the admin's life easier.

That part that confuses me in your response is what you mean by 'custom defined'...

The theme urls are set under admin control, just like the others mentioned above. All are set by the admin.

I don't think we should set them without explicit admin approval. S/he may not realize we're going to step on them when changing the force_url setting. My compromise solutions up there make that explicit.

I repeat, no one, not SMF, not us, not anybody will do ANYTHING without the admins approval.

SMF won't set any urls, those urls are already set, they are defined and set on install.

The only change SMF will do to those urls is changing from http to https and only, I repeat, ONLY after the admin CHOSES to do so.

by custom defined I mean when the admin HAS those urls changed (ike simplemachines.org has), in that case SMF shouldn't do anything.

The only change SMF will do to those urls is changing from http to https and only, I repeat, ONLY after the admin CHOSES to do so.

The above sounds like, upon user selection of force_ssl = 2, you propose displaying a list of URLs that remain http, and let the admin choose which ones to change to https?

That would work & address my concerns. Feels like overkill, but it would address my concerns.

No.

We are in two different channels, lets leave it at it.

If anyone else knows about what I'm talking about please do try to add this change, its a pretty simple one and it really should be done to make sure all urls are changed to https when the admin uses that option.

To me, the parts that feel contradictory are:

those urls is changing from http to https and only, I repeat, ONLY after the admin CHOOSES to do so
and:
make sure all urls are changed to https when the admin uses that option

It would be extremely easy to write code to loop thru some URLs and change them. At question is how much visibility should the admin have into exactly what specific set of changes are made beforehand.

Which brings me to my other question, which is, which set of URLs? Is it all 6 I have listed above, or some subset? If a subset, why a subset?

I still don't sense a holistic solution spelled out. On an item that is flagged as holding up RC.

When you switch to https then any http url should be changed to https otherwise you'll get mixed content warnings.

Its what I would expect to happen otherwise it's just hassle to go through them all.

Schemeless URLs are even better but for some reason the SMF team doesn't like them.

Yes, tinoest... thats the whole point I've been trying to explain here but it seems I miserably failed.

It really is a small change, I will see if I get some time over the weekend to send a PR.

edit,typos

Just a couple questions to keep in mind:

  • Is the proposal restricted to admin panel updates only?
  • Should we check for a cert first?
  • Which URLs? All of them, including $boardurl?
  • What to do if they change force_ssl to 0? _Should they all be changed back to http?_
  • What to do if they change force_ssl to 1? Anything? (kind of a trick question, as this doesn't work...)
  • How will the admin know what changes were made?

Last attempt.

There are 3 ways to force ssl on a forum:

  • On install
  • On upgrade
  • From the admin panel

On those 3 cases ALL default urls needs to be changed to https. This isn't something I decided on and def isn't some whim of mine. When forcing SSL all urls needs to be changed to avoid mixed content or worst, having the browser to completely reject the server petition.

Lets see the procedure for each case:

  • On install the urls aren't set yet so this is just a matter of define them on https

  • On upgrade check if the urls are all based on $scripturl, this means the admin hasn't changed them to custom values. If they indeed are based on $scripturl, make the change.

  • From the admin panel Same behavior as on upgrade.

Please do note that this is a key part: check if the urls are all based on $scripturl, in fact, allow me to bold it: check if the urls are all based on $scripturl

If an url isn't based on $script it means the admin already modified that url, for example, to set it to a custom domain for static content. In this case SMF shouldn't modify that url at all. I can argue that SMF should also attempt to change those urls too but since making you guys to understand changing the $scripturl based ones its resulting to be pretty difficult, it will be impossible to make you guys to go for changing the custom ones.

The check to force ssl on a forum should have to deal with making all the necessary checks, once those checks were done and everything is in order then SMF needs to change all configurable urls to https.

By ALL urls I do mean ALL of them. No a subset, not a part, not the ones I like. All of them.

NO, this doesn't mean "stepping on the admin's toes".

No, this doesn't mean taking away the admins choice either simply because there IS NO CHOICE. You CANNOT chose to have some urls on http and others in https, its full or nothing, black or white, 0 or 1, true or false.

So, working under this premise (full https or nothing), one can easily deduct that theres no need to let the admin to "know" or to "chose" which urls to convert, in fact, as tinoest has pointed out already, when an user decides to go https it is expected for SMF to change all urls where applicable.

Lets present two cases here:

Admin 1 is a poweruser with a big forum
Admin 2 is a newbie, shared hosting dude/tte.

Admin 1 has proudly move all her images to a static server and has changed the urls accordingly.
Admin 2 uses the default SMF behavior where images are stored in a relative path to the same domain. (Themes/default/images)

Both admins has decided to use https.

With this set up in mind, the current approach for both admins is as follows:

Make the change to ssl and change all urls using repair_settings or some other tool.

What _I propose_ is the following:

Admin 1 _still_ has to change their custom urls via repair_settings or whatever some other tool they prefer.

Admin 2 doesn't need to use repair_settings because SMF will change ALL urls for him/her.
Admin 2 won't be coming to the support forum asking why his/her forum doesn't work because the browser rejected the connection because of mixed content.

Thats all. The proposed change is intended to make life easier to those newbies who will otherwise struggle more than really needed.

I really hope you guys can finally see my point here, otherwise I'm out of ideas.

I'm just finishing off some code to implement (my version of) @MissAllSunday's token idea. What it does is quite simple. Just before we write to the database, any occurrences of $boardurl in the values to be written are replaced with a token. The moment anything is retrieved from the database, any occurrences of the token are immediately replaced with the current value of $boardurl.

All your other concerns about force_ssl, etc., are a separate matter, @sbulen. I know you have put a lot of thought into how to handle all that, but what this change does is quite simple. Once whatever has been done that needs to be done regarding force_ssl in order to set the correct value of $boardurl, this change simply ensures that the current value of $boardurl is applied seamlessly to values that were already set to use $boardurl, and those only. If the admin decides to use a different URL in any of the settings, that will be retained just as the admin entered it.

Well, I was trying to push this RC dependency towards crisp definition & resolution.

Now I have not one, but two solutions... I'm still not sure I know how either solution really addresses my 6 questions above in my last post. Guess I'll have to test 'em as they come...

@MissAllSunday - Thank you for your input & time & thoughtful responses. Bear in mind "you guys" was just me! (No need to take it out on anyone else!)

Correct me if I am wrong, @sbulen, but #4245 deals specifically with determining the correct value for $boardurl during the installation process, right? In the comments on that PR, you say that this is the key for determining the correct values for the other settings during install. That's perfect, because we do indeed need some code to set all those values correctly to begin with.

4264 complements that work by ensuring that we remember which of those URLs were based on $boardurl, so that if the admin changes the value of $boardurl (e.g. by activating or deactivating SSL) at a later time, everything will be updated at once.

So basically, your code figures out whether $boardurl should start with http: or https: and sets it that way, while mine ensures that once the value of $boardurl has been determined, we use it consistently.

To answer your questions directly:

Is the proposal restricted to admin panel updates only?

4264 deals with any and all strings that contain the value of $boardurl whenever they are retrieved from or written to the database. So, no.

Should we check for a cert first?

Sure, but that has nothing to do with #4264.

Which URLs? All of them, including $boardurl?

$boardurl and any that are based on $boardurl will be affected by #4264, and no others.

What to do if they change force_ssl to 0? Should they all be changed back to http?

4264 is agnostic on this matter. All it does is make sure that if $boardurl changes for whatever reason, that the change is reflected in all URLs retrieved from the database that start with $boardurl.

What to do if they change force_ssl to 1? Anything? (kind of a trick question, as this doesn't work...)

See previous answer.

How will the admin know what changes were made?

If the admin changes $boardurl in some way, the admin will happily discover that all instances of $boardurl in any settings and post throughout the site have been updated instantly. They will know it has happened because, well, they changed $boardurl.

See my comments over on PR #4264 .

In short - I'm wary about such a broad, fundamental, potentially code-destabilizing change this late in the game. IMO, this belongs in the 3.0 discussion.

And it doesn't address MissAllSunday's initial concern about what to do when we change the force_ssl setting. MissAllSunday's approach is closer; it is starting to take shape.

With latest commit there is no problem with http(s) and themes url. Im guessing recent pull request about http(s) solved these one to. There is still protocol and domain in database but i would just leave it as it is since its working the way it should. (Maybe close issue now???)

@NegativeIQ - for installs and upgrades, I think you are correct. The remaining question - raised in the original post above - is how to deal with changes to the force_ssl setting made in the admin control panel.

Should the various URLs be updated automatically when the setting is changed? (When setting force_ssl to 2? To 0?)

Should the various URLs be updated automatically when the setting is changed? (When setting force_ssl to 2? To 0?)

Yes.

@sbulen yes it should automatically change depending on request and if we are forcing ssl. Basically themes and images should follow request protocol. Other links may or may not (these is arguable) follow http(s) protocol but resource files must (if they dont browser will raises warning/error).

OK, I guess I'm outnumbered.

I'll take a stab at this, following @MissAllSunday 's input above. (Unless someone else is already working on it...)

Just to add some more weight, this type of support request will become more common: https://www.simplemachines.org/community/index.php?topic=555935.msg3940343#msg3940343 The proposed change will help to mitigate those requests.

@MissAllSunday for 2.0.x maybe/probably, but not for 2.1 since everything is being handled automatically in 2.1 (or im missing something, but im pretty sure i manually tested every situation except external content and image proxy)

The gap that remains is the admin control panel - folks who do the conversion while in 2.1. This change removes the need to bounce around the acp or repair_settings.php to fix urls.

PR #4282 submitted.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jdarwood007 picture jdarwood007  路  5Comments

sbulen picture sbulen  路  6Comments

jdarwood007 picture jdarwood007  路  3Comments

NegativeIQ picture NegativeIQ  路  11Comments

Gwenwyfar picture Gwenwyfar  路  4Comments