Skip to content

web: fix a couple of PHP warnings #6389

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 13, 2025
Merged

web: fix a couple of PHP warnings #6389

merged 1 commit into from
Jun 13, 2025

Conversation

davidpanderson
Copy link
Contributor

No description provided.

@Copilot Copilot AI review requested due to automatic review settings June 12, 2025 23:12
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes PHP warnings by initializing missing variables before use.

  • Adds a call to master_url() to initialize $master_url in weak_auth.php.
  • Retrieves the logged-in user before subscription actions to prevent undefined variable warnings in forum_forum.php.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
html/user/weak_auth.php Initialize $master_url before calling strpos and substr to eliminate undefined variable.
html/user/forum_forum.php Call get_logged_in_user() before subscription methods to ensure $user is defined.
Comments suppressed due to low confidence (1)

html/user/forum_forum.php:275

  • Add a check to ensure get_logged_in_user() returns a valid user before using $user->id, and handle unauthorized access (e.g., redirect to login or show an error) to avoid null dereference warnings.
$user = get_logged_in_user();

Comment on lines +275 to +279
$user = get_logged_in_user();
BoincSubscription::replace($user->id, -$id);
forum_page($forum, $user, 'You are now subscribed to this forum.');
} else if ($action == 'unsubscribe') {
$user = get_logged_in_user();
Copy link
Preview

Copilot AI Jun 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider retrieving the logged-in user once before the conditional rather than repeating get_logged_in_user() in both branches to reduce duplication.

Suggested change
$user = get_logged_in_user();
BoincSubscription::replace($user->id, -$id);
forum_page($forum, $user, 'You are now subscribed to this forum.');
} else if ($action == 'unsubscribe') {
$user = get_logged_in_user();
BoincSubscription::replace($user->id, -$id);
forum_page($forum, $user, 'You are now subscribed to this forum.');
} else if ($action == 'unsubscribe') {

Copilot uses AI. Check for mistakes.

@AenBleidd AenBleidd added this to Server Jun 13, 2025
@github-project-automation github-project-automation bot moved this to Backlog in Server Jun 13, 2025
@AenBleidd AenBleidd added this to the Server Release 1.6.0 milestone Jun 13, 2025
@AenBleidd AenBleidd merged commit 474589e into master Jun 13, 2025
175 checks passed
@AenBleidd AenBleidd deleted the dpa_web32 branch June 13, 2025 03:06
@github-project-automation github-project-automation bot moved this from Backlog to Done in Server Jun 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants