Serendipity Forums

Posted: **Sun Sep 25, 2011 7:53 pm**

HTTPS handling of Serendipity is not optimal.
To use both, HTTP and HTTPS, "Autodetect used HTTP-Host" has to be set. This causes "URL to blog" to change depending on the domain name or protocol.

The first thing that should be optimized is the port. When calling Serendipity over HTTPS with the setting above, the URL changes to https://example.com:443, but it would be better if it would just be https://example.com. The Port should only be added if it is not 80 for HTTP and not 443 for HTTPS.

The second and more severe issue is that I can't use the backend over HTTPS when I want my users to come over HTTP by default. For instance when I write a new article, all the paths in the sitemap (generated by the sitemap plugin) now begin with https://example.com:443/ instead of http://example.com/.

Therefore the setting "URL to blog" should be a fixed default setting used for things like this. It should not change when "Autodetect used HTTP-Host" is activated. Of course, the general path handling should remain the some, otherwise I would get mixed content on HTTPS connections when "URL to blog" is a HTTP URL, but for things like generating the sitemap, a fixed URL should be used, regardless of the domain or protocol I'm currently using for the backend.

Posted: **Mon Sep 26, 2011 11:25 am**

Hi!

The first thing that should be optimized is the port. When calling Serendipity over HTTPS with the setting above, the URL changes to https://example.com:443, but it would be better if it would just be https://example.com. The Port should only be added if it is not 80 for HTTP and not 443 for HTTPS.

Good idea! I've committed this.

The second and more severe issue is that I can't use the backend over HTTPS when I want my users to come over HTTP by default. For instance when I write a new article, all the paths in the sitemap (generated by the sitemap plugin) now begin with https://example.com:443/ instead of http://example.com/.

Technically, that's quite a problem. s9y relies on the serendipityBaseURL on quite a few plugins and instances, which is where the autodetect kicks in and modifies it accordingly, when autodetection is on.

More sever things might break in this scenario if the HTTP host does not get changed (cross-domain security js issues, ajax callbacks not working, possible problem in internal/external domain scenarios).

You specifically mention only the sitemap problem; this shouldn't be a problem for crawlers? Do you have anything other in mind that really breaks your backend when using SSL?

Fixing the sitemap (for URLs that would work fine actually) would IMHO not have a bigger advantage over possibly breaking other scenarios where the s9y configured URL must adhere to the currently used URL...

Regards,
Garvin

Posted: **Mon Sep 26, 2011 11:34 am**

Technically, that's quite a problem. s9y relies on the serendipityBaseURL on quite a few plugins and instances, which is where the autodetect kicks in and modifies it accordingly, when autodetection is on.

I know. My suggestion would be to leave serendipityBaseURL as is, but implement a second variable (e.g. serendipityDefaultBaseURL). This is the variable I change in the backend and this variable doesn't change its value.
serendipityBaseURL, however, is not directly modified by the user and gets the value of serendipityDefaultBaseURL automatically. And if "Autodetect used HTTP-Host" is set. only serendipityBaseURL changes.

That doesn't break anything but plugins such as the sitemap plugin can use serendipityDefaultBaseURL to generate the paths. That prevents duplicate content or inconsistent scrawling and indexing due to changing paths but still maintains the old structure.

Posted: **Mon Sep 26, 2011 10:00 pm**

Hi!

But that would then require a new variable for the s9y core, and would require all plugisn that use baseURL probably a rework, and those plugins then would need a version switch to use baseURL if an older s9y version is used...that seems like an awful lot of work to evaluate?

IMO it would be better to isolatedly patch the offending plugins like sitemap, and add a configuration directive for those plugins on which domain to use by default? Then we wouldn't "pollute" the global configuration with such a variable that might be unrequired by most plugins?

Regards,
Garvin

Posted: **Tue Sep 27, 2011 12:36 am**

For plugins which use the new variable it wouldn't be more than

Code: Select all

$baseUrl = isset($serendipityBaseURL) ? $serendipityBaseURL : $serendipityDefaultBaseURL;

and for all the other plugins everything would remain the same.
Alternatively an API function getBaseUrl($defaultUrl = false) or similar could be implemented which does this stuff.

