[COMPONENT][Posting] Fix request handling issues that resulted from splitting creation and controller

This commit is contained in:
Diogo Peralta Cordeiro 2022-03-11 03:14:47 +00:00
parent 68ee996ec4
commit 9f49fbacc9
Signed by: diogo
GPG Key ID: 18D2D35001FBFAB0
4 changed files with 65 additions and 40 deletions

View File

@ -137,28 +137,6 @@ class Conversation extends Component
return Event::next; return Event::next;
} }
/**
* Posting event to add extra information to Component\Posting form data
*
* @param array $data Transport data to be filled with reply_to_id
*
* @throws \App\Util\Exception\ClientException
* @throws \App\Util\Exception\NoSuchNoteException
*
* @return bool EventHook
*/
public function onPostingModifyData(Request $request, Actor $actor, array &$data): bool
{
$data['reply_to_id'] = $request->get('_route') === 'conversation_reply_to' && $request->query->has('reply_to_id')
? $request->query->getInt('reply_to_id')
: null;
if (!\is_null($data['reply_to_id'])) {
Note::ensureCanInteract(Note::getById($data['reply_to_id']), $actor);
}
return Event::next;
}
/** /**
* Append on note information about user actions. * Append on note information about user actions.
* *
@ -194,6 +172,22 @@ class Conversation extends Component
return Event::next; return Event::next;
} }
private function getReplyToIdFromRequest(Request $request): ?int
{
if (!\is_array($request->get('post_note')) || !\array_key_exists('_next', $request->get('post_note'))) {
return null;
}
$next = parse_url($request->get('post_note')['_next']);
if (!\array_key_exists('query', $next)) {
return null;
}
parse_str($next['query'], $query);
if (!\array_key_exists('reply_to_id', $query)) {
return null;
}
return (int) $query['reply_to_id'];
}
/** /**
* Informs **\App\Component\Posting::onAppendRightPostingBlock**, of the **current page context** in which the given * Informs **\App\Component\Posting::onAppendRightPostingBlock**, of the **current page context** in which the given
* Actor is in. This is valuable when posting within a group route, allowing \App\Component\Posting to create a * Actor is in. This is valuable when posting within a group route, allowing \App\Component\Posting to create a
@ -206,7 +200,7 @@ class Conversation extends Component
*/ */
public function onPostingGetContextActor(Request $request, Actor $actor, ?Actor &$context_actor): bool public function onPostingGetContextActor(Request $request, Actor $actor, ?Actor &$context_actor): bool
{ {
$to_note_id = $request->query->get('reply_to_id'); $to_note_id = $this->getReplyToIdFromRequest($request);
if (!\is_null($to_note_id)) { if (!\is_null($to_note_id)) {
// Getting the actor itself // Getting the actor itself
$context_actor = Actor::getById(Note::getById((int) $to_note_id)->getActorId()); $context_actor = Actor::getById(Note::getById((int) $to_note_id)->getActorId());
@ -215,6 +209,27 @@ class Conversation extends Component
return Event::next; return Event::next;
} }
/**
* Posting event to add extra information to Component\Posting form data
*
* @param array $data Transport data to be filled with reply_to_id
*
* @throws \App\Util\Exception\ClientException
* @throws \App\Util\Exception\NoSuchNoteException
*
* @return bool EventHook
*/
public function onPostingModifyData(Request $request, Actor $actor, array &$data): bool
{
$to_note_id = $this->getReplyToIdFromRequest($request);
if (!\is_null($to_note_id)) {
Note::ensureCanInteract(Note::getById($to_note_id), $actor);
$data['reply_to_id'] = $to_note_id;
}
return Event::next;
}
/** /**
* Add minimal Note card to RightPanel template * Add minimal Note card to RightPanel template
*/ */

View File

@ -85,13 +85,21 @@ class Group extends Component
*/ */
private function getGroupFromContext(Request $request): ?Actor private function getGroupFromContext(Request $request): ?Actor
{ {
if (str_starts_with($request->get('_route'), 'group_actor_view_')) { if (\is_array($request->get('post_note')) && \array_key_exists('_next', $request->get('post_note'))) {
if (!\is_null($id = $request->get('id'))) { $next = parse_url($request->get('post_note')['_next']);
return Actor::getById((int) $id); $match = Router::match($next['path']);
} $route = $match['_route'];
$identifier = $match['id'] ?? $match['nickname'] ?? null;
if (!\is_null($nickname = $request->get('nickname'))) { } else {
return LocalGroup::getActorByNickname($nickname); $route = $request->get('_route');
$identifier = $request->get('id') ?? $request->get('nickname');
}
if (str_starts_with($route, 'group_actor_view_')) {
switch ($route) {
case 'group_actor_view_nickname':
return LocalGroup::getActorByNickname($identifier);
case 'group_actor_view_id':
return Actor::getById($identifier);
} }
} }
return null; return null;

View File

@ -53,7 +53,7 @@ class Posting extends Controller
locale: $data['language'], locale: $data['language'],
scope: VisibilityScope::from($data['visibility']), scope: VisibilityScope::from($data['visibility']),
targets: isset($target) ? [$target] : [], targets: isset($target) ? [$target] : [],
reply_to: $data['reply_to_id'], reply_to: \array_key_exists('reply_to_id', $data) ? $data['reply_to_id'] : null,
attachments: $data['attachments'], attachments: $data['attachments'],
process_note_content_extra_args: $extra_args, process_note_content_extra_args: $extra_args,
); );

View File

@ -37,7 +37,6 @@ use function App\Core\I18n\_m;
use App\Core\Router\Router; use App\Core\Router\Router;
use App\Util\Common; use App\Util\Common;
use App\Util\Exception\ClientException; use App\Util\Exception\ClientException;
use App\Util\Exception\RedirectException;
use App\Util\Exception\ServerException; use App\Util\Exception\ServerException;
use App\Util\Formatting; use App\Util\Formatting;
use Symfony\Component\Form\Extension\Core\Type\HiddenType; use Symfony\Component\Form\Extension\Core\Type\HiddenType;
@ -92,7 +91,7 @@ abstract class Form
* Create a form with the given associative array $form as fields * Create a form with the given associative array $form as fields
* *
* If the current request has a GET parameter `next`, adds `_next` hidden. Default values gotten * If the current request has a GET parameter `next`, adds `_next` hidden. Default values gotten
* from the request, but can be overriden by `$extra_data['_next']` * from the request, but can be overridden by `$extra_data['_next']`
*/ */
public static function create( public static function create(
array $form, array $form,
@ -147,15 +146,16 @@ abstract class Form
} }
/** /**
* Handle the full life cycle of a form. Creates it with @param array $form_definition * Handle the full life cycle of a form
* *
* @param array $extra_args If specified, is used as $target->set($data[$key], $extra_args[$key]) * @param array $form_definition Creates it with a definition
* @param null|object $target Must be a persistent DB object
* @param array $extra_args If specified, is used as $target->set($data[$key], $extra_args[$key])
* *
* @throws RedirectException * @throws \Doctrine\ORM\ORMException
* @throws ServerException * @throws ServerException
* *
* @see * @see self::create and inserts the submitted values into the database
* self::create and inserts the submitted values into the database
*/ */
public static function handle(array $form_definition, Request $request, ?object $target, array $extra_args = [], ?callable $before_step = null, ?callable $after_step = null, array $create_args = [], ?SymfForm $testing_only_form = null): mixed public static function handle(array $form_definition, Request $request, ?object $target, array $extra_args = [], ?callable $before_step = null, ?callable $after_step = null, array $create_args = [], ?SymfForm $testing_only_form = null): mixed
{ {
@ -204,7 +204,6 @@ abstract class Form
// @codeCoverageIgnoreEnd // @codeCoverageIgnoreEnd
} }
DB::merge($target);
DB::flush(); DB::flush();
} }
} }
@ -216,6 +215,9 @@ abstract class Form
* Force a redirect to the `_next` form field, which is either a page to go after filling a * Force a redirect to the `_next` form field, which is either a page to go after filling a
* form, or the page where the form was (given we use form actions). This prevents the browser * form, or the page where the form was (given we use form actions). This prevents the browser
* from attempting to resubmit the form when the user merely ment to refresh the page * from attempting to resubmit the form when the user merely ment to refresh the page
*
* @throws ClientException
* @throws ServerException
*/ */
public static function forceRedirect(SymfFormInterface $form, Request $request): RedirectResponse public static function forceRedirect(SymfFormInterface $form, Request $request): RedirectResponse
{ {
@ -227,7 +229,7 @@ abstract class Form
$user = Common::user(); $user = Common::user();
$user_id = !\is_null($user) ? $user->getId() : '(not logged in)'; $user_id = !\is_null($user) ? $user->getId() : '(not logged in)';
Log::warning("Suspicious activity: User with ID {$user_id} submitted a form where the `_next` parameter is not a valid local URL ({$next})"); Log::warning("Suspicious activity: User with ID {$user_id} submitted a form where the `_next` parameter is not a valid local URL ({$next})");
throw new ClientException(_m('Invalid form submission'), previous: $e); throw new ClientException(_m('Invalid form submission'));
} }
} }
} }