From 9f49fbacc9cba2bc6edd7bb02ebd12a1eebbcb14 Mon Sep 17 00:00:00 2001 From: Diogo Peralta Cordeiro Date: Fri, 11 Mar 2022 03:14:47 +0000 Subject: [PATCH] [COMPONENT][Posting] Fix request handling issues that resulted from splitting creation and controller --- components/Conversation/Conversation.php | 61 ++++++++++++++--------- components/Group/Group.php | 22 +++++--- components/Posting/Controller/Posting.php | 2 +- src/Core/Form.php | 20 ++++---- 4 files changed, 65 insertions(+), 40 deletions(-) diff --git a/components/Conversation/Conversation.php b/components/Conversation/Conversation.php index 616c9db945..a7af95c94c 100644 --- a/components/Conversation/Conversation.php +++ b/components/Conversation/Conversation.php @@ -137,28 +137,6 @@ class Conversation extends Component 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. * @@ -194,6 +172,22 @@ class Conversation extends Component 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 * 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 { - $to_note_id = $request->query->get('reply_to_id'); + $to_note_id = $this->getReplyToIdFromRequest($request); if (!\is_null($to_note_id)) { // Getting the actor itself $context_actor = Actor::getById(Note::getById((int) $to_note_id)->getActorId()); @@ -215,6 +209,27 @@ class Conversation extends Component 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 */ diff --git a/components/Group/Group.php b/components/Group/Group.php index 1daa7d41d0..99911fdf42 100644 --- a/components/Group/Group.php +++ b/components/Group/Group.php @@ -85,13 +85,21 @@ class Group extends Component */ private function getGroupFromContext(Request $request): ?Actor { - if (str_starts_with($request->get('_route'), 'group_actor_view_')) { - if (!\is_null($id = $request->get('id'))) { - return Actor::getById((int) $id); - } - - if (!\is_null($nickname = $request->get('nickname'))) { - return LocalGroup::getActorByNickname($nickname); + if (\is_array($request->get('post_note')) && \array_key_exists('_next', $request->get('post_note'))) { + $next = parse_url($request->get('post_note')['_next']); + $match = Router::match($next['path']); + $route = $match['_route']; + $identifier = $match['id'] ?? $match['nickname'] ?? null; + } else { + $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; diff --git a/components/Posting/Controller/Posting.php b/components/Posting/Controller/Posting.php index 2da6960156..5f5e8bdac7 100644 --- a/components/Posting/Controller/Posting.php +++ b/components/Posting/Controller/Posting.php @@ -53,7 +53,7 @@ class Posting extends Controller locale: $data['language'], scope: VisibilityScope::from($data['visibility']), 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'], process_note_content_extra_args: $extra_args, ); diff --git a/src/Core/Form.php b/src/Core/Form.php index a1eaa877b2..c3ee18eb7d 100644 --- a/src/Core/Form.php +++ b/src/Core/Form.php @@ -37,7 +37,6 @@ use function App\Core\I18n\_m; use App\Core\Router\Router; use App\Util\Common; use App\Util\Exception\ClientException; -use App\Util\Exception\RedirectException; use App\Util\Exception\ServerException; use App\Util\Formatting; 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 * * 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( 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 * - * @see - * self::create and inserts the submitted values into the database + * @see 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 { @@ -204,7 +204,6 @@ abstract class Form // @codeCoverageIgnoreEnd } - DB::merge($target); 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 * 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 + * + * @throws ClientException + * @throws ServerException */ public static function forceRedirect(SymfFormInterface $form, Request $request): RedirectResponse { @@ -227,7 +229,7 @@ abstract class Form $user = Common::user(); $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})"); - throw new ClientException(_m('Invalid form submission'), previous: $e); + throw new ClientException(_m('Invalid form submission')); } } }