Files: 
CommentFileSizeAuthor
#45 https-1798832-44.patch8.65 KBBerdir
PASSED: [[SimpleTest]]: [MySQL] 54,135 pass(es).
[ View ]
#45 https-1798832-44-interdiff.txt2.09 KBBerdir
#41 https-1798832-41.patch6.56 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 52,436 pass(es), 6 fail(s), and 0 exception(s).
[ View ]
#36 https-cmi-1798832-36.patch8.48 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 47,956 pass(es), 20 fail(s), and 0 exception(s).
[ View ]
#31 1798832-https_to_config-drupal8-30.patch8.46 KBACF
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1798832-https_to_config-drupal8-30.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#28 https_in_config_not_variables-1798832-28.patch9.34 KBtyphonius
FAILED: [[SimpleTest]]: [MySQL] 48,203 pass(es), 15 fail(s), and 0 exception(s).
[ View ]
#26 https_in_config_not_variables-1798832.patch9.34 KBjohan.gant
FAILED: [[SimpleTest]]: [MySQL] 48,246 pass(es), 15 fail(s), and 0 exception(s).
[ View ]
#20 https_cmi.patch9.3 KBaspilicious
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch https_cmi.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#18 1798832_https_18.patch9.44 KBheyrocker
FAILED: [[SimpleTest]]: [MySQL] 42,096 pass(es), 4 fail(s), and 0 exception(s).
[ View ]
#15 1798832_https_6.patch9.49 KBandreiashu
FAILED: [[SimpleTest]]: [MySQL] 41,977 pass(es), 4 fail(s), and 0 exception(s).
[ View ]
#13 1798832_https_5.patch8.88 KBandreiashu
FAILED: [[SimpleTest]]: [MySQL] 41,974 pass(es), 4 fail(s), and 0 exception(s).
[ View ]
#11 1798832_https_4.patch8.36 KBandreiashu
PASSED: [[SimpleTest]]: [MySQL] 41,974 pass(es).
[ View ]
#6 1798832_https_3.patch8.36 KBandreiashu
PASSED: [[SimpleTest]]: [MySQL] 41,898 pass(es).
[ View ]
#4 1798832_https_2.patch6.18 KBandreiashu
FAILED: [[SimpleTest]]: [MySQL] 41,904 pass(es), 6 fail(s), and 0 exception(s).
[ View ]
#1 1798832_https_1.patch6.19 KBandreiashu
FAILED: [[SimpleTest]]: [MySQL] 41,882 pass(es), 6 fail(s), and 0 exception(s).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new6.19 KB
FAILED: [[SimpleTest]]: [MySQL] 41,882 pass(es), 6 fail(s), and 0 exception(s).
[ View ]

Title:Move https variable to Configuration SystemConvert https to CMI

updated title

Status:Needs review» Needs work

The last submitted patch, 1798832_https_1.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new6.18 KB
FAILED: [[SimpleTest]]: [MySQL] 41,904 pass(es), 6 fail(s), and 0 exception(s).
[ View ]

moved default value to system.site.yml

Status:Needs review» Needs work

The last submitted patch, 1798832_https_2.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new8.36 KB
PASSED: [[SimpleTest]]: [MySQL] 41,898 pass(es).
[ View ]

I missed some a few tests

Status:Needs review» Needs work

The last submitted patch, 1798832_https_3.patch, failed testing.

Status:Needs work» Needs review

known random failure in EntityTranslationTest.php. Submitting for retest.

Issue tags:-Configuration system

#6: 1798832_https_3.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+Configuration system

The last submitted patch, 1798832_https_3.patch, failed testing.

StatusFileSize
new8.36 KB
PASSED: [[SimpleTest]]: [MySQL] 41,974 pass(es).
[ View ]

Attached patch also updates from variable to config

Status:Needs work» Needs review

StatusFileSize
new8.88 KB
FAILED: [[SimpleTest]]: [MySQL] 41,974 pass(es), 4 fail(s), and 0 exception(s).
[ View ]

Attached the patch that contains the actual hook_update_N

Status:Needs review» Needs work

The last submitted patch, 1798832_https_5.patch, failed testing.

StatusFileSize
new9.49 KB
FAILED: [[SimpleTest]]: [MySQL] 41,977 pass(es), 4 fail(s), and 0 exception(s).
[ View ]

The tests failed in a different place for the patch at #13. I was useful though because I see we have SystemUpgradePathTest::testSystemVariableUpgrade() in which I added the test for the https variable upgrade.

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, 1798832_https_6.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new9.44 KB
FAILED: [[SimpleTest]]: [MySQL] 42,096 pass(es), 4 fail(s), and 0 exception(s).
[ View ]

This is a reroll to HEAD, but I'm not sure what is causing those test failures.

Status:Needs review» Needs work

The last submitted patch, 1798832_https_18.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new9.3 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch https_cmi.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Maybe this fixes it...