The advantage here: we could eliminate a very confusing setting in the backend. It's quite a mess to have input controls for settings which change themselves. "URL to blog" should at least be disabled/removed when "Autodetect used HTTP-Host" is set.

However, I think it is the better solution to implement this new variable as more than just one plugin might require a fixed URL (all the functionality such as trackbacks, sitemap, short url generator, automatic posting of links to social networks such as Twitter, Facebook, Google+, pings to Technorati etc. might not work properly without) and if all of them implemented such a setting, we'd have a huge amount of redundancy. It would be very annoying and error-prone if I had to change the same settings for 10 plugins at 10 different locations.

Posted: **Tue Sep 27, 2011 3:21 pm**

Hi!

I still don't think there are really so many plugins that would require such a variable; the social network plugins use the baseURL that is set for the visitors (so it's not https, but http), and the trackbacks are not applied to your own URL but only to the foreign URL?

The problem I see is this, we have those options:

1. Patch s9y core, Patch s9y plugins

Provide those config options: "Autodetect Host", "Default Host"
Change baseURL to no longer be a user-specified option, but set it to "Default Host" when autodetection is off, and to current host, when autodetection is on.

Corresponding Plugins would need to be patched so that if they perform actions that would affect frontend visitors, they need to use "Default Host" instead of the current only option "base URL".

Advantage: Proper core variable
Drawbacks: Needs testing, cannot be provided with the upcoming serendipity 1.6 release, might have side effects on installation or other places where the DB configuration might not be loaded yet, also needs changing of any involved plugin

2. Only patch s9y plugin

Add a new config option for each plugin that currently misbehaves

Advantage: Easy fix, can be done instantly, without people upgrading their s9y. Has no side effects.
Drawbacks: When you have multiple misbehaving plugins, you need to manage those on their own.

I'm a bit reluctant to go route #1, even though it's the cleaner route it might have some nasty influences that would need more testing than for the given 1.6 release timeframe. The benefits of those people using https with specific plugins that misbehave are far less (IMHO) that the people on which this might have a negative impact on ruining the host autodetection/installation.

What I could offer is a "expert" setting, where we could add a new serendpity_config_local.inc.php variable like $serendipity['defaultHost'] and modify current plugins to see if that variable exist, and automatically use that for the "Host" configuration field instead of it's locally added new variable. That would make it easier for people like you?

Of course we could add this "defaultHost" setting to the current configuration as a proper configurable field, so that the user would have the current situation + a new field (to not screw with possible error situations), however the downside of this would then be to have a new option for many people that don't really need an additional variable there.

I'm torn.

Regards,
Garvin

Posted: **Tue Sep 27, 2011 5:59 pm**

Provide those config options: "Autodetect Host", "Default Host"
Change baseURL to no longer be a user-specified option, but set it to "Default Host" when autodetection is off, and to current host, when autodetection is on.

No. Not that way. $serendipityBaseURL stays all the same. No change at all. We only add $serendipityDefaultBaseURL additionally. And we also change the assigment of the setting "URL to blog". This no longer feeds the variable $serendipityBaseURL directly but $serendipityDefaultBaseURL. And $serendipityDefaultBaseURL then feeds $serendipityBaseURL if "Autodetect used HTTP-Host" is not set. Otherwise $serendipityBaseURL contains the current host as usual. In the end $serendipityBaseURL has always the same value it would have without this modification.
And since current plugins only know $serendipityDefaultBaseURL and this variable hasn't changed at all, exactly nothing has changed for any plugin and therefore nothing can break. The only thing that has changed is that this confusing self-modifying setting in the backend does not change itself anymore. But this is just a usability feature for the blog administrator and has no effect on any plugin or any other s9y core code.

But… due to the implementation of this additional variable, plugins which need a static URL such as the sitemap plugin CAN use it from now on.

What I could offer is a "expert" setting, where we could add a new serendpity_config_local.inc.php variable like $serendipity['defaultHost'] and modify current plugins to see if that variable exist, and automatically use that for the "Host" configuration field instead of it's locally added new variable. That would make it easier for people like you?

