From 9591c3c9428cf3434502a335112a1305b4301ad1 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Sun, 19 Apr 2026 09:50:17 -0700 Subject: [PATCH 01/10] Support consent manager --- ad/manager.php | 86 ++++++++ event/main_listener.php | 22 +- language/en/acp.php | 2 +- language/en/common.php | 2 + tests/ad/prepare_ad_code_test.php | 64 ++++++ tests/event/main_listener_test.php | 34 ++++ tests/event/setup_ads_consentmanager_test.php | 190 ++++++++++++++++++ tests/functional/ads_consentmanager_test.php | 40 ++++ tests/functional/functional_base.php | 9 +- 9 files changed, 444 insertions(+), 5 deletions(-) create mode 100644 tests/ad/prepare_ad_code_test.php create mode 100644 tests/event/setup_ads_consentmanager_test.php create mode 100644 tests/functional/ads_consentmanager_test.php diff --git a/ad/manager.php b/ad/manager.php index ca659df2..d1f703d0 100644 --- a/ad/manager.php +++ b/ad/manager.php @@ -12,6 +12,8 @@ class manager { + public const CONSENT_CATEGORY = 'marketing'; + /** @var \phpbb\db\driver\driver_interface */ protected $db; @@ -372,6 +374,90 @@ public function load_groups($ad_id) return $groups; } + /** + * Prepare ad code for output, applying consent-manager deferrals when enabled. + * + * @param string $ad_code Stored advertisement code + * @param bool $consent_enabled Whether marketing consent is required + * @return string + */ + public function prepare_ad_code($ad_code, $consent_enabled) + { + $ad_code = htmlspecialchars_decode($ad_code, ENT_COMPAT); + $original_ad_code = $ad_code; + + if (!$consent_enabled || $ad_code === '') + { + return $ad_code; + } + + $ad_code = preg_replace_callback('#]*)>(.*?)#is', function ($matches) + { + $attributes = $matches[1] ?? ''; + $content = $matches[2] ?? ''; + + if (!$this->should_defer_script_tag($attributes)) + { + return $matches[0]; + } + + return 'inject_consent_attributes($attributes) . '>' . $content . ''; + }, $ad_code); + + return $ad_code ?? $original_ad_code; + } + + /** + * Determine whether a script tag is executable and should be deferred. + * + * @param string $attributes Script tag attributes + * @return bool + */ + protected function should_defer_script_tag($attributes) + { + if (preg_match('/\bdata-consent-category\s*=/i', $attributes)) + { + return false; + } + + if (!preg_match('/\btype\s*=\s*([\'"])(.*?)\1/i', $attributes, $matches)) + { + return true; + } + + $type = strtolower(trim(explode(';', $matches[2])[0])); + return $type === '' + || $type === 'text/plain' + || $type === 'module' + || strpos($type, 'javascript') !== false + || strpos($type, 'ecmascript') !== false; + } + + /** + * Replace script tag attributes with consent-aware placeholders. + * + * @param string $attributes Script tag attributes + * @return string + */ + protected function inject_consent_attributes($attributes) + { + if (preg_match('/\btype\s*=\s*([\'"])(.*?)\1/i', $attributes)) + { + $attributes = preg_replace('/\btype\s*=\s*([\'"])(.*?)\1/i', 'type="text/plain"', $attributes, 1); + } + else + { + $attributes .= ' type="text/plain"'; + } + + if (!preg_match('/\bdata-consent-category\s*=/i', $attributes)) + { + $attributes .= ' data-consent-category="' . self::CONSENT_CATEGORY . '"'; + } + + return $attributes; + } + /** * Make sure only necessary data make their way to SQL query * diff --git a/event/main_listener.php b/event/main_listener.php index f98d954c..2a1009c2 100644 --- a/event/main_listener.php +++ b/event/main_listener.php @@ -64,6 +64,7 @@ public static function getSubscribedEvents() 'core.adm_page_header_after' => 'disable_xss_protection', 'core.group_add_user_after' => 'destroy_user_group_cache', 'core.group_delete_user_after' => 'destroy_user_group_cache', + 'phpbb.consentmanager.collect_registrations' => 'register_ads', ); } @@ -133,16 +134,18 @@ public function setup_ads() { // check for the existence of 'MESSAGE_TEXT', which signals it's an error page. $non_content_page = $this->template->retrieve_var('MESSAGE_TEXT') || $this->is_non_content_page(); + $consent_enabled = (bool) $this->template->retrieve_var('S_CONSENTMANAGER_MARKETING_ENABLED'); $location_ids = $this->location_manager->get_all_location_ids(); $user_groups = $this->manager->load_memberships($this->user->data['user_id']); $ad_ids = array(); + $ads = $this->manager->get_ads($location_ids, $user_groups, $non_content_page); - foreach ($this->manager->get_ads($location_ids, $user_groups, $non_content_page) as $row) + foreach ($ads as $row) { $ad_ids[] = $row['ad_id']; $this->template->assign_vars(array( - 'AD_' . strtoupper($row['location_id']) => htmlspecialchars_decode($row['ad_code'], ENT_COMPAT), + 'AD_' . strtoupper($row['location_id']) => $this->manager->prepare_ad_code($row['ad_code'], $consent_enabled), 'AD_' . strtoupper($row['location_id']) . '_ID' => (int) $row['ad_id'], 'AD_' . strtoupper($row['location_id']) . '_CENTER' => (bool) $row['ad_centering'], )); @@ -296,4 +299,19 @@ public function append_agreement() $this->template->append_var('AGREEMENT_TEXT', $this->language->lang('PHPBB_ADS_PRIVACY_POLICY', $this->config['sitename'])); } + + /** + * Register the advertisement extension with Consent Manager. + * + * @param \phpbb\event\data|array $event The event object or event data + * @return void + */ + public function register_ads($event) + { + $event['consent_manager']->register('phpbb.ads', array( + 'label' => $this->language->lang('PHPBB_ADS_CONSENT_LABEL'), + 'category' => \phpbb\ads\ad\manager::CONSENT_CATEGORY, + 'description' => $this->language->lang('PHPBB_ADS_CONSENT_DESCRIPTION'), + )); + } } diff --git a/language/en/acp.php b/language/en/acp.php index 5af42e27..4f637efd 100644 --- a/language/en/acp.php +++ b/language/en/acp.php @@ -31,7 +31,7 @@ 'AD_NOTE' => 'Notes', 'AD_NOTE_EXPLAIN' => 'Enter any notes for this advertisement. These notes are not shown anywhere except in the ACP and are optional.', 'AD_CODE' => 'Code', - 'AD_CODE_EXPLAIN' => 'Enter the advertisement code here. All code must use HTML markup, BBCodes are not supported.

Note: If your advertisement code places cookies, collects user data, or tracks user behaviour (for example, ads from Google AdSense or other third-party ad networks), then you should enable the Advertising Disclosure in the Advertisement Management Settings panel to ensure compliance. If you are uncertain, it is recommended that you enable it.', + 'AD_CODE_EXPLAIN' => 'Enter the advertisement code here. All code must use HTML markup, BBCodes are not supported.

If the Consent Manager extension is enabled, ads with script tags are automatically held back until a visitor allows marketing. Normal HTML, such as images and links, will still display, so you usually do not need to change your ad code manually.

Note: If your advertisement code places cookies, collects user data, or tracks user behaviour (for example, ads from Google AdSense or other third-party ad networks), then you should enable the Advertising Disclosure in the Advertisement Management Settings panel to ensure compliance. If you are uncertain, it is recommended that you enable it.', 'ANALYSE_AD_CODE' => 'Analyse advertisement code', 'EVERYTHING_OK' => 'The code appears OK.', 'AD_BANNER' => 'Advertisement banner', diff --git a/language/en/common.php b/language/en/common.php index ebe45c83..fa60493f 100644 --- a/language/en/common.php +++ b/language/en/common.php @@ -26,6 +26,8 @@ ], 'ADVERTISEMENT' => 'Advertisement', 'HIDE_AD' => 'Hide advertisement', + 'PHPBB_ADS_CONSENT_LABEL' => 'Advertisements', + 'PHPBB_ADS_CONSENT_DESCRIPTION' => 'Advertising features that may use cookies or similar technologies to collect data.', 'VISUAL_DEMO' => 'Visual demo for ad locations is active', 'DISABLE_VISUAL_DEMO' => 'Click to disable visual demo', diff --git a/tests/ad/prepare_ad_code_test.php b/tests/ad/prepare_ad_code_test.php new file mode 100644 index 00000000..8e90477e --- /dev/null +++ b/tests/ad/prepare_ad_code_test.php @@ -0,0 +1,64 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + */ + +namespace phpbb\ads\tests\ad; + +class prepare_ad_code_test extends ad_base +{ + public function test_consent_category_constant() + { + self::assertSame('marketing', \phpbb\ads\ad\manager::CONSENT_CATEGORY); + } + + public function test_returns_decoded_code_when_consent_disabled() + { + $raw = htmlspecialchars('', ENT_COMPAT); + $result = $this->get_manager()->prepare_ad_code($raw, false); + self::assertSame('', $result); + self::assertStringNotContainsString('type="text/plain"', $result); + } + + public function test_returns_empty_string_unchanged() + { + self::assertSame('', $this->get_manager()->prepare_ad_code('', true)); + } + + public function test_defers_script_tag_when_consent_enabled() + { + $raw = htmlspecialchars('', ENT_COMPAT); + $result = $this->get_manager()->prepare_ad_code($raw, true); + self::assertStringContainsString('type="text/plain"', $result); + self::assertStringContainsString('data-consent-category="marketing"', $result); + self::assertStringContainsString('src="https://ads.example.com/tag.js"', $result); + } + + public function test_adds_consent_category_to_existing_text_plain_script() + { + $raw = htmlspecialchars('', ENT_COMPAT); + $result = $this->get_manager()->prepare_ad_code($raw, true); + self::assertStringContainsString('type="text/plain"', $result); + self::assertStringContainsString('data-consent-category="marketing"', $result); + } + + public function test_does_not_double_wrap_already_tagged_script() + { + $raw = htmlspecialchars('', ENT_COMPAT); + $result = $this->get_manager()->prepare_ad_code($raw, true); + self::assertSame(1, substr_count($result, 'data-consent-category=')); + } + + public function test_non_script_html_is_preserved() + { + $raw = htmlspecialchars('
Ad
', ENT_COMPAT); + $result = $this->get_manager()->prepare_ad_code($raw, true); + self::assertStringContainsString('', $result); + self::assertStringNotContainsString('type="text/plain"', $result); + } +} diff --git a/tests/event/main_listener_test.php b/tests/event/main_listener_test.php index 803fe43a..2e9c184f 100644 --- a/tests/event/main_listener_test.php +++ b/tests/event/main_listener_test.php @@ -35,6 +35,40 @@ public function test_getSubscribedEvents() 'core.adm_page_header_after', 'core.group_add_user_after', 'core.group_delete_user_after', + 'phpbb.consentmanager.collect_registrations', ), array_keys(\phpbb\ads\event\main_listener::getSubscribedEvents())); } + + public function test_register_ads() + { + $this->language->add_lang('common', 'phpbb/ads'); + $listener = $this->get_listener(); + $consent_manager = new consent_manager_double(); + + $listener->register_ads(array( + 'consent_manager' => $consent_manager, + )); + + self::assertCount(1, $consent_manager->registrations); + self::assertSame('phpbb.ads', $consent_manager->registrations[0]['id']); + self::assertSame(array( + 'label' => $this->language->lang('PHPBB_ADS_CONSENT_LABEL'), + 'category' => \phpbb\ads\ad\manager::CONSENT_CATEGORY, + 'description' => $this->language->lang('PHPBB_ADS_CONSENT_DESCRIPTION'), + ), $consent_manager->registrations[0]['definition']); + } +} + +class consent_manager_double +{ + /** @var array */ + public $registrations = array(); + + public function register($id, array $definition) + { + $this->registrations[] = array( + 'id' => $id, + 'definition' => $definition, + ); + } } diff --git a/tests/event/setup_ads_consentmanager_test.php b/tests/event/setup_ads_consentmanager_test.php new file mode 100644 index 00000000..1c1e8662 --- /dev/null +++ b/tests/event/setup_ads_consentmanager_test.php @@ -0,0 +1,190 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + */ + +namespace phpbb\ads\tests\event; + +class setup_ads_consentmanager_test extends main_listener_base +{ + public function test_setup_ads_defers_ad_markup_when_consentmanager_is_enabled() + { + $stored_ad_code = htmlspecialchars( + '
Ad
', + ENT_COMPAT + ); + + $this->user->data['user_id'] = 1; + $this->user->page['page_name'] = 'index.' . $this->php_ext; + $this->user->page['page_dir'] = ''; + + $this->manager = $this->getMockBuilder('\phpbb\ads\ad\manager') + ->disableOriginalConstructor() + ->setMethods(array('load_memberships', 'get_ads')) + ->getMock(); + $this->location_manager = $this->getMockBuilder('\phpbb\ads\location\manager') + ->disableOriginalConstructor() + ->setMethods(array('get_all_location_ids')) + ->getMock(); + + $this->location_manager->expects(self::once()) + ->method('get_all_location_ids') + ->willReturn(array('above_header')); + + $this->manager->expects(self::once()) + ->method('load_memberships') + ->with(1) + ->willReturn(array()); + + $this->manager->expects(self::once()) + ->method('get_ads') + ->with(array('above_header'), array(), false) + ->willReturn(array(array( + 'location_id' => 'above_header', + 'ad_id' => 42, + 'ad_code' => $stored_ad_code, + 'ad_centering' => 0, + ))); + + $this->template->expects(self::exactly(2)) + ->method('retrieve_var') + ->willReturnCallback(function ($var_name) + { + return $var_name === 'S_CONSENTMANAGER_MARKETING_ENABLED'; + }); + + $this->template->expects(self::once()) + ->method('assign_vars') + ->with(self::callback(function ($vars) + { + return $vars['AD_ABOVE_HEADER_ID'] === 42 + && $vars['AD_ABOVE_HEADER_CENTER'] === false + && strpos($vars['AD_ABOVE_HEADER'], 'type="text/plain"') !== false + && strpos($vars['AD_ABOVE_HEADER'], 'data-consent-category="marketing"') !== false + && strpos($vars['AD_ABOVE_HEADER'], 'src="https://ads.example.com/tag.js"') !== false + && strpos($vars['AD_ABOVE_HEADER'], '') !== false + && strpos($vars['AD_ABOVE_HEADER'], 'phpbb-ads-consent-placeholder') === false; + })); + + $this->get_listener()->setup_ads(); + } + + public function test_setup_ads_adds_consent_category_to_text_plain_scripts() + { + $stored_ad_code = htmlspecialchars( + '', + ENT_COMPAT + ); + + $this->user->data['user_id'] = 1; + $this->user->page['page_name'] = 'index.' . $this->php_ext; + $this->user->page['page_dir'] = ''; + + $this->manager = $this->getMockBuilder('\phpbb\ads\ad\manager') + ->disableOriginalConstructor() + ->setMethods(array('load_memberships', 'get_ads')) + ->getMock(); + $this->location_manager = $this->getMockBuilder('\phpbb\ads\location\manager') + ->disableOriginalConstructor() + ->setMethods(array('get_all_location_ids')) + ->getMock(); + + $this->location_manager->expects(self::once()) + ->method('get_all_location_ids') + ->willReturn(array('above_header')); + + $this->manager->expects(self::once()) + ->method('load_memberships') + ->with(1) + ->willReturn(array()); + + $this->manager->expects(self::once()) + ->method('get_ads') + ->with(array('above_header'), array(), false) + ->willReturn(array(array( + 'location_id' => 'above_header', + 'ad_id' => 99, + 'ad_code' => $stored_ad_code, + 'ad_centering' => 0, + ))); + + $this->template->expects(self::exactly(2)) + ->method('retrieve_var') + ->willReturnCallback(function ($var_name) + { + return $var_name === 'S_CONSENTMANAGER_MARKETING_ENABLED'; + }); + + $this->template->expects(self::once()) + ->method('assign_vars') + ->with(self::callback(function ($vars) + { + return $vars['AD_ABOVE_HEADER_ID'] === 99 + && strpos($vars['AD_ABOVE_HEADER'], 'type="text/plain"') !== false + && strpos($vars['AD_ABOVE_HEADER'], 'data-consent-category="marketing"') !== false + && strpos($vars['AD_ABOVE_HEADER'], 'src="https://ads.example.com/legacy.js"') !== false; + })); + + $this->get_listener()->setup_ads(); + } + + public function test_setup_ads_does_not_defer_when_marketing_category_is_disabled() + { + $stored_ad_code = htmlspecialchars( + '', + ENT_COMPAT + ); + + $this->user->data['user_id'] = 1; + $this->user->page['page_name'] = 'index.' . $this->php_ext; + $this->user->page['page_dir'] = ''; + + $this->manager = $this->getMockBuilder('\phpbb\ads\ad\manager') + ->disableOriginalConstructor() + ->setMethods(array('load_memberships', 'get_ads')) + ->getMock(); + $this->location_manager = $this->getMockBuilder('\phpbb\ads\location\manager') + ->disableOriginalConstructor() + ->setMethods(array('get_all_location_ids')) + ->getMock(); + + $this->location_manager->expects(self::once()) + ->method('get_all_location_ids') + ->willReturn(array('above_header')); + + $this->manager->expects(self::once()) + ->method('load_memberships') + ->with(1) + ->willReturn(array()); + + $this->manager->expects(self::once()) + ->method('get_ads') + ->with(array('above_header'), array(), false) + ->willReturn(array(array( + 'location_id' => 'above_header', + 'ad_id' => 77, + 'ad_code' => $stored_ad_code, + 'ad_centering' => 0, + ))); + + $this->template->expects(self::exactly(2)) + ->method('retrieve_var') + ->willReturn(false); + + $this->template->expects(self::once()) + ->method('assign_vars') + ->with(self::callback(function ($vars) + { + return strpos($vars['AD_ABOVE_HEADER'], 'type="text/plain"') === false + && strpos($vars['AD_ABOVE_HEADER'], 'data-consent-category="marketing"') === false + && strpos($vars['AD_ABOVE_HEADER'], 'src="https://ads.example.com/tag.js"') !== false; + })); + + $this->get_listener()->setup_ads(); + } +} diff --git a/tests/functional/ads_consentmanager_test.php b/tests/functional/ads_consentmanager_test.php new file mode 100644 index 00000000..9d99b4b0 --- /dev/null +++ b/tests/functional/ads_consentmanager_test.php @@ -0,0 +1,40 @@ +create_ad( + 'above_header', + '', + false, + true, + '', + '
Example ad
' + ); + + $crawler = self::request('GET', 'index.php'); + + self::assertSame(1, $crawler->filter('.phpbb-ads script[type="text/plain"][data-consent-category="marketing"][src*="ads.example.com/tag.js"]')->count()); + self::assertSame(0, $crawler->filter('.phpbb-ads script[src*="ads.example.com/tag.js"]:not([type="text/plain"])')->count()); + self::assertSame(1, $crawler->filter('.phpbb-ads iframe[src*="ads.example.com/frame"]')->count()); + } +} diff --git a/tests/functional/functional_base.php b/tests/functional/functional_base.php index bd5395d1..f7e42a1b 100644 --- a/tests/functional/functional_base.php +++ b/tests/functional/functional_base.php @@ -42,7 +42,7 @@ protected function setUp(): void $this->admin_login(); } - protected function create_ad($location, $end_date = '', $content_only = false, $centering = true, $start_date = '') + protected function create_ad($location, $end_date = '', $content_only = false, $centering = true, $start_date = '', $ad_code = '') { // Load Advertisement management ACP page $crawler = self::request('GET', "adm/index.php?i=-phpbb-ads-acp-main_module&mode=manage&sid={$this->sid}"); @@ -51,11 +51,16 @@ protected function create_ad($location, $end_date = '', $content_only = false, $ $form = $crawler->selectButton($this->lang('ACP_ADS_ADD'))->form(); $crawler = self::submit($form); + if ($ad_code === '') + { + $ad_code = ''; + } + // Create ad $form_data = array( 'ad_name' => 'Functional test template location ' . $location, 'ad_note' => '', - 'ad_code' => '', + 'ad_code' => $ad_code, 'ad_enabled' => 1, 'ad_locations' => array($location), 'ad_start_date' => $start_date, From 89458804546183e34060552c0a13520613e236a8 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Tue, 26 May 2026 10:15:12 -0700 Subject: [PATCH 02/10] Add a toggle so ads can disable consent management on an ad --- ad/manager.php | 3 +- adm/style/manage_ads.html | 9 +++ controller/admin_controller.php | 16 ++++- controller/admin_input.php | 1 + controller/helper.php | 1 + event/main_listener.php | 3 +- language/en/acp.php | 4 +- migrations/v20x/m6_ad_consent_option.php | 64 +++++++++++++++++++ tests/ad/get_ad_test.php | 1 + tests/ad/get_ads_test.php | 6 +- tests/controller/admin_input_test.php | 41 ++++++------ tests/controller/helper_test.php | 6 ++ tests/event/setup_ads_consentmanager_test.php | 60 +++++++++++++++++ 13 files changed, 188 insertions(+), 27 deletions(-) create mode 100644 migrations/v20x/m6_ad_consent_option.php diff --git a/ad/manager.php b/ad/manager.php index d1f703d0..b1638279 100644 --- a/ad/manager.php +++ b/ad/manager.php @@ -89,7 +89,7 @@ public function get_ads($ad_locations, $user_groups, $non_content_page = false) $user_now = $this->user->create_datetime(); $sql_time = $this->user->get_timestamp_from_format('Y-m-d H:i:s', $user_now->format('Y-m-d H:i:s'), new \DateTimeZone('UTC')); - $sql = 'SELECT al.location_id, a.ad_id, a.ad_code, a.ad_centering + $sql = 'SELECT al.location_id, a.ad_id, a.ad_code, a.ad_centering, a.ad_consent FROM ' . $this->ad_locations_table . ' al LEFT JOIN ' . $this->ads_table . ' a ON (al.ad_id = a.ad_id) @@ -479,6 +479,7 @@ protected function intersect_ad_data($data) 'ad_owner' => '', 'ad_content_only' => '', 'ad_centering' => '', + 'ad_consent' => '', ]); } diff --git a/adm/style/manage_ads.html b/adm/style/manage_ads.html index 0678b63c..c56b2352 100644 --- a/adm/style/manage_ads.html +++ b/adm/style/manage_ads.html @@ -115,6 +115,15 @@

{{ lang('WARNING') }}

{{ lang('AD_OPTIONS') }} + {% if S_ADS_CONSENTMANAGER_AVAILABLE %} +
+

{{ lang('AD_CONSENT_EXPLAIN') }}
+
+
+
+ {% else %} + + {% endif %}

{{ lang('AD_OWNER_EXPLAIN') }}
diff --git a/controller/admin_controller.php b/controller/admin_controller.php index 73e3e111..a650265b 100644 --- a/controller/admin_controller.php +++ b/controller/admin_controller.php @@ -88,7 +88,10 @@ public function __construct(\phpbb\template\template $template, \phpbb\language\ $this->language->add_lang('posting'); // Used by banner_upload() file errors $this->language->add_lang('acp', 'phpbb/ads'); - $this->template->assign_var('S_PHPBB_ADS', true); + $this->template->assign_vars([ + 'S_PHPBB_ADS' => true, + 'S_ADS_CONSENTMANAGER_AVAILABLE' => $this->is_consent_manager_available() + ]); if (!class_exists('auth_admin')) { @@ -526,4 +529,15 @@ protected function toggle_permission($user_id) $this->auth_admin->acl_set('user', 0, $user_id, array('u_phpbb_ads' => (int) $has_ads)); } } + + /** + * Check whether Consent Manager's marketing category is available. + * + * @return bool + */ + protected function is_consent_manager_available() + { + return $this->config->offsetExists('consentmanager_marketing_enabled') + && (bool) $this->config['consentmanager_marketing_enabled']; + } } diff --git a/controller/admin_input.php b/controller/admin_input.php index 77c88c4d..6ae1fe0f 100644 --- a/controller/admin_input.php +++ b/controller/admin_input.php @@ -97,6 +97,7 @@ public function get_form_data() 'ad_owner' => $this->request->variable('ad_owner', '', true), 'ad_groups' => $this->request->variable('ad_groups', array(0)), 'ad_centering' => $this->request->variable('ad_centering', true), + 'ad_consent' => $this->request->variable('ad_consent', 1), ); // Validate form key diff --git a/controller/helper.php b/controller/helper.php index 81c64f1a..113260fc 100644 --- a/controller/helper.php +++ b/controller/helper.php @@ -103,6 +103,7 @@ public function assign_data($data, $errors) 'AD_CLICKS_LIMIT' => $data['ad_clicks_limit'], 'AD_OWNER' => $this->get_username($data['ad_owner']), 'AD_CENTERING' => $data['ad_centering'], + 'AD_CONSENT' => $data['ad_consent'] ?? 1, )); } diff --git a/event/main_listener.php b/event/main_listener.php index 2a1009c2..aded6ef0 100644 --- a/event/main_listener.php +++ b/event/main_listener.php @@ -143,9 +143,10 @@ public function setup_ads() foreach ($ads as $row) { $ad_ids[] = $row['ad_id']; + $ad_consent_enabled = $consent_enabled && (bool) ($row['ad_consent'] ?? true); $this->template->assign_vars(array( - 'AD_' . strtoupper($row['location_id']) => $this->manager->prepare_ad_code($row['ad_code'], $consent_enabled), + 'AD_' . strtoupper($row['location_id']) => $this->manager->prepare_ad_code($row['ad_code'], $ad_consent_enabled), 'AD_' . strtoupper($row['location_id']) . '_ID' => (int) $row['ad_id'], 'AD_' . strtoupper($row['location_id']) . '_CENTER' => (bool) $row['ad_centering'], )); diff --git a/language/en/acp.php b/language/en/acp.php index 4f637efd..7fcfb6c8 100644 --- a/language/en/acp.php +++ b/language/en/acp.php @@ -31,7 +31,7 @@ 'AD_NOTE' => 'Notes', 'AD_NOTE_EXPLAIN' => 'Enter any notes for this advertisement. These notes are not shown anywhere except in the ACP and are optional.', 'AD_CODE' => 'Code', - 'AD_CODE_EXPLAIN' => 'Enter the advertisement code here. All code must use HTML markup, BBCodes are not supported.

If the Consent Manager extension is enabled, ads with script tags are automatically held back until a visitor allows marketing. Normal HTML, such as images and links, will still display, so you usually do not need to change your ad code manually.

Note: If your advertisement code places cookies, collects user data, or tracks user behaviour (for example, ads from Google AdSense or other third-party ad networks), then you should enable the Advertising Disclosure in the Advertisement Management Settings panel to ensure compliance. If you are uncertain, it is recommended that you enable it.', + 'AD_CODE_EXPLAIN' => 'Enter the advertisement code here. All code must use HTML markup, BBCodes are not supported.

If the Consent Manager extension is enabled, ads with script tags can be held back until a visitor allows marketing. Normal HTML, such as images and links, will still display, so you usually do not need to change your ad code manually.

Note: If your advertisement code places cookies, collects user data, or tracks user behaviour (for example, ads from Google AdSense or other third-party ad networks), then you should enable the Advertising Disclosure in the Advertisement Management Settings panel to ensure compliance. If you are uncertain, it is recommended that you enable it.', 'ANALYSE_AD_CODE' => 'Analyse advertisement code', 'EVERYTHING_OK' => 'The code appears OK.', 'AD_BANNER' => 'Advertisement banner', @@ -56,6 +56,8 @@ 'AD_CLICKS' => 'Clicks', 'AD_CLICKS_LIMIT' => 'Clicks Limit', 'AD_CLICKS_LIMIT_EXPLAIN' => 'Set the maximum number of times the advertisement will be clicked, after which the advertisement will no longer be displayed. Set 0 for unlimited clicks.', + 'AD_CONSENT' => 'Require marketing consent', + 'AD_CONSENT_EXPLAIN' => 'Set to yes to defer script tags in this advertisement until the visitor allows marketing in Privacy Settings. Set to no only for ad code that does not load marketing, tracking, cookies, or other consent-controlled resources.', 'AD_START_DATE' => 'Start Date', 'AD_START_DATE_EXPLAIN' => 'Set the date when the advertisement can begin displaying (starting at 00:00). The ad must still be manually enabled to appear. If no date is set, the ad can display immediately once enabled.', 'AD_END_DATE' => 'End Date', diff --git a/migrations/v20x/m6_ad_consent_option.php b/migrations/v20x/m6_ad_consent_option.php new file mode 100644 index 00000000..396ec011 --- /dev/null +++ b/migrations/v20x/m6_ad_consent_option.php @@ -0,0 +1,64 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + */ + +namespace phpbb\ads\migrations\v20x; + +class m6_ad_consent_option extends \phpbb\db\migration\migration +{ + /** + * {@inheritDoc} + */ + public function effectively_installed() + { + return $this->db_tools->sql_column_exists($this->table_prefix . 'ads', 'ad_consent'); + } + + /** + * {@inheritDoc} + */ + public static function depends_on() + { + return array( + '\phpbb\ads\migrations\v20x\m5_add_privacy_setting', + ); + } + + /** + * Add the per-ad consent option to ads table. + * + * @return array Array of table schema + */ + public function update_schema() + { + return array( + 'add_columns' => array( + $this->table_prefix . 'ads' => array( + 'ad_consent' => array('BOOL', 1), + ), + ), + ); + } + + /** + * Drop the per-ad consent option from ads table. + * + * @return array Array of table schema + */ + public function revert_schema() + { + return array( + 'drop_columns' => array( + $this->table_prefix . 'ads' => array( + 'ad_consent', + ), + ), + ); + } +} diff --git a/tests/ad/get_ad_test.php b/tests/ad/get_ad_test.php index 15e61c4a..46071965 100644 --- a/tests/ad/get_ad_test.php +++ b/tests/ad/get_ad_test.php @@ -36,6 +36,7 @@ public function get_ad_data() 'ad_owner' => '2', 'ad_content_only' => '0', 'ad_centering' => '1', + 'ad_consent' => '1', )), array(0, array()), ); diff --git a/tests/ad/get_ads_test.php b/tests/ad/get_ads_test.php index 88fced45..c628c2a1 100644 --- a/tests/ad/get_ads_test.php +++ b/tests/ad/get_ads_test.php @@ -21,13 +21,13 @@ public function get_ads_data() { return array( array(array('after_profile'), array( - array('location_id' => 'after_profile', 'ad_code' => 'Ad Code #1', 'ad_id' => '1', 'ad_centering' => '1'), + array('location_id' => 'after_profile', 'ad_code' => 'Ad Code #1', 'ad_id' => '1', 'ad_centering' => '1', 'ad_consent' => '1'), ), false), array(array('before_profile'), array( - array('location_id' => 'before_profile', 'ad_code' => 'Ad Code #4', 'ad_id' => '4', 'ad_centering' => '1'), + array('location_id' => 'before_profile', 'ad_code' => 'Ad Code #4', 'ad_id' => '4', 'ad_centering' => '1', 'ad_consent' => '1'), ), false), array(array('below_footer'), array( - array('location_id' => 'below_footer', 'ad_code' => 'Ad Code #7', 'ad_id' => '7', 'ad_centering' => '1'), + array('location_id' => 'below_footer', 'ad_code' => 'Ad Code #7', 'ad_id' => '7', 'ad_centering' => '1', 'ad_consent' => '1'), ), false), array(array('below_footer'), array(), true), array(array('foo_bar'), array(), false), diff --git a/tests/controller/admin_input_test.php b/tests/controller/admin_input_test.php index 39f53d33..d01c10d8 100644 --- a/tests/controller/admin_input_test.php +++ b/tests/controller/admin_input_test.php @@ -107,22 +107,22 @@ public function get_input_controller() public function get_form_data_data() { return array( - array(false, ['Ad Name #1', 'Ad Note #1', 'Ad Code #1', 0, '', '', '', 5, 0, 0, 0, '', [], false], 0, ['FORM_INVALID']), - array(true, ['', 'Ad Note #1', 'Ad Code #1', 0, '', '', '', 5, 0, 0, 0, '', [], false], 0, ['AD_NAME_REQUIRED']), - array(true, [str_repeat('a', 256), 'Ad Note #1', 'Ad Code #1', 0, '', '', '', 5, 0, 0, 0, '', [], false], 0, ['AD_NAME_TOO_LONG']), - array(true, ['Ad Name #1', 'Ad Note #1', 'Ad Code with emoji šŸ˜€', 0, '', '', '', 5, 0, 0, 0, '', [], false], 0, ['AD_CODE_ILLEGAL_CHARS']), - array(true, ['Ad Name #1', 'Ad Note #1', 'Ad Code #1', 0, '', 'blah', '', 5, 0, 0, 0, '', [], false], 0, ['AD_START_DATE_INVALID']), - array(true, ['Ad Name #1', 'Ad Note #1', 'Ad Code #1', 0, '', '', 'blah', 5, 0, 0, 0, '', [], false], 0, ['AD_END_DATE_INVALID']), - array(true, ['Ad Name #1', 'Ad Note #1', 'Ad Code #1', 0, '', '1970-01-01', '', 5, 0, 0, 0, '', [], false], 0, ['AD_START_DATE_INVALID']), - array(true, ['Ad Name #1', 'Ad Note #1', 'Ad Code #1', 0, '', '', '1970-01-01', 5, 0, 0, 0, '', [], false], 0, ['AD_END_DATE_INVALID']), - array(true, ['Ad Name #1', 'Ad Note #1', 'Ad Code #1', 0, '', '2060-01-01', '2050-01-01', 5, 0, 0, 0, '', [], false], 0, ['END_DATE_TOO_SOON']), - array(true, ['Ad Name #1', 'Ad Note #1', 'Ad Code #1', 0, '', '', '', 0, 0, 0, 0, '', [], false], 0, ['AD_PRIORITY_INVALID']), - array(true, ['Ad Name #1', 'Ad Note #1', 'Ad Code #1', 0, '', '', '', 11, 0, 0, 0, '', [], false], 0, ['AD_PRIORITY_INVALID']), - array(true, ['Ad Name #1', 'Ad Note #1', 'Ad Code #1', 0, '', '', '', 5, 0, -1, 0, '', [], false], 0, ['AD_VIEWS_LIMIT_INVALID']), - array(true, ['Ad Name #1', 'Ad Note #1', 'Ad Code #1', 0, '', '', '', 5, 0, 0, -1, '', [], false], 0, ['AD_CLICKS_LIMIT_INVALID']), - array(true, ['Ad Name #1', 'Ad Note #1', 'Ad Code #1', 0, '', '', '', 5, 0, 0, 0, 'adm', [], false], 0, ['AD_OWNER_INVALID']), - array(true, ['Ad Name #1', 'Ad Note #1', 'Ad Code #1', 0, '', '', '', 5, 0, 0, 0, 'adm', [], false], 0, ['AD_OWNER_INVALID']), - array(false, ['', 'Ad Note #1', 'Ad Code #1', 0, '', 'blah', 'blah', 0, 0, -1, -1, 'adm', [], false], 0, [ + array(false, ['Ad Name #1', 'Ad Note #1', 'Ad Code #1', 0, '', '', '', 5, 0, 0, 0, '', [], false, 1], 0, ['FORM_INVALID']), + array(true, ['', 'Ad Note #1', 'Ad Code #1', 0, '', '', '', 5, 0, 0, 0, '', [], false, 1], 0, ['AD_NAME_REQUIRED']), + array(true, [str_repeat('a', 256), 'Ad Note #1', 'Ad Code #1', 0, '', '', '', 5, 0, 0, 0, '', [], false, 1], 0, ['AD_NAME_TOO_LONG']), + array(true, ['Ad Name #1', 'Ad Note #1', 'Ad Code with emoji šŸ˜€', 0, '', '', '', 5, 0, 0, 0, '', [], false, 1], 0, ['AD_CODE_ILLEGAL_CHARS']), + array(true, ['Ad Name #1', 'Ad Note #1', 'Ad Code #1', 0, '', 'blah', '', 5, 0, 0, 0, '', [], false, 1], 0, ['AD_START_DATE_INVALID']), + array(true, ['Ad Name #1', 'Ad Note #1', 'Ad Code #1', 0, '', '', 'blah', 5, 0, 0, 0, '', [], false, 1], 0, ['AD_END_DATE_INVALID']), + array(true, ['Ad Name #1', 'Ad Note #1', 'Ad Code #1', 0, '', '1970-01-01', '', 5, 0, 0, 0, '', [], false, 1], 0, ['AD_START_DATE_INVALID']), + array(true, ['Ad Name #1', 'Ad Note #1', 'Ad Code #1', 0, '', '', '1970-01-01', 5, 0, 0, 0, '', [], false, 1], 0, ['AD_END_DATE_INVALID']), + array(true, ['Ad Name #1', 'Ad Note #1', 'Ad Code #1', 0, '', '2060-01-01', '2050-01-01', 5, 0, 0, 0, '', [], false, 1], 0, ['END_DATE_TOO_SOON']), + array(true, ['Ad Name #1', 'Ad Note #1', 'Ad Code #1', 0, '', '', '', 0, 0, 0, 0, '', [], false, 1], 0, ['AD_PRIORITY_INVALID']), + array(true, ['Ad Name #1', 'Ad Note #1', 'Ad Code #1', 0, '', '', '', 11, 0, 0, 0, '', [], false, 1], 0, ['AD_PRIORITY_INVALID']), + array(true, ['Ad Name #1', 'Ad Note #1', 'Ad Code #1', 0, '', '', '', 5, 0, -1, 0, '', [], false, 1], 0, ['AD_VIEWS_LIMIT_INVALID']), + array(true, ['Ad Name #1', 'Ad Note #1', 'Ad Code #1', 0, '', '', '', 5, 0, 0, -1, '', [], false, 1], 0, ['AD_CLICKS_LIMIT_INVALID']), + array(true, ['Ad Name #1', 'Ad Note #1', 'Ad Code #1', 0, '', '', '', 5, 0, 0, 0, 'adm', [], false, 1], 0, ['AD_OWNER_INVALID']), + array(true, ['Ad Name #1', 'Ad Note #1', 'Ad Code #1', 0, '', '', '', 5, 0, 0, 0, 'adm', [], false, 1], 0, ['AD_OWNER_INVALID']), + array(false, ['', 'Ad Note #1', 'Ad Code #1', 0, '', 'blah', 'blah', 0, 0, -1, -1, 'adm', [], false, 1], 0, [ 'FORM_INVALID', 'AD_NAME_REQUIRED', 'AD_START_DATE_INVALID', @@ -132,7 +132,7 @@ public function get_form_data_data() 'AD_CLICKS_LIMIT_INVALID', 'AD_OWNER_INVALID', ]), - array(true, ['Ad Name #1', 'Ad Note #1', 'Ad Code #1', '1', array('above_header', 'above_footer'), '2018-01-01', '2033-01-01', '4', '1', '50', '30', 'admin', ['5'], 0], 2, []), + array(true, ['Ad Name #1', 'Ad Note #1', 'Ad Code #1', '1', array('above_header', 'above_footer'), '2018-01-01', '2033-01-01', '4', '1', '50', '30', 'admin', ['5'], 0, 0], 2, []), ); } @@ -143,14 +143,14 @@ public function get_form_data_data() */ public function test_get_form_data($valid_form, $data, $ad_owner_expected, $errors) { - [$ad_name, $ad_note, $ad_code, $ad_enabled, $ad_locations, $ad_start_date, $ad_end_date, $ad_priority, $ad_content_only, $ad_views_limit, $ad_clicks_limit, $ad_owner, $ad_groups, $ad_centering] = $data; + [$ad_name, $ad_note, $ad_code, $ad_enabled, $ad_locations, $ad_start_date, $ad_end_date, $ad_priority, $ad_content_only, $ad_views_limit, $ad_clicks_limit, $ad_owner, $ad_groups, $ad_centering, $ad_consent] = $data; self::$valid_form = $valid_form; $input_controller = $this->get_input_controller(); - $this->request->expects(self::exactly(14)) + $this->request->expects(self::exactly(15)) ->method('variable') - ->will(self::onConsecutiveCalls($ad_name, $ad_note, $ad_code, $ad_enabled, $ad_locations, $ad_start_date, $ad_end_date, $ad_priority, $ad_content_only, $ad_views_limit, $ad_clicks_limit, $ad_owner, $ad_groups, $ad_centering)); + ->will(self::onConsecutiveCalls($ad_name, $ad_note, $ad_code, $ad_enabled, $ad_locations, $ad_start_date, $ad_end_date, $ad_priority, $ad_content_only, $ad_views_limit, $ad_clicks_limit, $ad_owner, $ad_groups, $ad_centering, $ad_consent)); $result = $input_controller->get_form_data(); @@ -176,6 +176,7 @@ public function test_get_form_data($valid_form, $data, $ad_owner_expected, $erro 'ad_owner' => $ad_owner_expected, 'ad_groups' => $ad_groups, 'ad_centering' => $ad_centering, + 'ad_consent' => $ad_consent, ), $result); } } diff --git a/tests/controller/helper_test.php b/tests/controller/helper_test.php index 5d8a1eb7..79f035f6 100644 --- a/tests/controller/helper_test.php +++ b/tests/controller/helper_test.php @@ -152,6 +152,7 @@ public function assign_data_data() 'ad_clicks_limit' => 0, 'ad_owner' => 0, 'ad_centering' => false, + 'ad_consent' => 1, 'ad_locations' => [], ), '', array('AD_PRIORITY_INVALID'), true, 'AD_PRIORITY_INVALID'), array(array( @@ -167,6 +168,7 @@ public function assign_data_data() 'ad_clicks_limit' => 0, 'ad_owner' => 0, 'ad_centering' => 0, + 'ad_consent' => 1, 'ad_locations' => [], ), '', array('AD_PRIORITY_INVALID', 'AD_NAME_REQUIRED'), true, 'AD_PRIORITY_INVALID
AD_NAME_REQUIRED'), array(array( @@ -182,6 +184,7 @@ public function assign_data_data() 'ad_clicks_limit' => 0, 'ad_owner' => 99, 'ad_centering' => 0, + 'ad_consent' => 1, 'ad_locations' => [], ), 'Anonymous', array(), false, ''), array(array( @@ -197,6 +200,7 @@ public function assign_data_data() 'ad_clicks_limit' => 0, 'ad_owner' => 99, 'ad_centering' => 0, + 'ad_consent' => 1, 'ad_locations' => [], ), 'Anonymous', array(), false, ''), array(array( @@ -212,6 +216,7 @@ public function assign_data_data() 'ad_clicks_limit' => 0, 'ad_owner' => 2, 'ad_centering' => 0, + 'ad_consent' => 0, 'ad_locations' => [], ), 'admin', array(), false, ''), ); @@ -252,6 +257,7 @@ public function test_assign_data($data, $owner, $errors, $s_errors, $error_msg) 'AD_CLICKS_LIMIT' => $data['ad_clicks_limit'], 'AD_OWNER' => $owner, 'AD_CENTERING' => $data['ad_centering'], + 'AD_CONSENT' => $data['ad_consent'], )); $helper->assign_data($data, $errors); diff --git a/tests/event/setup_ads_consentmanager_test.php b/tests/event/setup_ads_consentmanager_test.php index 1c1e8662..3d4580fa 100644 --- a/tests/event/setup_ads_consentmanager_test.php +++ b/tests/event/setup_ads_consentmanager_test.php @@ -187,4 +187,64 @@ public function test_setup_ads_does_not_defer_when_marketing_category_is_disable $this->get_listener()->setup_ads(); } + + public function test_setup_ads_does_not_defer_when_ad_consent_is_disabled() + { + $stored_ad_code = htmlspecialchars( + '', + ENT_COMPAT + ); + + $this->user->data['user_id'] = 1; + $this->user->page['page_name'] = 'index.' . $this->php_ext; + $this->user->page['page_dir'] = ''; + + $this->manager = $this->getMockBuilder('\phpbb\ads\ad\manager') + ->disableOriginalConstructor() + ->setMethods(array('load_memberships', 'get_ads')) + ->getMock(); + $this->location_manager = $this->getMockBuilder('\phpbb\ads\location\manager') + ->disableOriginalConstructor() + ->setMethods(array('get_all_location_ids')) + ->getMock(); + + $this->location_manager->expects(self::once()) + ->method('get_all_location_ids') + ->willReturn(array('above_header')); + + $this->manager->expects(self::once()) + ->method('load_memberships') + ->with(1) + ->willReturn(array()); + + $this->manager->expects(self::once()) + ->method('get_ads') + ->with(array('above_header'), array(), false) + ->willReturn(array(array( + 'location_id' => 'above_header', + 'ad_id' => 78, + 'ad_code' => $stored_ad_code, + 'ad_centering' => 0, + 'ad_consent' => 0, + ))); + + $this->template->expects(self::exactly(2)) + ->method('retrieve_var') + ->willReturnCallback(function ($var_name) + { + return $var_name === 'S_CONSENTMANAGER_MARKETING_ENABLED'; + }); + + $this->template->expects(self::once()) + ->method('assign_vars') + ->with(self::callback(function ($vars) + { + return $vars['AD_ABOVE_HEADER_ID'] === 78 + && strpos($vars['AD_ABOVE_HEADER'], 'type="text/plain"') === false + && strpos($vars['AD_ABOVE_HEADER'], 'data-consent-category="marketing"') === false + && strpos($vars['AD_ABOVE_HEADER'], 'src="https://ads.example.com/tag.js"') !== false; + })); + + $this->get_listener()->setup_ads(); + } } From 0a6c0a79c551b90f74966ffe2e7a9911a277fbc3 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Tue, 26 May 2026 11:31:37 -0700 Subject: [PATCH 03/10] Remove functional consent manager test since it depends on the extension --- tests/functional/ads_consentmanager_test.php | 40 -------------------- 1 file changed, 40 deletions(-) delete mode 100644 tests/functional/ads_consentmanager_test.php diff --git a/tests/functional/ads_consentmanager_test.php b/tests/functional/ads_consentmanager_test.php deleted file mode 100644 index 9d99b4b0..00000000 --- a/tests/functional/ads_consentmanager_test.php +++ /dev/null @@ -1,40 +0,0 @@ -create_ad( - 'above_header', - '', - false, - true, - '', - '
Example ad
' - ); - - $crawler = self::request('GET', 'index.php'); - - self::assertSame(1, $crawler->filter('.phpbb-ads script[type="text/plain"][data-consent-category="marketing"][src*="ads.example.com/tag.js"]')->count()); - self::assertSame(0, $crawler->filter('.phpbb-ads script[src*="ads.example.com/tag.js"]:not([type="text/plain"])')->count()); - self::assertSame(1, $crawler->filter('.phpbb-ads iframe[src*="ads.example.com/frame"]')->count()); - } -} From e967caf95f6a94fcd8daf5e1841009fd08833ede Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Tue, 26 May 2026 12:10:56 -0700 Subject: [PATCH 04/10] Update tests --- tests/ad/prepare_ad_code_test.php | 52 +++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 10 deletions(-) diff --git a/tests/ad/prepare_ad_code_test.php b/tests/ad/prepare_ad_code_test.php index 8e90477e..20d217d3 100644 --- a/tests/ad/prepare_ad_code_test.php +++ b/tests/ad/prepare_ad_code_test.php @@ -30,27 +30,59 @@ public function test_returns_empty_string_unchanged() self::assertSame('', $this->get_manager()->prepare_ad_code('', true)); } - public function test_defers_script_tag_when_consent_enabled() + public function executable_script_type_data() { - $raw = htmlspecialchars('', ENT_COMPAT); + return [ + 'normal script' => [ + '', + '', + ], + 'empty type' => [ + '', + '', + ], + 'text/plain type' => [ + '', + '', + ], + 'module type' => [ + '', + '', + ], + 'javascript type with charset' => [ + '', + '', + ], + 'ecmascript type' => [ + '', + '', + ], + ]; + } + + /** + * @dataProvider executable_script_type_data + */ + public function test_defers_executable_script_types($input, $expected) + { + $raw = htmlspecialchars($input, ENT_COMPAT); $result = $this->get_manager()->prepare_ad_code($raw, true); - self::assertStringContainsString('type="text/plain"', $result); - self::assertStringContainsString('data-consent-category="marketing"', $result); - self::assertStringContainsString('src="https://ads.example.com/tag.js"', $result); + self::assertSame($expected, $result); } - public function test_adds_consent_category_to_existing_text_plain_script() + public function test_preserves_non_executable_script_type() { - $raw = htmlspecialchars('', ENT_COMPAT); + $raw = htmlspecialchars('', ENT_COMPAT); $result = $this->get_manager()->prepare_ad_code($raw, true); - self::assertStringContainsString('type="text/plain"', $result); - self::assertStringContainsString('data-consent-category="marketing"', $result); + self::assertSame('', $result); } public function test_does_not_double_wrap_already_tagged_script() { - $raw = htmlspecialchars('', ENT_COMPAT); + $script = ''; + $raw = htmlspecialchars($script, ENT_COMPAT); $result = $this->get_manager()->prepare_ad_code($raw, true); + self::assertSame($script, $result); self::assertSame(1, substr_count($result, 'data-consent-category=')); } From 8861cfecca4e7ec8013ad0b4c9ca68ff47162cfd Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Wed, 3 Jun 2026 09:33:40 -0700 Subject: [PATCH 05/10] Ad code analyser will flag ads that could require consent --- analyser/manager.php | 5 +- analyser/test/alert.php | 2 +- analyser/test/location_href.php | 2 +- analyser/test/marketing_consent.php | 167 +++++++++++++++++++++ analyser/test/script_without_async.php | 2 +- analyser/test/test_interface.php | 3 +- analyser/test/untrusted_connection.php | 2 +- config/analyser.yml | 7 + controller/admin_controller.php | 4 +- language/en/acp.php | 2 + tests/analyser/analyser_base.php | 11 ++ tests/analyser/run_test.php | 83 ++++++++-- tests/controller/admin_controller_test.php | 2 +- 13 files changed, 267 insertions(+), 25 deletions(-) create mode 100644 analyser/test/marketing_consent.php diff --git a/analyser/manager.php b/analyser/manager.php index 0611e988..52368b3c 100644 --- a/analyser/manager.php +++ b/analyser/manager.php @@ -39,13 +39,14 @@ public function __construct($tests, \phpbb\template\template $template, \phpbb\l * Test the ad code for potential problems. * * @param string $ad_code Advertisement code + * @param array $context Optional form context */ - public function run($ad_code) + public function run($ad_code, array $context = array()) { $results = array(); foreach ($this->tests as $test) { - $result = $test->run($ad_code); + $result = $test->run($ad_code, $context); if ($result !== false) { $results[] = $result; diff --git a/analyser/test/alert.php b/analyser/test/alert.php index e15be5f3..e0a3524a 100644 --- a/analyser/test/alert.php +++ b/analyser/test/alert.php @@ -20,7 +20,7 @@ class alert implements test_interface * There is no reason why ad would trigger alert, so it's * categorized as warning. */ - public function run($ad_code) + public function run($ad_code, array $context = array()) { if (preg_match('/alert\s*\(/U', $ad_code)) { diff --git a/analyser/test/location_href.php b/analyser/test/location_href.php index 8909aa9a..026c1c46 100644 --- a/analyser/test/location_href.php +++ b/analyser/test/location_href.php @@ -20,7 +20,7 @@ class location_href implements test_interface * There is no reason why ad would redirect user to another page, * so it's categorized as warning. */ - public function run($ad_code) + public function run($ad_code, array $context = array()) { if (preg_match('/location\.href(\s)*=/U', $ad_code)) { diff --git a/analyser/test/marketing_consent.php b/analyser/test/marketing_consent.php new file mode 100644 index 00000000..adcd9282 --- /dev/null +++ b/analyser/test/marketing_consent.php @@ -0,0 +1,167 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + */ + +namespace phpbb\ads\analyser\test; + +class marketing_consent implements test_interface +{ + /** + * Common advertising and marketing script hosts. + * + * Keep this list script-focused. Ads extension only defers script tags, so + * host hints should not expand warnings to iframe-only or image-only embeds. + */ + protected const MARKETING_HOST_PATTERNS = array( + '/(^|[\/.])pagead2\.googlesyndication\.com(?=[:\/]|$)/i', + '/(^|[\/.])partner\.googleadservices\.com(?=[:\/]|$)/i', + '/(^|[\/.])googleads\.g\.doubleclick\.net(?=[:\/]|$)/i', + '/(^|[\/.])securepubads\.g\.doubleclick\.net(?=[:\/]|$)/i', + '/(^|[\/.])www\.googletagservices\.com(?=[:\/]|$)/i', + '/(^|[\/.])www\.googletagmanager\.com(?=[:\/]|$)/i', + '/(^|[\/.])c\.amazon-adsystem\.com(?=[:\/]|$)/i', + '/(^|[\/.])aax\.amazon-adsystem\.com(?=[:\/]|$)/i', + '/(^|[\/.])trc\.taboola\.com(?=[:\/]|$)/i', + '/(^|[\/.])cdn\.taboola\.com(?=[:\/]|$)/i', + '/(^|[\/.])widgets\.outbrain\.com(?=[:\/]|$)/i', + '/(^|[\/.])odr\.outbrain\.com(?=[:\/]|$)/i', + '/(^|[\/.])static\.criteo\.net(?=[:\/]|$)/i', + '/(^|[\/.])gum\.criteo\.com(?=[:\/]|$)/i', + '/(^|[\/.])secure\.adnxs\.com(?=[:\/]|$)/i', + '/(^|[\/.])ib\.adnxs\.com(?=[:\/]|$)/i', + ); + + /** @var \phpbb\config\config */ + protected $config; + + /** + * @param \phpbb\config\config $config Config object + */ + public function __construct(\phpbb\config\config $config) + { + $this->config = $config; + } + + /** + * {@inheritDoc} + * + * Suggest enabling Require marketing consent when executable script tags are + * present, Consent Manager marketing is available, and the ad-level consent + * toggle is currently disabled. + */ + public function run($ad_code, array $context = array()) + { + if (!$this->config->offsetExists('consentmanager_marketing_enabled') + || empty($this->config['consentmanager_marketing_enabled']) + || !isset($context['ad_consent']) + || !empty($context['ad_consent'])) + { + return false; + } + + $decoded = htmlspecialchars_decode($ad_code, ENT_COMPAT); + $message = $this->get_recommendation_message($decoded); + if ($message === false) + { + return false; + } + + return array( + 'severity' => 'notice', + 'message' => $message, + ); + } + + /** + * Get consent recommendation message for ad code, if any. + * + * @param string $ad_code Advertisement code + * @return string|false + */ + protected function get_recommendation_message($ad_code) + { + if (!preg_match_all('#]*)>(.*?)#is', $ad_code, $matches)) + { + return false; + } + + foreach ($matches[1] as $index => $attributes) + { + if (!$this->should_flag_script_tag($attributes)) + { + continue; + } + + $content = $matches[2][$index] ?? ''; + if ($this->contains_marketing_host_hint($attributes, $content)) + { + return 'MARKETING_CONSENT_VENDOR_RECOMMENDED'; + } + + return 'MARKETING_CONSENT_RECOMMENDED'; + } + + return false; + } + + /** + * Check for known advertising vendor hints inside script markup or content. + * + * @param string $attributes Script tag attributes + * @param string $content Script tag content + * @return bool + */ + protected function contains_marketing_host_hint($attributes, $content) + { + $haystacks = array($attributes, $content); + + if (preg_match('/\bsrc\s*=\s*([\'"])(.*?)\1/i', $attributes, $matches)) + { + $haystacks[] = $matches[2]; + } + + foreach ($haystacks as $haystack) + { + foreach (self::MARKETING_HOST_PATTERNS as $pattern) + { + if (preg_match($pattern, $haystack)) + { + return true; + } + } + } + + return false; + } + + /** + * Mirror ads defer logic closely enough to avoid flagging inert script types. + * + * @param string $attributes Script tag attributes + * @return bool + */ + protected function should_flag_script_tag($attributes) + { + if (preg_match('/\bdata-consent-category\s*=/i', $attributes)) + { + return false; + } + + if (!preg_match('/\btype\s*=\s*([\'"])(.*?)\1/i', $attributes, $matches)) + { + return true; + } + + $type = strtolower(trim(explode(';', $matches[2])[0])); + return $type === '' + || $type === 'module' + || strpos($type, 'javascript') !== false + || strpos($type, 'ecmascript') !== false; + } +} diff --git a/analyser/test/script_without_async.php b/analyser/test/script_without_async.php index 2ab7e7b8..3b6828bb 100644 --- a/analyser/test/script_without_async.php +++ b/analyser/test/script_without_async.php @@ -20,7 +20,7 @@ class script_without_async implements test_interface * to load itself asynchronously. Such scripts slow down page rendering * time and should be made asynchronous. */ - public function run($ad_code) + public function run($ad_code, array $context = array()) { if (preg_match_all('/<script(.*)src(.*)>/U', $ad_code, $matches)) { diff --git a/analyser/test/test_interface.php b/analyser/test/test_interface.php index 03b235d8..8445d20a 100644 --- a/analyser/test/test_interface.php +++ b/analyser/test/test_interface.php @@ -19,7 +19,8 @@ interface test_interface * Test ad code for potential problems. * * @param string $ad_code Advertisement code + * @param array $context Optional form context * @return mixed List of notices and warnings or false when there are none. */ - public function run($ad_code); + public function run($ad_code, array $context = array()); } diff --git a/analyser/test/untrusted_connection.php b/analyser/test/untrusted_connection.php index c13194bc..7b9723fe 100644 --- a/analyser/test/untrusted_connection.php +++ b/analyser/test/untrusted_connection.php @@ -32,7 +32,7 @@ public function __construct(\phpbb\request\request $request) * When board runs on HTTPS and ad tries to load a file from * HTTP source, browser throws a warning. We should prevent that. */ - public function run($ad_code) + public function run($ad_code, array $context = array()) { $is_https = $this->request->server('HTTPS', false); if ($is_https && preg_match('/http[^s]/', $ad_code)) diff --git a/config/analyser.yml b/config/analyser.yml index 65ce08c3..ba6cce58 100644 --- a/config/analyser.yml +++ b/config/analyser.yml @@ -29,6 +29,13 @@ services: tags: - { name: phpbb.ads.analyser.test } + phpbb.ads.analyser.test.marketing_consent: + class: phpbb\ads\analyser\test\marketing_consent + arguments: + - '@config' + tags: + - { name: phpbb.ads.analyser.test } + phpbb.ads.analyser.test.untrusted_connection: class: phpbb\ads\analyser\test\untrusted_connection arguments: diff --git a/controller/admin_controller.php b/controller/admin_controller.php index a650265b..9fcd1161 100644 --- a/controller/admin_controller.php +++ b/controller/admin_controller.php @@ -430,13 +430,13 @@ protected function upload_banner() /** * Submit action "analyse_ad_code". - * Upload banner and append it to the ad code. + * Analyse submitted ad code with current form state. * * @return void */ protected function analyse_ad_code() { - $this->analyser->run($this->data['ad_code']); + $this->analyser->run($this->data['ad_code'], $this->data); } /** diff --git a/language/en/acp.php b/language/en/acp.php index 7fcfb6c8..fe62907c 100644 --- a/language/en/acp.php +++ b/language/en/acp.php @@ -101,6 +101,8 @@ // Analyser tests 'UNSECURE_CONNECTION' => 'Mixed Content
Your board runs on a secure HTTPS connection; however, the advertisement code is attempting to load content from an insecure HTTP connection. This can cause browsers to generate a ā€œMixed Contentā€ warning to let users know that the page contains insecure resources.', 'SCRIPT_WITHOUT_ASYNC' => 'Non-asynchronous javascript
This advertisement code loads JavaScript code in a non-asynchronous way. This means it will block any other JavaScript from loading until it has completed loading, which can affect page load performance. Use of the async attribute can speed up the page load.', + 'MARKETING_CONSENT_RECOMMENDED' => 'Require marketing consent
This advertisement contains executable <script> tags. If this ad loads marketing, tracking, cookies, or other consent-controlled resources, enable Require marketing consent below for this ad so its scripts are deferred until the visitor allows marketing in Privacy Settings.', + 'MARKETING_CONSENT_VENDOR_RECOMMENDED' => 'Known ad vendor detected
This advertisement contains executable <script> tags from a known advertising or marketing vendor. Enable Require marketing consent below for this ad so its scripts are deferred until the visitor allows marketing in Privacy Settings.', 'ALERT_USAGE' => 'Usage of alert()
Your code uses the alert() function which is not a good practice and can distract users. Some browsers may also block page load and display additional warnings to the user.', 'LOCATION_CHANGE' => 'Redirection
Your code appears it can redirect a user to another page or site. Redirects can sometimes send users to unintended, often malicious, destinations. Please verify the integrity of your advertisement codeadvertisement codeā€™s redirection destination.', diff --git a/tests/analyser/analyser_base.php b/tests/analyser/analyser_base.php index bdde8299..0d87a494 100644 --- a/tests/analyser/analyser_base.php +++ b/tests/analyser/analyser_base.php @@ -24,6 +24,9 @@ class analyser_base extends \phpbb_test_case /** @var \phpbb\language\language */ protected $lang; + /** @var \phpbb\config\config */ + protected $config; + protected static function setup_extensions() { return array('phpbb/ads'); @@ -47,12 +50,16 @@ protected function setUp(): void ->disableOriginalConstructor() ->getMock(); $this->lang = new \phpbb\language\language($lang_loader); + $this->config = new \phpbb\config\config(array( + 'consentmanager_marketing_enabled' => 0, + )); // Tests $tests = array( 'alert', 'location_href', 'script_without_async', + 'marketing_consent', 'untrusted_connection', ); $analyser_tests = array(); @@ -63,6 +70,10 @@ protected function setUp(): void { $analyser_tests['phpbb.ads.analyser.test.' . $test] = new $class($this->request); } + elseif ($test === 'marketing_consent') + { + $analyser_tests['phpbb.ads.analyser.test.' . $test] = new $class($this->config); + } else { $analyser_tests['phpbb.ads.analyser.test.' . $test] = new $class(); diff --git a/tests/analyser/run_test.php b/tests/analyser/run_test.php index 6f67f46d..168bf85e 100644 --- a/tests/analyser/run_test.php +++ b/tests/analyser/run_test.php @@ -20,64 +20,64 @@ class run_test extends analyser_base public function run_data() { return array( - array('<script async>alert()</script>', false, array( + array('<script async>alert()</script>', false, array(), array( array( 'severity' => 'warning', 'lang_key' => 'ALERT_USAGE', ), )), - array('<script async>alert ()</script>', false, array( + array('<script async>alert ()</script>', false, array(), array( array( 'severity' => 'warning', 'lang_key' => 'ALERT_USAGE', ), )), - array('<script async>window.location.href = "new url"</script>', false, array( + array('<script async>window.location.href = "new url"</script>', false, array(), array( array( 'severity' => 'warning', 'lang_key' => 'LOCATION_CHANGE', ), )), - array('<script async>window.location.href= "new url"</script>', false, array( + array('<script async>window.location.href= "new url"</script>', false, array(), array( array( 'severity' => 'warning', 'lang_key' => 'LOCATION_CHANGE', ), )), - array('<script></script>', false, array()), - array('<script src="script src"></script>', false, array( + array('<script></script>', false, array(), array()), + array('<script src="script src"></script>', false, array(), array( array( 'severity' => 'notice', 'lang_key' => 'SCRIPT_WITHOUT_ASYNC', ), )), - array('<script src="script src"></script><script src="another script src"></script>', false, array( + array('<script src="script src"></script><script src="another script src"></script>', false, array(), array( array( 'severity' => 'notice', 'lang_key' => 'SCRIPT_WITHOUT_ASYNC', ), )), - array('<script async src="script src"></script><script src="another script src"></script>', false, array( + array('<script async src="script src"></script><script src="another script src"></script>', false, array(), array( array( 'severity' => 'notice', 'lang_key' => 'SCRIPT_WITHOUT_ASYNC', ), )), - array('<script src="script src"></script><script async src="another script src"></script>', false, array( + array('<script src="script src"></script><script async src="another script src"></script>', false, array(), array( array( 'severity' => 'notice', 'lang_key' => 'SCRIPT_WITHOUT_ASYNC', ), )), - array('<script async src="http://some.url"></script>', false, array()), - array('<script async src="https://some.url"></script>', true, array()), - array('<script async src="http://some.url"></script>', true, array( + array('<script async src="http://some.url"></script>', false, array(), array()), + array('<script async src="https://some.url"></script>', true, array(), array()), + array('<script async src="http://some.url"></script>', true, array(), array( array( 'severity' => 'warning', 'lang_key' => 'UNSECURE_CONNECTION', ), )), - array('<script src="http://some.url"></script><script>alert("e");window.location.href="new url"</script>', true, array( + array('<script src="http://some.url"></script><script>alert("e");window.location.href="new url"</script>', true, array(), array( array( 'severity' => 'warning', 'lang_key' => 'ALERT_USAGE', @@ -95,6 +95,58 @@ public function run_data() 'lang_key' => 'UNSECURE_CONNECTION', ), )), + array('', false, array( + 'ad_consent' => 0, + 'consentmanager_marketing_enabled' => 1, + ), array( + array( + 'severity' => 'notice', + 'lang_key' => 'MARKETING_CONSENT_RECOMMENDED', + ), + )), + array('', false, array( + 'ad_consent' => 0, + 'consentmanager_marketing_enabled' => 1, + ), array( + array( + 'severity' => 'notice', + 'lang_key' => 'MARKETING_CONSENT_RECOMMENDED', + ), + )), + array('', false, array( + 'ad_consent' => 0, + 'consentmanager_marketing_enabled' => 1, + ), array( + array( + 'severity' => 'notice', + 'lang_key' => 'MARKETING_CONSENT_VENDOR_RECOMMENDED', + ), + )), + array('', false, array( + 'ad_consent' => 0, + 'consentmanager_marketing_enabled' => 1, + ), array( + array( + 'severity' => 'notice', + 'lang_key' => 'MARKETING_CONSENT_VENDOR_RECOMMENDED', + ), + )), + array('', false, array( + 'ad_consent' => 0, + 'consentmanager_marketing_enabled' => 1, + ), array()), + array('', false, array( + 'ad_consent' => 0, + 'consentmanager_marketing_enabled' => 1, + ), array()), + array('', false, array( + 'ad_consent' => 1, + 'consentmanager_marketing_enabled' => 1, + ), array()), + array('', false, array( + 'ad_consent' => 0, + 'consentmanager_marketing_enabled' => 0, + ), array()), ); } @@ -103,9 +155,10 @@ public function run_data() * * @dataProvider run_data */ - public function test_run($ad_code, $is_https, $expected) + public function test_run($ad_code, $is_https, $context, $expected) { $manager = $this->get_manager(); + $this->config['consentmanager_marketing_enabled'] = $context['consentmanager_marketing_enabled'] ?? 0; $this->request ->method('server') @@ -132,6 +185,6 @@ public function test_run($ad_code, $is_https, $expected) ->method('assign_block_vars'); } - $manager->run($ad_code); + $manager->run($ad_code, $context); } } diff --git a/tests/controller/admin_controller_test.php b/tests/controller/admin_controller_test.php index 1bead390..391f2bc0 100644 --- a/tests/controller/admin_controller_test.php +++ b/tests/controller/admin_controller_test.php @@ -479,7 +479,7 @@ public function test_action_add_analyse_ad_code() $this->analyser->expects(self::once()) ->method('run') - ->with($data['ad_code']); + ->with($data['ad_code'], $data); $this->input->expects(self::once()) ->method('get_errors') From ac94cf0c22962099dd41d482872bbe0b18f1bf6c Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Wed, 3 Jun 2026 12:11:19 -0700 Subject: [PATCH 06/10] Test for iframes --- analyser/test/iframe.php | 34 +++++++++++++++++++++++++++++ analyser/test/marketing_consent.php | 2 +- config/analyser.yml | 6 +++++ language/en/acp.php | 3 ++- tests/analyser/analyser_base.php | 3 ++- tests/analyser/run_test.php | 7 ++++++ 6 files changed, 52 insertions(+), 3 deletions(-) create mode 100644 analyser/test/iframe.php diff --git a/analyser/test/iframe.php b/analyser/test/iframe.php new file mode 100644 index 00000000..e20b3b32 --- /dev/null +++ b/analyser/test/iframe.php @@ -0,0 +1,34 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + */ + +namespace phpbb\ads\analyser\test; + +class iframe implements test_interface +{ + /** + * {@inheritDoc} + * + * Iframes test. + * This test looks for iframe tags with src attributes. Such scripts could introduce + * external trackers and data collectors that could require user consent. + */ + public function run($ad_code, array $context = array()) + { + if (preg_match('/<iframe(?>(?!>).)*?(?<=\s|")src\s*=\s*".*?>/is', $ad_code)) + { + return array( + 'severity' => 'notice', + 'message' => 'IFRAME_USAGE', + ); + } + + return false; + } +} diff --git a/analyser/test/marketing_consent.php b/analyser/test/marketing_consent.php index adcd9282..e055a9f2 100644 --- a/analyser/test/marketing_consent.php +++ b/analyser/test/marketing_consent.php @@ -3,7 +3,7 @@ * * Advertisement management. An extension for the phpBB Forum Software package. * - * @copyright (c) 2017 phpBB Limited + * @copyright (c) 2026 phpBB Limited * @license GNU General Public License, version 2 (GPL-2.0) * */ diff --git a/config/analyser.yml b/config/analyser.yml index ba6cce58..82fdafa1 100644 --- a/config/analyser.yml +++ b/config/analyser.yml @@ -42,3 +42,9 @@ services: - '@request' tags: - { name: phpbb.ads.analyser.test } + + phpbb.ads.analyser.test.iframe: + class: phpbb\ads\analyser\test\iframe + tags: + - { name: phpbb.ads.analyser.test } + diff --git a/language/en/acp.php b/language/en/acp.php index fe62907c..eee7a8ff 100644 --- a/language/en/acp.php +++ b/language/en/acp.php @@ -104,7 +104,8 @@ 'MARKETING_CONSENT_RECOMMENDED' => 'Require marketing consent
This advertisement contains executable <script> tags. If this ad loads marketing, tracking, cookies, or other consent-controlled resources, enable Require marketing consent below for this ad so its scripts are deferred until the visitor allows marketing in Privacy Settings.', 'MARKETING_CONSENT_VENDOR_RECOMMENDED' => 'Known ad vendor detected
This advertisement contains executable <script> tags from a known advertising or marketing vendor. Enable Require marketing consent below for this ad so its scripts are deferred until the visitor allows marketing in Privacy Settings.', 'ALERT_USAGE' => 'Usage of alert()
Your code uses the alert() function which is not a good practice and can distract users. Some browsers may also block page load and display additional warnings to the user.', - 'LOCATION_CHANGE' => 'Redirection
Your code appears it can redirect a user to another page or site. Redirects can sometimes send users to unintended, often malicious, destinations. Please verify the integrity of your advertisement codeadvertisement codeā€™s redirection destination.', + 'LOCATION_CHANGE' => 'Redirection
Your code appears it can redirect a user to another page or site. Redirects can sometimes send users to unintended, often malicious, destinations. Please verify the integrity of your advertisement codeā€™s redirection destination.', + 'IFRAME_USAGE' => 'Usage of <iframe>
Your code contains HTML-encoded <iframe> tags. Because iframes can introduce third-party tracking or data collection, please review this advertisement snippet to ensure it complies with your user consent policies.', // Template location categories 'CAT_TOP_OF_PAGE' => 'Top of page', diff --git a/tests/analyser/analyser_base.php b/tests/analyser/analyser_base.php index 0d87a494..13cfadf8 100644 --- a/tests/analyser/analyser_base.php +++ b/tests/analyser/analyser_base.php @@ -61,6 +61,7 @@ protected function setUp(): void 'script_without_async', 'marketing_consent', 'untrusted_connection', + 'iframe', ); $analyser_tests = array(); foreach ($tests as $test) @@ -70,7 +71,7 @@ protected function setUp(): void { $analyser_tests['phpbb.ads.analyser.test.' . $test] = new $class($this->request); } - elseif ($test === 'marketing_consent') + else if ($test === 'marketing_consent') { $analyser_tests['phpbb.ads.analyser.test.' . $test] = new $class($this->config); } diff --git a/tests/analyser/run_test.php b/tests/analyser/run_test.php index 168bf85e..a5ec0a10 100644 --- a/tests/analyser/run_test.php +++ b/tests/analyser/run_test.php @@ -95,6 +95,13 @@ public function run_data() 'lang_key' => 'UNSECURE_CONNECTION', ), )), + array('<iframe src="https://some.url" width="640" height="360" allowfullscreen></iframe>', false, array(), array( + array( + 'severity' => 'notice', + 'lang_key' => 'IFRAME_USAGE', + ), + )), + array('<iframe data-consent-src="https://some.url" width="640" height="360" allowfullscreen></iframe>', false, array(), array()), array('', false, array( 'ad_consent' => 0, 'consentmanager_marketing_enabled' => 1, From 0c2e0bf210166322d91f6c657731a6669a06e4a6 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Thu, 4 Jun 2026 15:20:31 -0700 Subject: [PATCH 07/10] Fix language files --- language/en/acp.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/language/en/acp.php b/language/en/acp.php index eee7a8ff..ba8e1359 100644 --- a/language/en/acp.php +++ b/language/en/acp.php @@ -31,7 +31,7 @@ 'AD_NOTE' => 'Notes', 'AD_NOTE_EXPLAIN' => 'Enter any notes for this advertisement. These notes are not shown anywhere except in the ACP and are optional.', 'AD_CODE' => 'Code', - 'AD_CODE_EXPLAIN' => 'Enter the advertisement code here. All code must use HTML markup, BBCodes are not supported.

If the Consent Manager extension is enabled, ads with script tags can be held back until a visitor allows marketing. Normal HTML, such as images and links, will still display, so you usually do not need to change your ad code manually.

Note: If your advertisement code places cookies, collects user data, or tracks user behaviour (for example, ads from Google AdSense or other third-party ad networks), then you should enable the Advertising Disclosure in the Advertisement Management Settings panel to ensure compliance. If you are uncertain, it is recommended that you enable it.', + 'AD_CODE_EXPLAIN' => 'Enter the advertisement code here. All code must use HTML markup, BBCodes are not supported.

Note: If your advertisement code places cookies, collects user data, or tracks user behaviour (for example, ads from Google AdSense or other third-party ad networks), then you should enable the Advertising Disclosure in the Advertisement Management Settings panel to ensure compliance. If you are uncertain, it is recommended that you enable it.', 'ANALYSE_AD_CODE' => 'Analyse advertisement code', 'EVERYTHING_OK' => 'The code appears OK.', 'AD_BANNER' => 'Advertisement banner', @@ -105,7 +105,7 @@ 'MARKETING_CONSENT_VENDOR_RECOMMENDED' => 'Known ad vendor detected
This advertisement contains executable <script> tags from a known advertising or marketing vendor. Enable Require marketing consent below for this ad so its scripts are deferred until the visitor allows marketing in Privacy Settings.', 'ALERT_USAGE' => 'Usage of alert()
Your code uses the alert() function which is not a good practice and can distract users. Some browsers may also block page load and display additional warnings to the user.', 'LOCATION_CHANGE' => 'Redirection
Your code appears it can redirect a user to another page or site. Redirects can sometimes send users to unintended, often malicious, destinations. Please verify the integrity of your advertisement codeā€™s redirection destination.', - 'IFRAME_USAGE' => 'Usage of <iframe>
Your code contains HTML-encoded <iframe> tags. Because iframes can introduce third-party tracking or data collection, please review this advertisement snippet to ensure it complies with your user consent policies.', + 'IFRAME_USAGE' => 'Usage of <iframe>
Your code contains HTML-encoded <iframe> tags. Because iframes can introduce third-party tracking or data collection, please review this advertisement snippet to ensure it complies with your user privacy policies.', // Template location categories 'CAT_TOP_OF_PAGE' => 'Top of page', From cf907fd7ca6bbe56cc15036dccf96a628fb4811e Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Sun, 7 Jun 2026 14:04:29 -0700 Subject: [PATCH 08/10] Let Google Ad codes be handled by Google Consent Mode --- ad/manager.php | 131 ++++++++++++++++++++++++++-- analyser/test/marketing_consent.php | 122 ++++++++++++++++++++++++-- language/en/acp.php | 2 +- tests/ad/prepare_ad_code_test.php | 51 +++++++++++ tests/analyser/run_test.php | 18 ++-- 5 files changed, 297 insertions(+), 27 deletions(-) diff --git a/ad/manager.php b/ad/manager.php index b1638279..a9f63f41 100644 --- a/ad/manager.php +++ b/ad/manager.php @@ -14,6 +14,19 @@ class manager { public const CONSENT_CATEGORY = 'marketing'; + /** + * Google ad/tag scripts that support Google Consent Mode. + * + * These should run immediately so Consent Mode can control storage and + * personalization instead of blocking the ad tag entirely. + */ + protected const GOOGLE_CONSENT_AWARE_SCRIPT_SOURCE_PATTERNS = array( + '~(^|[/.])pagead2\.googlesyndication\.com/pagead/js/adsbygoogle\.js(?:[?#]|$)~i', + '~(^|[/.])securepubads\.g\.doubleclick\.net/tag/js/gpt\.js(?:[?#]|$)~i', + '~(^|[/.])www\.googletagservices\.com/tag/js/gpt\.js(?:[?#]|$)~i', + '~(^|[/.])www\.googletagmanager\.com/(?:gtag/js|gtm\.js)(?:[?#]|$)~i', + ); + /** @var \phpbb\db\driver\driver_interface */ protected $db; @@ -391,12 +404,14 @@ public function prepare_ad_code($ad_code, $consent_enabled) return $ad_code; } - $ad_code = preg_replace_callback('#]*)>(.*?)#is', function ($matches) + $google_consent_aware_sources = $this->get_google_consent_aware_script_sources($ad_code); + + $ad_code = preg_replace_callback('#]*)>(.*?)#is', function ($matches) use ($google_consent_aware_sources) { $attributes = $matches[1] ?? ''; $content = $matches[2] ?? ''; - if (!$this->should_defer_script_tag($attributes)) + if (!$this->should_defer_script_tag($attributes, $content, $google_consent_aware_sources)) { return $matches[0]; } @@ -411,26 +426,128 @@ public function prepare_ad_code($ad_code, $consent_enabled) * Determine whether a script tag is executable and should be deferred. * * @param string $attributes Script tag attributes + * @param string $content Script tag content + * @param array $google_consent_aware_sources Known Google loader sources in this ad block * @return bool */ - protected function should_defer_script_tag($attributes) + protected function should_defer_script_tag($attributes, $content = '', array $google_consent_aware_sources = array()) { if (preg_match('/\bdata-consent-category\s*=/i', $attributes)) { return false; } - if (!preg_match('/\btype\s*=\s*([\'"])(.*?)\1/i', $attributes, $matches)) + if (preg_match('/\btype\s*=\s*([\'"])(.*?)\1/i', $attributes, $matches)) { - return true; + $type = strtolower(trim(explode(';', $matches[2])[0])); + } + else + { + $type = ''; } - $type = strtolower(trim(explode(';', $matches[2])[0])); - return $type === '' + $is_executable = $type === '' || $type === 'text/plain' || $type === 'module' || strpos($type, 'javascript') !== false || strpos($type, 'ecmascript') !== false; + + if (!$is_executable) + { + return false; + } + + return !$this->is_google_consent_aware_script($attributes, $content, $google_consent_aware_sources); + } + + /** + * Determine whether a script should run under Google Consent Mode. + * + * @param string $attributes Script tag attributes + * @param string $content Script tag content + * @param array $google_consent_aware_sources Known Google loader sources in this ad block + * @return bool + */ + protected function is_google_consent_aware_script($attributes, $content, array $google_consent_aware_sources) + { + $source = $this->extract_script_source($attributes); + if ($source !== '') + { + return isset($google_consent_aware_sources[$this->normalize_script_source($source)]); + } + + return !empty($google_consent_aware_sources) + && preg_match('/\b(?:adsbygoogle|googletag|gtag|dataLayer)\b/', $content); + } + + /** + * Return known Google Consent Mode-aware loader sources in an ad block. + * + * @param string $ad_code Advertisement code + * @return array + */ + protected function get_google_consent_aware_script_sources($ad_code) + { + $sources = array(); + + if (!preg_match_all('#]*)>#is', $ad_code, $matches)) + { + return $sources; + } + + foreach ($matches[1] as $attributes) + { + $source = $this->extract_script_source($attributes); + if ($source !== '' && $this->is_google_consent_aware_script_source($source)) + { + $sources[$this->normalize_script_source($source)] = true; + } + } + + return $sources; + } + + /** + * Extract the src attribute from a script tag attribute string. + * + * @param string $attributes Script tag attributes + * @return string + */ + protected function extract_script_source($attributes) + { + return preg_match('/\bsrc\s*=\s*([\'"])(.*?)\1/i', $attributes, $matches) ? $matches[2] : ''; + } + + /** + * Check whether a script source is a known Google Consent Mode-aware loader. + * + * @param string $source Script source URL + * @return bool + */ + protected function is_google_consent_aware_script_source($source) + { + $source = $this->normalize_script_source($source); + + foreach (self::GOOGLE_CONSENT_AWARE_SCRIPT_SOURCE_PATTERNS as $pattern) + { + if (preg_match($pattern, $source)) + { + return true; + } + } + + return false; + } + + /** + * Normalize a script source before comparing against allowlisted loaders. + * + * @param string $source Script source URL + * @return string + */ + protected function normalize_script_source($source) + { + return preg_replace('#^//#', 'https://', trim($source)); } /** diff --git a/analyser/test/marketing_consent.php b/analyser/test/marketing_consent.php index e055a9f2..944e48b4 100644 --- a/analyser/test/marketing_consent.php +++ b/analyser/test/marketing_consent.php @@ -19,12 +19,8 @@ class marketing_consent implements test_interface * host hints should not expand warnings to iframe-only or image-only embeds. */ protected const MARKETING_HOST_PATTERNS = array( - '/(^|[\/.])pagead2\.googlesyndication\.com(?=[:\/]|$)/i', '/(^|[\/.])partner\.googleadservices\.com(?=[:\/]|$)/i', '/(^|[\/.])googleads\.g\.doubleclick\.net(?=[:\/]|$)/i', - '/(^|[\/.])securepubads\.g\.doubleclick\.net(?=[:\/]|$)/i', - '/(^|[\/.])www\.googletagservices\.com(?=[:\/]|$)/i', - '/(^|[\/.])www\.googletagmanager\.com(?=[:\/]|$)/i', '/(^|[\/.])c\.amazon-adsystem\.com(?=[:\/]|$)/i', '/(^|[\/.])aax\.amazon-adsystem\.com(?=[:\/]|$)/i', '/(^|[\/.])trc\.taboola\.com(?=[:\/]|$)/i', @@ -37,6 +33,20 @@ class marketing_consent implements test_interface '/(^|[\/.])ib\.adnxs\.com(?=[:\/]|$)/i', ); + /** + * Google ad/tag scripts that support Google Consent Mode. + * + * Ads extension does not recommend script deferral for these tags because + * Consent Manager communicates the marketing consent state through Google + * Consent Mode instead. + */ + protected const GOOGLE_CONSENT_AWARE_SCRIPT_SOURCE_PATTERNS = array( + '~(^|[/.])pagead2\.googlesyndication\.com/pagead/js/adsbygoogle\.js(?:[?#]|$)~i', + '~(^|[/.])securepubads\.g\.doubleclick\.net/tag/js/gpt\.js(?:[?#]|$)~i', + '~(^|[/.])www\.googletagservices\.com/tag/js/gpt\.js(?:[?#]|$)~i', + '~(^|[/.])www\.googletagmanager\.com/(?:gtag/js|gtm\.js)(?:[?#]|$)~i', + ); + /** @var \phpbb\config\config */ protected $config; @@ -91,14 +101,21 @@ protected function get_recommendation_message($ad_code) return false; } + $google_consent_aware_sources = $this->get_google_consent_aware_script_sources($ad_code); + foreach ($matches[1] as $index => $attributes) { + $content = $matches[2][$index] ?? ''; if (!$this->should_flag_script_tag($attributes)) { continue; } - $content = $matches[2][$index] ?? ''; + if ($this->is_google_consent_aware_script($attributes, $content, $google_consent_aware_sources)) + { + continue; + } + if ($this->contains_marketing_host_hint($attributes, $content)) { return 'MARKETING_CONSENT_VENDOR_RECOMMENDED'; @@ -110,6 +127,96 @@ protected function get_recommendation_message($ad_code) return false; } + /** + * Determine whether a script should run under Google Consent Mode. + * + * @param string $attributes Script tag attributes + * @param string $content Script tag content + * @param array $google_consent_aware_sources Known Google loader sources in this ad block + * @return bool + */ + protected function is_google_consent_aware_script($attributes, $content, array $google_consent_aware_sources) + { + $source = $this->extract_script_source($attributes); + if ($source !== '') + { + return isset($google_consent_aware_sources[$this->normalize_script_source($source)]); + } + + return !empty($google_consent_aware_sources) + && preg_match('/\b(?:adsbygoogle|googletag|gtag|dataLayer)\b/', $content); + } + + /** + * Return known Google Consent Mode-aware loader sources in an ad block. + * + * @param string $ad_code Advertisement code + * @return array + */ + protected function get_google_consent_aware_script_sources($ad_code) + { + $sources = array(); + + if (!preg_match_all('#]*)>#is', $ad_code, $matches)) + { + return $sources; + } + + foreach ($matches[1] as $attributes) + { + $source = $this->extract_script_source($attributes); + if ($source !== '' && $this->is_google_consent_aware_script_source($source)) + { + $sources[$this->normalize_script_source($source)] = true; + } + } + + return $sources; + } + + /** + * Extract the src attribute from a script tag attribute string. + * + * @param string $attributes Script tag attributes + * @return string + */ + protected function extract_script_source($attributes) + { + return preg_match('/\bsrc\s*=\s*([\'"])(.*?)\1/i', $attributes, $matches) ? $matches[2] : ''; + } + + /** + * Check whether a script source is a known Google Consent Mode-aware loader. + * + * @param string $source Script source URL + * @return bool + */ + protected function is_google_consent_aware_script_source($source) + { + $source = $this->normalize_script_source($source); + + foreach (self::GOOGLE_CONSENT_AWARE_SCRIPT_SOURCE_PATTERNS as $pattern) + { + if (preg_match($pattern, $source)) + { + return true; + } + } + + return false; + } + + /** + * Normalize a script source before comparing against allowlisted loaders. + * + * @param string $source Script source URL + * @return string + */ + protected function normalize_script_source($source) + { + return preg_replace('#^//#', 'https://', trim($source)); + } + /** * Check for known advertising vendor hints inside script markup or content. * @@ -121,9 +228,10 @@ protected function contains_marketing_host_hint($attributes, $content) { $haystacks = array($attributes, $content); - if (preg_match('/\bsrc\s*=\s*([\'"])(.*?)\1/i', $attributes, $matches)) + $source = $this->extract_script_source($attributes); + if ($source !== '') { - $haystacks[] = $matches[2]; + $haystacks[] = $source; } foreach ($haystacks as $haystack) diff --git a/language/en/acp.php b/language/en/acp.php index ba8e1359..55ceea8d 100644 --- a/language/en/acp.php +++ b/language/en/acp.php @@ -57,7 +57,7 @@ 'AD_CLICKS_LIMIT' => 'Clicks Limit', 'AD_CLICKS_LIMIT_EXPLAIN' => 'Set the maximum number of times the advertisement will be clicked, after which the advertisement will no longer be displayed. Set 0 for unlimited clicks.', 'AD_CONSENT' => 'Require marketing consent', - 'AD_CONSENT_EXPLAIN' => 'Set to yes to defer script tags in this advertisement until the visitor allows marketing in Privacy Settings. Set to no only for ad code that does not load marketing, tracking, cookies, or other consent-controlled resources.', + 'AD_CONSENT_EXPLAIN' => 'Set to Yes to defer script tags in this advertisement until the visitor grants marketing consent in Privacy Settings. Set to No only for ad code that does not load marketing, tracking, cookies, profiling, or other consent-controlled resources.

Note: This setting has no effect on supported Google AdSense or Google Publisher Tag (GPT) code. Consent Manager automatically manages consent for Google Ads through Google Consent Mode.', 'AD_START_DATE' => 'Start Date', 'AD_START_DATE_EXPLAIN' => 'Set the date when the advertisement can begin displaying (starting at 00:00). The ad must still be manually enabled to appear. If no date is set, the ad can display immediately once enabled.', 'AD_END_DATE' => 'End Date', diff --git a/tests/ad/prepare_ad_code_test.php b/tests/ad/prepare_ad_code_test.php index 20d217d3..15b5f34c 100644 --- a/tests/ad/prepare_ad_code_test.php +++ b/tests/ad/prepare_ad_code_test.php @@ -86,6 +86,57 @@ public function test_does_not_double_wrap_already_tagged_script() self::assertSame(1, substr_count($result, 'data-consent-category=')); } + public function google_consent_aware_script_data() + { + return [ + 'adsense loader' => [ + '', + ], + 'gpt loader' => [ + '', + ], + 'gtag loader' => [ + '', + ], + 'gtm loader' => [ + '', + ], + ]; + } + + /** + * @dataProvider google_consent_aware_script_data + */ + public function test_does_not_defer_google_consent_aware_loaders($script) + { + $raw = htmlspecialchars($script, ENT_COMPAT); + $result = $this->get_manager()->prepare_ad_code($raw, true); + self::assertSame($script, $result); + } + + public function test_does_not_defer_adsense_inline_script_when_adsense_loader_is_present() + { + $script = ''; + $raw = htmlspecialchars($script, ENT_COMPAT); + $result = $this->get_manager()->prepare_ad_code($raw, true); + self::assertSame($script, $result); + } + + public function test_does_not_defer_gpt_inline_script_when_gpt_loader_is_present() + { + $script = ''; + $raw = htmlspecialchars($script, ENT_COMPAT); + $result = $this->get_manager()->prepare_ad_code($raw, true); + self::assertSame($script, $result); + } + + public function test_defers_google_named_inline_script_without_google_loader() + { + $raw = htmlspecialchars('', ENT_COMPAT); + $result = $this->get_manager()->prepare_ad_code($raw, true); + self::assertSame('', $result); + } + public function test_non_script_html_is_preserved() { $raw = htmlspecialchars('
Ad
', ENT_COMPAT); diff --git a/tests/analyser/run_test.php b/tests/analyser/run_test.php index a5ec0a10..2b35e17e 100644 --- a/tests/analyser/run_test.php +++ b/tests/analyser/run_test.php @@ -123,21 +123,15 @@ public function run_data() array('', false, array( 'ad_consent' => 0, 'consentmanager_marketing_enabled' => 1, - ), array( - array( - 'severity' => 'notice', - 'lang_key' => 'MARKETING_CONSENT_VENDOR_RECOMMENDED', - ), - )), + ), array()), + array('', false, array( + 'ad_consent' => 0, + 'consentmanager_marketing_enabled' => 1, + ), array()), array('', false, array( 'ad_consent' => 0, 'consentmanager_marketing_enabled' => 1, - ), array( - array( - 'severity' => 'notice', - 'lang_key' => 'MARKETING_CONSENT_VENDOR_RECOMMENDED', - ), - )), + ), array()), array('', false, array( 'ad_consent' => 0, 'consentmanager_marketing_enabled' => 1, From 8f2dd12b6798ee31326eb797211a9517ba9b4f59 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Mon, 8 Jun 2026 07:16:34 -0700 Subject: [PATCH 09/10] Update tests --- tests/analyser/run_test.php | 70 ++++++++++++++++++++++++------------- 1 file changed, 46 insertions(+), 24 deletions(-) diff --git a/tests/analyser/run_test.php b/tests/analyser/run_test.php index 2b35e17e..76bd4c56 100644 --- a/tests/analyser/run_test.php +++ b/tests/analyser/run_test.php @@ -20,64 +20,64 @@ class run_test extends analyser_base public function run_data() { return array( - array('<script async>alert()</script>', false, array(), array( + 'warns on alert call' => array('<script async>alert()</script>', false, array(), array( array( 'severity' => 'warning', 'lang_key' => 'ALERT_USAGE', ), )), - array('<script async>alert ()</script>', false, array(), array( + 'warns on spaced alert call' => array('<script async>alert ()</script>', false, array(), array( array( 'severity' => 'warning', 'lang_key' => 'ALERT_USAGE', ), )), - array('<script async>window.location.href = "new url"</script>', false, array(), array( + 'warns on location href assignment' => array('<script async>window.location.href = "new url"</script>', false, array(), array( array( 'severity' => 'warning', 'lang_key' => 'LOCATION_CHANGE', ), )), - array('<script async>window.location.href= "new url"</script>', false, array(), array( + 'warns on compact location href assignment' => array('<script async>window.location.href= "new url"</script>', false, array(), array( array( 'severity' => 'warning', 'lang_key' => 'LOCATION_CHANGE', ), )), - array('<script></script>', false, array(), array()), - array('<script src="script src"></script>', false, array(), array( + 'allows empty script without src' => array('<script></script>', false, array(), array()), + 'notices script without async' => array('<script src="script src"></script>', false, array(), array( array( 'severity' => 'notice', 'lang_key' => 'SCRIPT_WITHOUT_ASYNC', ), )), - array('<script src="script src"></script><script src="another script src"></script>', false, array(), array( + 'notices first of multiple scripts without async' => array('<script src="script src"></script><script src="another script src"></script>', false, array(), array( array( 'severity' => 'notice', 'lang_key' => 'SCRIPT_WITHOUT_ASYNC', ), )), - array('<script async src="script src"></script><script src="another script src"></script>', false, array(), array( + 'notices second script without async' => array('<script async src="script src"></script><script src="another script src"></script>', false, array(), array( array( 'severity' => 'notice', 'lang_key' => 'SCRIPT_WITHOUT_ASYNC', ), )), - array('<script src="script src"></script><script async src="another script src"></script>', false, array(), array( + 'notices first script without async before async script' => array('<script src="script src"></script><script async src="another script src"></script>', false, array(), array( array( 'severity' => 'notice', 'lang_key' => 'SCRIPT_WITHOUT_ASYNC', ), )), - array('<script async src="http://some.url"></script>', false, array(), array()), - array('<script async src="https://some.url"></script>', true, array(), array()), - array('<script async src="http://some.url"></script>', true, array(), array( + 'allows http script on http page' => array('<script async src="http://some.url"></script>', false, array(), array()), + 'allows https script on https page' => array('<script async src="https://some.url"></script>', true, array(), array()), + 'warns on http script on https page' => array('<script async src="http://some.url"></script>', true, array(), array( array( 'severity' => 'warning', 'lang_key' => 'UNSECURE_CONNECTION', ), )), - array('<script src="http://some.url"></script><script>alert("e");window.location.href="new url"</script>', true, array(), array( + 'collects multiple analyser warnings' => array('<script src="http://some.url"></script><script>alert("e");window.location.href="new url"</script>', true, array(), array( array( 'severity' => 'warning', 'lang_key' => 'ALERT_USAGE', @@ -95,14 +95,14 @@ public function run_data() 'lang_key' => 'UNSECURE_CONNECTION', ), )), - array('<iframe src="https://some.url" width="640" height="360" allowfullscreen></iframe>', false, array(), array( + 'notices iframe usage' => array('<iframe src="https://some.url" width="640" height="360" allowfullscreen></iframe>', false, array(), array( array( 'severity' => 'notice', 'lang_key' => 'IFRAME_USAGE', ), )), - array('<iframe data-consent-src="https://some.url" width="640" height="360" allowfullscreen></iframe>', false, array(), array()), - array('', false, array( + 'allows consent-aware iframe placeholder' => array('<iframe data-consent-src="https://some.url" width="640" height="360" allowfullscreen></iframe>', false, array(), array()), + 'recommends marketing consent for generic ad script' => array('', false, array( 'ad_consent' => 0, 'consentmanager_marketing_enabled' => 1, ), array( @@ -111,7 +111,7 @@ public function run_data() 'lang_key' => 'MARKETING_CONSENT_RECOMMENDED', ), )), - array('', false, array( + 'recommends marketing consent for inline cookie script' => array('', false, array( 'ad_consent' => 0, 'consentmanager_marketing_enabled' => 1, ), array( @@ -120,34 +120,47 @@ public function run_data() 'lang_key' => 'MARKETING_CONSENT_RECOMMENDED', ), )), - array('', false, array( + 'recommends marketing consent for known non-Google vendor script' => array('', false, array( + 'ad_consent' => 0, + 'consentmanager_marketing_enabled' => 1, + ), array( + array( + 'severity' => 'notice', + 'lang_key' => 'MARKETING_CONSENT_VENDOR_RECOMMENDED', + ), + )), + 'allows AdSense loader under Google Consent Mode' => array('', false, array( 'ad_consent' => 0, 'consentmanager_marketing_enabled' => 1, ), array()), - array('', false, array( + 'allows full AdSense snippet under Google Consent Mode' => array('', false, array( 'ad_consent' => 0, 'consentmanager_marketing_enabled' => 1, ), array()), - array('', false, array( + 'allows GPT loader under Google Consent Mode' => array('', false, array( 'ad_consent' => 0, 'consentmanager_marketing_enabled' => 1, ), array()), - array('', false, array( + 'allows non-executable json script' => array('', false, array( 'ad_consent' => 0, 'consentmanager_marketing_enabled' => 1, ), array()), - array('', false, array( + 'allows Google ad iframe because marketing consent analyser only handles scripts' => array('', false, array( 'ad_consent' => 0, 'consentmanager_marketing_enabled' => 1, ), array()), - array('', false, array( + 'allows generic ad script when ad consent is already enabled' => array('', false, array( 'ad_consent' => 1, 'consentmanager_marketing_enabled' => 1, ), array()), - array('', false, array( + 'allows generic ad script when Consent Manager marketing category is disabled' => array('', false, array( 'ad_consent' => 0, 'consentmanager_marketing_enabled' => 0, ), array()), + 'allows already consent-tagged script' => array('', false, array( + 'ad_consent' => 0, + 'consentmanager_marketing_enabled' => 1, + ), array()), ); } @@ -188,4 +201,13 @@ public function test_run($ad_code, $is_https, $context, $expected) $manager->run($ad_code, $context); } + + public function test_google_consent_aware_source_lookup_returns_empty_without_script_tags() + { + $test = new \phpbb\ads\analyser\test\marketing_consent($this->config); + $method = new \ReflectionMethod($test, 'get_google_consent_aware_script_sources'); + $method->setAccessible(true); + + self::assertSame(array(), $method->invoke($test, '
No scripts
')); + } } From 4e9995e77caf2638fa956c4c4e7370723b763497 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Mon, 8 Jun 2026 09:41:41 -0700 Subject: [PATCH 10/10] Remove code duplication --- ad/manager.php | 26 +++---- analyser/test/marketing_consent.php | 110 +--------------------------- tests/ad/prepare_ad_code_test.php | 5 ++ tests/analyser/run_test.php | 9 --- 4 files changed, 21 insertions(+), 129 deletions(-) diff --git a/ad/manager.php b/ad/manager.php index a9f63f41..1aea3215 100644 --- a/ad/manager.php +++ b/ad/manager.php @@ -404,7 +404,7 @@ public function prepare_ad_code($ad_code, $consent_enabled) return $ad_code; } - $google_consent_aware_sources = $this->get_google_consent_aware_script_sources($ad_code); + $google_consent_aware_sources = self::get_google_consent_aware_script_sources($ad_code); $ad_code = preg_replace_callback('#]*)>(.*?)#is', function ($matches) use ($google_consent_aware_sources) { @@ -457,7 +457,7 @@ protected function should_defer_script_tag($attributes, $content = '', array $go return false; } - return !$this->is_google_consent_aware_script($attributes, $content, $google_consent_aware_sources); + return !self::is_google_consent_aware_script($attributes, $content, $google_consent_aware_sources); } /** @@ -468,12 +468,12 @@ protected function should_defer_script_tag($attributes, $content = '', array $go * @param array $google_consent_aware_sources Known Google loader sources in this ad block * @return bool */ - protected function is_google_consent_aware_script($attributes, $content, array $google_consent_aware_sources) + public static function is_google_consent_aware_script($attributes, $content, array $google_consent_aware_sources) { - $source = $this->extract_script_source($attributes); + $source = self::extract_script_source($attributes); if ($source !== '') { - return isset($google_consent_aware_sources[$this->normalize_script_source($source)]); + return isset($google_consent_aware_sources[self::normalize_script_source($source)]); } return !empty($google_consent_aware_sources) @@ -486,7 +486,7 @@ protected function is_google_consent_aware_script($attributes, $content, array $ * @param string $ad_code Advertisement code * @return array */ - protected function get_google_consent_aware_script_sources($ad_code) + public static function get_google_consent_aware_script_sources($ad_code) { $sources = array(); @@ -497,10 +497,10 @@ protected function get_google_consent_aware_script_sources($ad_code) foreach ($matches[1] as $attributes) { - $source = $this->extract_script_source($attributes); - if ($source !== '' && $this->is_google_consent_aware_script_source($source)) + $source = self::extract_script_source($attributes); + if ($source !== '' && self::is_google_consent_aware_script_source($source)) { - $sources[$this->normalize_script_source($source)] = true; + $sources[self::normalize_script_source($source)] = true; } } @@ -513,7 +513,7 @@ protected function get_google_consent_aware_script_sources($ad_code) * @param string $attributes Script tag attributes * @return string */ - protected function extract_script_source($attributes) + public static function extract_script_source($attributes) { return preg_match('/\bsrc\s*=\s*([\'"])(.*?)\1/i', $attributes, $matches) ? $matches[2] : ''; } @@ -524,9 +524,9 @@ protected function extract_script_source($attributes) * @param string $source Script source URL * @return bool */ - protected function is_google_consent_aware_script_source($source) + protected static function is_google_consent_aware_script_source($source) { - $source = $this->normalize_script_source($source); + $source = self::normalize_script_source($source); foreach (self::GOOGLE_CONSENT_AWARE_SCRIPT_SOURCE_PATTERNS as $pattern) { @@ -545,7 +545,7 @@ protected function is_google_consent_aware_script_source($source) * @param string $source Script source URL * @return string */ - protected function normalize_script_source($source) + protected static function normalize_script_source($source) { return preg_replace('#^//#', 'https://', trim($source)); } diff --git a/analyser/test/marketing_consent.php b/analyser/test/marketing_consent.php index 944e48b4..62832291 100644 --- a/analyser/test/marketing_consent.php +++ b/analyser/test/marketing_consent.php @@ -33,20 +33,6 @@ class marketing_consent implements test_interface '/(^|[\/.])ib\.adnxs\.com(?=[:\/]|$)/i', ); - /** - * Google ad/tag scripts that support Google Consent Mode. - * - * Ads extension does not recommend script deferral for these tags because - * Consent Manager communicates the marketing consent state through Google - * Consent Mode instead. - */ - protected const GOOGLE_CONSENT_AWARE_SCRIPT_SOURCE_PATTERNS = array( - '~(^|[/.])pagead2\.googlesyndication\.com/pagead/js/adsbygoogle\.js(?:[?#]|$)~i', - '~(^|[/.])securepubads\.g\.doubleclick\.net/tag/js/gpt\.js(?:[?#]|$)~i', - '~(^|[/.])www\.googletagservices\.com/tag/js/gpt\.js(?:[?#]|$)~i', - '~(^|[/.])www\.googletagmanager\.com/(?:gtag/js|gtm\.js)(?:[?#]|$)~i', - ); - /** @var \phpbb\config\config */ protected $config; @@ -101,7 +87,7 @@ protected function get_recommendation_message($ad_code) return false; } - $google_consent_aware_sources = $this->get_google_consent_aware_script_sources($ad_code); + $google_consent_aware_sources = \phpbb\ads\ad\manager::get_google_consent_aware_script_sources($ad_code); foreach ($matches[1] as $index => $attributes) { @@ -111,7 +97,7 @@ protected function get_recommendation_message($ad_code) continue; } - if ($this->is_google_consent_aware_script($attributes, $content, $google_consent_aware_sources)) + if (\phpbb\ads\ad\manager::is_google_consent_aware_script($attributes, $content, $google_consent_aware_sources)) { continue; } @@ -127,96 +113,6 @@ protected function get_recommendation_message($ad_code) return false; } - /** - * Determine whether a script should run under Google Consent Mode. - * - * @param string $attributes Script tag attributes - * @param string $content Script tag content - * @param array $google_consent_aware_sources Known Google loader sources in this ad block - * @return bool - */ - protected function is_google_consent_aware_script($attributes, $content, array $google_consent_aware_sources) - { - $source = $this->extract_script_source($attributes); - if ($source !== '') - { - return isset($google_consent_aware_sources[$this->normalize_script_source($source)]); - } - - return !empty($google_consent_aware_sources) - && preg_match('/\b(?:adsbygoogle|googletag|gtag|dataLayer)\b/', $content); - } - - /** - * Return known Google Consent Mode-aware loader sources in an ad block. - * - * @param string $ad_code Advertisement code - * @return array - */ - protected function get_google_consent_aware_script_sources($ad_code) - { - $sources = array(); - - if (!preg_match_all('#]*)>#is', $ad_code, $matches)) - { - return $sources; - } - - foreach ($matches[1] as $attributes) - { - $source = $this->extract_script_source($attributes); - if ($source !== '' && $this->is_google_consent_aware_script_source($source)) - { - $sources[$this->normalize_script_source($source)] = true; - } - } - - return $sources; - } - - /** - * Extract the src attribute from a script tag attribute string. - * - * @param string $attributes Script tag attributes - * @return string - */ - protected function extract_script_source($attributes) - { - return preg_match('/\bsrc\s*=\s*([\'"])(.*?)\1/i', $attributes, $matches) ? $matches[2] : ''; - } - - /** - * Check whether a script source is a known Google Consent Mode-aware loader. - * - * @param string $source Script source URL - * @return bool - */ - protected function is_google_consent_aware_script_source($source) - { - $source = $this->normalize_script_source($source); - - foreach (self::GOOGLE_CONSENT_AWARE_SCRIPT_SOURCE_PATTERNS as $pattern) - { - if (preg_match($pattern, $source)) - { - return true; - } - } - - return false; - } - - /** - * Normalize a script source before comparing against allowlisted loaders. - * - * @param string $source Script source URL - * @return string - */ - protected function normalize_script_source($source) - { - return preg_replace('#^//#', 'https://', trim($source)); - } - /** * Check for known advertising vendor hints inside script markup or content. * @@ -228,7 +124,7 @@ protected function contains_marketing_host_hint($attributes, $content) { $haystacks = array($attributes, $content); - $source = $this->extract_script_source($attributes); + $source = \phpbb\ads\ad\manager::extract_script_source($attributes); if ($source !== '') { $haystacks[] = $source; diff --git a/tests/ad/prepare_ad_code_test.php b/tests/ad/prepare_ad_code_test.php index 15b5f34c..f49864c3 100644 --- a/tests/ad/prepare_ad_code_test.php +++ b/tests/ad/prepare_ad_code_test.php @@ -137,6 +137,11 @@ public function test_defers_google_named_inline_script_without_google_loader() self::assertSame('', $result); } + public function test_google_consent_aware_source_lookup_returns_empty_without_script_tags() + { + self::assertSame(array(), \phpbb\ads\ad\manager::get_google_consent_aware_script_sources('
No scripts
')); + } + public function test_non_script_html_is_preserved() { $raw = htmlspecialchars('
Ad
', ENT_COMPAT); diff --git a/tests/analyser/run_test.php b/tests/analyser/run_test.php index 76bd4c56..84a27e64 100644 --- a/tests/analyser/run_test.php +++ b/tests/analyser/run_test.php @@ -201,13 +201,4 @@ public function test_run($ad_code, $is_https, $context, $expected) $manager->run($ad_code, $context); } - - public function test_google_consent_aware_source_lookup_returns_empty_without_script_tags() - { - $test = new \phpbb\ads\analyser\test\marketing_consent($this->config); - $method = new \ReflectionMethod($test, 'get_google_consent_aware_script_sources'); - $method->setAccessible(true); - - self::assertSame(array(), $method->invoke($test, '
No scripts
')); - } }