Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions config/services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ services:
- '@user'
- '%tables.wiki.article%'
- '@phpbbmodders.wiki.files'
- '@request'
phpbbmodders.wiki.view:
class: phpbbmodders\wiki\wiki\view
arguments:
Expand Down
53 changes: 53 additions & 0 deletions tests/unit/confirm_article_test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?php
/**
*
* Wiki extension for the phpBB Forum Software package
*
* @copyright (c) 2026, phpBB Modders, https://www.phpbbmodders.com/
* @license GNU General Public License, version 2 (GPL-2.0)
*
*/

namespace phpbbmodders\wiki\tests\unit;

use phpbbmodders\wiki\wiki\functions;

/**
* The article name used once a confirm box has been answered.
*/
class confirm_article_test extends \PHPUnit\Framework\TestCase
{
private function request(?string $posted): \phpbb\request\request_interface
{
$request = $this->createMock(\phpbb\request\request_interface::class);
$request->method('variable')->willReturnCallback(function ($name, $default, $multibyte) use ($posted) {
$this->assertSame('article', $name);
$this->assertTrue($multibyte, 'names can hold any character');

return $posted ?? $default;
});

return $request;
}

public function test_the_name_the_box_carried_wins_over_the_double_encoded_route_name()
{
$this->assertSame('Some Article', functions::confirmed_article($this->request('Some Article'), 'Some%20Article'));
}

public function test_before_the_box_the_route_name_is_used()
{
$this->assertSame('Some Article', functions::confirmed_article($this->request(null), 'Some Article'));
}

public function test_the_html_escaping_of_the_posted_value_is_undone()
{
// request->variable() escapes what it returns; article names are stored as typed
$this->assertSame('Tom & "Jerry"', functions::confirmed_article($this->request('Tom &amp; &quot;Jerry&quot;'), 'x'));
}

public function test_unicode_names_survive()
{
$this->assertSame('Übersicht ✓', functions::confirmed_article($this->request('Übersicht ✓'), 'x'));
}
}
9 changes: 8 additions & 1 deletion wiki/delete.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ class delete extends \phpbbmodders\wiki\wiki\functions
/** @var \phpbb\controller\helper */
protected $helper;

/** @var \phpbb\request\request */
protected $request;

/** @var \parse_message */
protected $message_parser;

Expand All @@ -44,10 +47,12 @@ class delete extends \phpbbmodders\wiki\wiki\functions
* @param \phpbb\user $user User object
* @param string $article_table
* @param \phpbbmodders\wiki\wiki\files $files Uploaded files
* @param \phpbb\request\request $request Request object
*/
public function __construct(\phpbb\auth\auth $auth, \phpbb\db\driver\driver_interface $db, \phpbb\controller\helper $helper, \phpbb\user $user, $article_table, \phpbbmodders\wiki\wiki\files $files)
public function __construct(\phpbb\auth\auth $auth, \phpbb\db\driver\driver_interface $db, \phpbb\controller\helper $helper, \phpbb\user $user, $article_table, \phpbbmodders\wiki\wiki\files $files, \phpbb\request\request $request)
{
$this->files = $files;
$this->request = $request;
$this->auth = $auth;
$this->db = $db;
$this->helper = $helper;
Expand Down Expand Up @@ -112,6 +117,8 @@ public function article($article)

if (confirm_box(true))
{
$article = self::confirmed_article($this->request, $article);

$sql = 'DELETE FROM ' . $this->article_table . "
WHERE article_url = '" . $this->db->sql_escape($article) . "'";
$this->db->sql_query($sql);
Expand Down
2 changes: 2 additions & 0 deletions wiki/edit.php
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ public function deactivate($article)

if (confirm_box(true))
{
$article = self::confirmed_article($this->request, $article);

// Set all versions to not approved
$sql = 'UPDATE ' . $this->article_table . "
SET article_approved = 0
Expand Down
9 changes: 8 additions & 1 deletion wiki/files.php
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,13 @@ public function serve($article_url, $file_id)
*/
public function delete($article_url, $file_id)
{
// confirm_box(true) can only be asked once per request
$confirmed = confirm_box(true);
if ($confirmed)
{
$article_url = functions::confirmed_article($this->request, $article_url);
}

$file = $this->get_file($article_url, $file_id);
if (!$file)
{
Expand All @@ -504,7 +511,7 @@ public function delete($article_url, $file_id)

$back = $this->helper->route('phpbbmodders_wiki_article', array('article' => $article_url));

if (confirm_box(true))
if ($confirmed)
{
$this->remove_files(array($file));
trigger_error($this->user->lang('WIKI_FILE_DELETED', $file['file_name']) . '<br /><br /><a href="' . $back . '">' . $this->user->lang('BACK_TO_ARTICLE') . '</a>');
Expand Down
18 changes: 18 additions & 0 deletions wiki/functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,24 @@ protected function setup_parser()
}
}

/**
* The article name to act on once a confirm box has been answered.
*
* confirm_box() posts back to a page address whose path is encoded a second
* time, so "Some Article" comes back from the route as "Some%20Article" and
* matches nothing. The name the box carried in a hidden field is the real one;
* on the first request, before the box, there is no such field and the route's
* name is used.
*
* @param \phpbb\request\request_interface $request
* @param string $article Name from the route
* @return string
*/
public static function confirmed_article(\phpbb\request\request_interface $request, $article)
{
return htmlspecialchars_decode($request->variable('article', $article, true), ENT_COMPAT);
}

/**
*
* @param int $id Version ID
Expand Down
Loading