That is pretty much the same what I suggested, only in form of a configuration directive. However, I'd prefer a normal global variable as described above as it's more flexible and can be set from the backend without any real change to it. Currently "URL to blog" is a useless setting anyway if "Autodetect used HTTP-Host" is set (and if it's not set, "URL to blog" behaves the same as it does today).

Posted: **Tue Sep 27, 2011 9:52 pm**

Hi!

No. Not that way. $serendipityBaseURL stays all the same.

Then I don't really grasp your proposal, it actually sounds like what I said? Maybe you can propose a patch, if you have a specific plan in mind?

Regads,
Garvin

Posted: **Wed Sep 28, 2011 10:09 am**

Well, here's the s9y patch:

Code: Select all

--- serendipity/include/tpl/config_local.inc.php	2009-10-27 10:08:48.000000000 +0100
+++ serendipity/include/tpl/config_local.inc.php.new	2011-09-27 23:26:43.000000000 +0200
@@ -110,7 +110,7 @@
                                           'permission'  => 'siteConfiguration',
                                           'default'     => 'uploads/'),
 
-                                    array('var'         => 'baseURL',
+                                    array('var'         => 'defaultBaseURL',
                                           'title'       => INSTALL_URL,
                                           'description' => INSTALL_URL_DESC,
                                           'type'        => 'string',
--- serendipity/include/admin/configuration.inc.php	2008-11-06 12:44:43.000000000 +0100
+++ serendipity/include/admin/configuration.inc.php.new	2011-09-27 23:30:43.000000000 +0200
@@ -30,14 +30,14 @@
             $permalinkOld = array(
                 $oldConfig['serendipityHTTPPath'],
                 $oldConfig['serendipityPath'],
-                $oldConfig['baseURL'],
+                $oldConfig['defaultBaseURL'],
                 $oldConfig['indexFile'],
                 $oldConfig['rewrite']);
 
             $permalinkNew = array(
                 $serendipity['serendipityHTTPPath'],
                 $serendipity['serendipityPath'],
-                $serendipity['baseURL'],
+                $serendipity['defaultBaseURL'],
                 $serendipity['indexFile'],
                 $serendipity['rewrite']);
 
--- serendipity/include/functions_config.inc.php	2010-08-26 18:02:36.000000000 +0200
+++ serendipity/include/functions_config.inc.php.new	2011-09-28 00:22:08.000000000 +0200
@@ -329,7 +329,12 @@
         }
     }
     $config_loaded[$author] = true;
-
+    
+    // Set baseURL to defaultBaseURL
+    if ((empty($author) || empty($serendipity['baseURL'])) && isset($serendipity['defaultBaseURL'])) {
+        $serendipity['baseURL'] = $serendipity['defaultBaseURL'];
+    }
+    
     // Store default language
     $serendipity['default_lang'] = $serendipity['lang'];
 }
--- serendipity/include/functions_smarty.inc.php	2010-08-26 17:51:33.000000000 +0200
+++ serendipity/include/functions_smarty.inc.php.new	2011-09-28 09:45:53.000000000 +0200
@@ -980,6 +980,7 @@
 
                 'serendipityHTTPPath'       => $serendipity['serendipityHTTPPath'],
                 'serendipityBaseURL'        => $serendipity['baseURL'],
+                'serendipityDefaultBaseURL' => $serendipity['defaultBaseURL'],
                 'serendipityRewritePrefix'  => $serendipity['rewrite'] == 'none' ? $serendipity['indexFile'] . '?/' : '',
                 'serendipityIndexFile'      => $serendipity['indexFile'],
                 'serendipityVersion'        => ($serendipity['expose_s9y'] ? $serendipity['version'] : ''),

And here is the patch for the sitemap plugin:

Code: Select all

--- serendipity_event_google_sitemap/serendipity_event_google_sitemap.php	2010-12-31 18:53:26.000000000 +0100
+++ serendipity_event_google_sitemap/serendipity_event_google_sitemap.php.new	2011-09-28 00:30:15.000000000 +0200
@@ -246,21 +246,6 @@
         return $sqlnullfunction;
     }
 