+++ b/core/modules/language/lib/Drupal/language/Tests/LanguageUILanguageNegotiationTest.phpundefined
@@ -474,11 +475,11 @@ function testLanguageDomain() {
+    $config->set('https', FALSE)->save();

Should we set with FALSE and TRUE or '0' and '1'? Will booleans be converted to 0 and 1?

diff --git a/core/modules/system/lib/Drupal/system/Tests/Upgrade/SystemUpgradePathTest.php b/core/modules/system/lib/Drupal/system/Tests/Upgrade/SystemUpgradePathTest.php
index db95f06..2b720f9 100644
--- a/core/modules/system/lib/Drupal/system/Tests/Upgrade/SystemUpgradePathTest.php
+++ b/core/modules/system/lib/Drupal/system/Tests/Upgrade/SystemUpgradePathTest.php
@@ -73,6 +73,7 @@ public function testSystemVariableUpgrade() {
       'page.403' => '403',
       'page.404' => '404',
       'page.front' => 'node',
+      'https' => '0',
     );
     foreach ($expected_config as $file => $values) {

Where do you set the 'https' variable? I'd expect something in core/modules/system/tests/upgrade/drupal-7.system.database.php (see http://drupal.org/node/1667896)

Issue tags:-Configuration system

#20: https_cmi.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+Configuration system

The last submitted patch, https_cmi.patch, failed testing.

From what I can tell, the patches on this ticket aim to set 'https' in system.install under update_8012.

Status:Needs work» Needs review
StatusFileSize
new9.34 KB
FAILED: [[SimpleTest]]: [MySQL] 48,246 pass(es), 15 fail(s), and 0 exception(s).
[ View ]

Attempted to re-roll the latest patch to work with latest HEAD version of D8. Hoping it'll pass...

Status:Needs review» Needs work

The last submitted patch, https_in_config_not_variables-1798832.patch, failed testing.

StatusFileSize
new9.34 KB
FAILED: [[SimpleTest]]: [MySQL] 48,203 pass(es), 15 fail(s), and 0 exception(s).
[ View ]

Running locally:

  • Uncaught PHP Exception Symfony\Component\DependencyInjection\Exception\InvalidArgumentException: "The service definition "config.factory" does not exist." at /home/d8/core/vendor/symfony/dependency-injection/Symfony/Component/DependencyInjection/ContainerBuilder.php line 690
  • Upgrade path test fails (even if not running the system upgrade path and instead running State system upgrade test [which has not been changed])
  • Language test passes
  • Session HTTPS test passes

Since system.site.yml has changed since the last patch with the addition of weight_select_max, here is a minor reroll so the patch applies cleanly.

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, https_in_config_not_variables-1798832-28.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new8.46 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1798832-https_to_config-drupal8-30.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

hopefully a fix.

Status:Needs review» Needs work

The last submitted patch, 1798832-https_to_config-drupal8-30.patch, failed testing.

Status:Needs work» Postponed

Thanks ACF!

The fatal in the test above is:

exception 'Symfony\Component\DependencyInjection\Exception\InvalidArgumentException' with message 'The service definition "config.factory" does not exist.' in /var/lib/drupaltestbot/sites/default/files/checkout/core/vendor/symfony/dependency-injection/Symfony/Component/DependencyInjection/ContainerBuilder.php:740

This is probably #1848998: Problems with update_module_enable()? Let's postpone on fixing that.

Status:Postponed» Needs review
Issue tags:-Configuration system

Status:Needs review» Needs work
Issue tags:+Configuration system

The last submitted patch, 1798832-https_to_config-drupal8-30.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new8.48 KB
FAILED: [[SimpleTest]]: [MySQL] 47,956 pass(es), 20 fail(s), and 0 exception(s).
[ View ]

Re-roll.

Status:Needs review» Needs work

The last submitted patch, https-cmi-1798832-36.patch, failed testing.

Hm, wondering if we want to make this a setting instead?

Sounds like a possibly environment specific thing, that is usually already set in settings.php (see securepages project page), is called very early in update.php and similar places.

While we're here, I don't think this setting should be called "https", since it is actually a harmful setting for those HTTPS sites that want to use only secure sessions. A more accurate description would be something like "mixed-mode sessions" because that's what it does: create both secure and insecure sessions when a session is created on an HTTPS site.

Title:Convert https to CMIConvert https to $settings

I think this should be in $settings as well, there's no need for a UI and session handling can't depend on CMI.

Status:Needs work» Needs review
StatusFileSize
new6.56 KB
FAILED: [[SimpleTest]]: [MySQL] 52,436 pass(es), 6 fail(s), and 0 exception(s).
[ View ]

Changed to settings(), also renamed to mixed_mode_sessions and documented in default.settings.php

No upgrade path yet, we need the updated drupal_write_settings() for that. If we even want to. Still not sure if we want to migrate settings other than those that we will have have to (database, drupal hash salt).

Status:Needs review» Needs work

The last submitted patch, https-1798832-41.patch, failed testing.

grep -r "variable_set('https" core/
core/modules/language/lib/Drupal/language/Tests/LanguageUILanguageNegotiationTest.php:    variable_set('https', TRUE);
core/modules/language/lib/Drupal/language/Tests/LanguageUILanguageNegotiationTest.php:    variable_set('https', FALSE);
core/modules/system/lib/Drupal/system/Tests/Session/SessionHttpsTest.php:    variable_set('https', TRUE);

Status:Needs work» Needs review
StatusFileSize
new2.09 KB
new8.65 KB
PASSED: [[SimpleTest]]: [MySQL] 54,135 pass(es).
[ View ]

Wooo, it works!

awesome, thanks for rolling this!
i see settings() being called twice in some functions, we could store the object somewhere and reuse instead of keep calling settings()?
or you think its out of scope of this issue? its just that it got a bit uglier now

It would be probably worth in url(), otherwise this seems RTBC for me.

Status:Needs review» Reviewed & tested by the community

i started cleaning it up, but then patch start getting bigger (especially in session.inc) , so lets get this in and cleanup elsewhere..variable_get() calls converted to settings()->get(), anything else is out of scope

Status:Reviewed & tested by the community» Fixed

Committed/pushed to 8.x, thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.