Host Whitelist

I will test this but can not easily run this patch. Can you include the entire .module file?

This input filter is part of Drupal 8 core now. I would recommend to create feature request against Drupal 8 core.

Patch no longer applies.

If at least we have a patch proposed for Drupal 8 but not committed, I think it would be ok to proceed with committing a patch here.

For Drupal.org, we need a whitelist because:

• Moving to a CDN would likely require moving to www.drupal.org. Both domains should be accepted.
• Dev & staging sites have a different domain, but should still allow Drupal.org.
• When using drush, $_SERVER['HTTP_HOST'] isn't set.

This must have tests since it is security-related.

And there should be a patch for D8 with added tests that are passing, so this doesn't get out of line.

Basic raised a good point in #1952278-17: White list travis-ci.org as a valid source for external images that we should proxy these images and store them locally, temporarily, on behalf of other services. This gets around privacy and stability concerns about those images.

I think the implementation would be something like:
* the images are requested and copied to the local filesystem
* a variable determines how often to go get a new copy of the image
* if the request for the new image fails, the local copy is kept

Hm... Not sure I agree with that.

1. If you explicitly whitelist external hosts, then why would you bother with caching them on your CDN to begin with? That only means that the cached resources very potentially get out of date? (especially in the travis-ci example)
2. A customizable whitelist is plausible, but a download facility for local copies seems out of scope for this filter. (IIRC, there is at least one other module for that though.)

If an external host is offline pages are not stuck waiting for a connection to the external host. The image cache should respect the cache headers sent by the image. Github uses this for images including travis-ci status. Images are loaded from https://camo.githubusercontent.com/. This also prevents tracking user data and browser data through external servers with all requests for images proxied at drupal.org.

Whitelisting is needed to move drupal.org to www.drupal.org, but I don't consider *.drupal.org external, not sure the download facility is required for that.

Let's limit the scope of this issue to whitelisting. We can decide to whitelist travis-ci, or a proxy setup, at #1952278: White list travis-ci.org as a valid source for external images.

#2257291: Handle alternate domains in filter_html_image_secure has a D8 patch. When that has a firm plan, this issue can proceed.

#2257291: Handle alternate domains in filter_html_image_secure has barely enough feedback for the backport to proceed. Looks like #1 will be a good starting point.

I committed the filter code only for the backport. This is an advanced setting, and not providing a UI is okay.

@drumm: It is absolutely required for this to come with tests, because this is a security related feature/module.

Adding proper test coverage (once) also ensures that drupal.org works correctly (forever). ;)

Here is tests and some fixes

-function filter_html_image_secure_filter_tips($filter, $format, $long = FALSE) {
+function _filter_html_image_secure_filter_tips($filter, $format, $long = FALSE) {

filter_html_image_secire_process == MODULE_NAME_process == implementation of template_process(&$vars, $hook) that means that filter callback is invoked for ALL theme functions.

-    if ($domains = variable_get('filter_html_image_secure_domains', NULL)) {
+    if ($domains = variable_get('filter_html_image_secure_' . $format->format . '_domains', NULL)) {

Allows to specify settings per format.

Great work!

+++ b/filter_html_image_secure.module
@@ -47,12 +47,14 @@ function filter_html_image_secure_process($text, $format) {
+    if ($domains = variable_get('filter_html_image_secure_' . $format->format . '_domains', NULL)) {

Would be nicer if you take it out of the if. Towards drupal 8 this format will be a bit harder to use.

Here is patch with fix for CLI testing.
I'm using $base_url instead if $_SERVER['HTTP_HOST']

Uninstall of variables is completly missing in this patches. Also in the one that has been committed.

Re-uploading for re-test.

I have rerolled the patch.

Sorry, that's an incorrect patch.