-    /*! Get the real baseURL from the DB to get the primary URLs and
-     *  prevent wrong URLs (e.g. with https or different hostnames)
-     */
-    function get_BaseURL() {
-        global $serendipity;
-
-        $url = serendipity_db_query(
-                'SELECT value
-                FROM '.$serendipity['dbPrefix'].'config AS config
-                WHERE name = \'baseURL\'',
-            false, 'assoc');
-
-        return (is_array($url))? $url[0]['value'] : false;
-    }
-
     function add_entries(&$sitemap_xml, $limit = 0) {
         global $serendipity;
         $sqlnullfunction = $this->get_sqlnullfunction();
@@ -555,9 +540,12 @@
                      * This is a nasty workaround the issues with $serendipity['baseURL'] not
                      * pointing to the primary url on hostname-autosensing. See this forum-thread
                      * for details and discussion: http://board.s9y.org/viewtopic.php?p=60164#60164
+                     * 
+                     * With introduction of $serendipity['defaultBaseURL'] this workaround has improved.
+                     * See here: http://board.s9y.org/viewtopic.php?f=3&t=18147&p=10427029
                      */
                     $old_url = $serendipity['baseURL'];
-                    $serendipity['baseURL'] = $this->get_BaseURL();
+                    $serendipity['baseURL'] = $serendipity['defaultBaseURL'];
                     $this->write_sitemap('sitemap.xml', $eventData);
 
                     if (serendipity_db_bool($this->get_config('gnews'))) {

While working on it, if found the code note you see in the patch above. That means, a workaround for this issue already exists, but it was quite buggy because it required rewriting the "URL to blog" setting each time I change some other value on the same settings page. Otherwise, the current value of the field would be saved to the database as the "real" baseURL as it's named in the comment of the get_BaseURL() method causing the workaround to fail.
With the current implementation the get_BaseURL() has become superfluous and everything got easier. Also all the other plugins or the the s9y core can now just use $serendipity['defaultBaseURL'] where they fetched the "real" baseURL from the database, reducing the number of redundant queries.

Posted: **Wed Sep 28, 2011 2:16 pm**

Hi!

Ah, I finally get it. I guess I was thinking around too many corners. I committed your patch into SVN trunk for a future release. I've patched the plugin as well, but I left the original code intact for people that use older s9y versions:

Code: Select all

Index: serendipity_event_google_sitemap.php
===================================================================
RCS file: /cvsroot/php-blog/additional_plugins/serendipity_event_google_sitemap/serendipity_event_google_sitemap.php,v
retrieving revision 1.59
diff -u -r1.59 serendipity_event_google_sitemap.php
--- serendipity_event_google_sitemap.php	31 Dec 2010 17:53:26 -0000	1.59
+++ serendipity_event_google_sitemap.php	28 Sep 2011 12:08:09 -0000
@@ -30,7 +30,7 @@
         $propbag->add('name', PLUGIN_EVENT_SITEMAP_TITLE);
         $propbag->add('description', PLUGIN_EVENT_SITEMAP_DESC);
         $propbag->add('author', 'Boris');
-        $propbag->add('version', '0.56');
+        $propbag->add('version', '0.57');
         $propbag->add('event_hooks',  array(
                 'backend_publish' => true,
                 'backend_save'    => true,
@@ -556,8 +556,14 @@
                      * pointing to the primary url on hostname-autosensing. See this forum-thread
                      * for details and discussion: http://board.s9y.org/viewtopic.php?p=60164#60164
                      */
-                    $old_url = $serendipity['baseURL'];
-                    $serendipity['baseURL'] = $this->get_BaseURL();
+                     
+                    if (!empty($serendipity['defaultBaseURL'])) {
+                        $serendipity['baseURL'] = $old_url = $serendipity['defaultBaseURL'];
+                    } else {    
+                        $old_url = $serendipity['baseURL'];
+                        $serendipity['baseURL'] = $this->get_BaseURL();
+                    }    
+
                     $this->write_sitemap('sitemap.xml', $eventData);
 
                     if (serendipity_db_bool($this->get_config('gnews'))) {

Thanks a lot for your help in improving this,
Garvin

Serendipity Forums

Bad HTTPS handling

Bad HTTPS handling

Re: Bad HTTPS handling

Re: Bad HTTPS handling

Re: Bad HTTPS handling

Re: Bad HTTPS handling

Re: Bad HTTPS handling

Re: Bad HTTPS handling

Re: Bad HTTPS handling

Re: Bad HTTPS handling

Re: Bad HTTPS handling