[3.0] Say which board when merge topics is asked without one - #9413
Open
albertlast wants to merge 1 commit into
Open
[3.0] Say which board when merge topics is asked without one#9413albertlast wants to merge 1 commit into
albertlast wants to merge 1 commit into
Conversation
?action=mergetopics without a board in the URL is an HTTP 500: Typed static property SMF\Board::$info must not be accessed before initialization Sources/Actions/TopicMerge.php:240 index() reads Board::$info in three places - to default the target board, to build the page index, and to look the source topic up in the current board - and the whole method is written around there being one. Board::$info is typed with no default, so without a board none of those reads work, and the first one takes the request down with an uncaught error rather than a message. The merge button in a topic always includes board=, so this is reached by editing the URL rather than by using the forum. It should still say what is wrong: it now stops at the top with no_board, the same string the method already uses further down when the source topic is not in this board. Signed-off-by: Mathias Papenbrock <mathiaspapealbert@hotmail.com> Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
?action=mergetopics;from=3;targetboard=1— noboard=in the URL — is an HTTP 500:index()readsBoard::$infothree times: to default the target board, to build the page index, and to look the source topic up in the current board. The method is written around there being one.Board::$infois typed with no default, so without a board none of those reads work — and the first one takes the request down with an uncaught error instead of a message.It now stops at the top with
no_board, which is the string the same method already uses further down when the source topic turns out not to be in this board:The normal flow is unaffected —
?action=mergetopics;board=1.0;from=3still returns 200. The merge button in a topic always includesboard=(Actions/Display.php:1426), so this is reached by editing the URL rather than by using the forum; it should still say what is wrong rather than fall over.A note on
??My first attempt was
Board::$info->id ?? 0, which does not work here and is worth flagging because the pattern appears elsewhere in the codebase.??only guards the final dereference; the static property access itself still throws.isset(Board::$info)is the test that works, which is whatActions/Feed.phpuses and what #9411 does for the same problem in the news feed.Found by walking the moderation actions on a running forum and watching for non-2xx responses.
Issues References (Fixes|Related|Closes)
Related to #7933