Fix loginpage.php Loop#971
Closed
def-WA2025 wants to merge 6 commits into
Closed
Conversation
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds defensive checks around auto-login redirects to avoid an infinite loop on loginpage.php when the server responds with a "Please logout First!" message, by restoring the last page instead of re-triggering login navigation. Sequence diagram for AutoLogin redirect with Please logout First handlingsequenceDiagram
actor User
participant Browser
participant UserScript
participant XMOJServer
User->>Browser: Navigate to protected_page
Browser->>XMOJServer: GET /protected_page
XMOJServer-->>Browser: 302 to /loginpage.php
XMOJServer-->>Browser: Body contains 请登录后继续操作 link
Browser-->>UserScript: DOM ready on protected_page
UserScript->>Browser: Check AutoLogin enabled
UserScript->>Browser: Detect 请登录后继续操作 link
UserScript->>Browser: Save location to localStorage UserScript-LastPage
UserScript->>Browser: Redirect to /loginpage.php
Browser->>XMOJServer: GET /loginpage.php
XMOJServer-->>Browser: 200 loginpage.php with link text Please logout First!
Browser-->>UserScript: DOM ready on loginpage.php
UserScript->>Browser: Check AutoLogin enabled and profile missing
UserScript->>Browser: Read first anchor text
alt Anchor text is Please logout First!
UserScript->>Browser: Read UserScript-LastPage
UserScript->>Browser: Redirect to stored last page
else Anchor text is not Please logout First!
UserScript->>Browser: Save loginpage.php as UserScript-LastPage
UserScript->>Browser: (Potential further AutoLogin handling)
end
Flow diagram for updated AutoLogin redirect logicflowchart TD
Start["Check if AutoLogin redirect is needed"]
AutoLoginEnabled{"UtilityEnabled(AutoLogin)?"}
HasLoginPrompt{"First body link text is 请登录后继续操作?"}
OnLoginPageProfileMissing{"On page without profile or profile is 登录?"}
IsLogoutFirst{"First anchor text is Please logout First!?"}
RestoreLastPage["Redirect to localStorage UserScript-LastPage"]
SaveLastPage["Save current location to UserScript-LastPage"]
GoToLoginPage["Redirect to https://www.xmoj.tech/loginpage.php"]
End["Page navigation complete"]
Start --> AutoLoginEnabled
AutoLoginEnabled -- No --> End
AutoLoginEnabled -- Yes --> HasLoginPrompt
HasLoginPrompt -- Yes --> IsLogoutFirst
HasLoginPrompt -- No --> OnLoginPageProfileMissing
OnLoginPageProfileMissing -- Yes --> IsLogoutFirst
OnLoginPageProfileMissing -- No --> End
IsLogoutFirst -- Yes --> RestoreLastPage --> End
IsLogoutFirst -- No --> SaveLastPage --> GoToLoginPage --> End
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
hendragon-bot
Bot
added
the
user-script
Deploying xmoj-script-dev-channel with
|
| Latest commit: |
2a8b158
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://20ea2cc3.xmoj-script-dev-channel.pages.dev |
| Branch Preview URL: | https://fix-loginpage-loop.xmoj-script-dev-channel.pages.dev |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The checks for
document.querySelector('a').innerText == "Please logout First!"are repeated in three places and assume the anchor always exists; consider storing the result ofdocument.querySelector('a')in a variable, null‑checking it, and extracting a small helper to keep this logic in one place. - When redirecting back with
location.href = localStorage.getItem("UserScript-LastPage"), it may be safer to guard against a missing or empty value (e.g., fall back to a sensible default) to avoid navigating tonullor an invalid URL.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The checks for `document.querySelector('a').innerText == "Please logout First!"` are repeated in three places and assume the anchor always exists; consider storing the result of `document.querySelector('a')` in a variable, null‑checking it, and extracting a small helper to keep this logic in one place.
- When redirecting back with `location.href = localStorage.getItem("UserScript-LastPage")`, it may be safer to guard against a missing or empty value (e.g., fall back to a sensible default) to avoid navigating to `null` or an invalid URL.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="XMOJ.user.js">
<violation number="1" location="XMOJ.user.js:863">
P1: The new "Please logout First!" redirect is immediately overridden by a following unconditional redirect, so it does not actually break the login loop.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| //otherwise CurrentUsername might be undefined | ||
| if (UtilityEnabled("AutoLogin") && document.querySelector("body > a:nth-child(1)") != null && document.querySelector("body > a:nth-child(1)").innerText == "请登录后继续操作") { | ||
| if (document.querySelector('a').innerText == "Please logout First!") location.href = localStorage.getItem("UserScript-LastPage"); |
Contributor
There was a problem hiding this comment.
P1: The new "Please logout First!" redirect is immediately overridden by a following unconditional redirect, so it does not actually break the login loop.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At XMOJ.user.js, line 863:
<comment>The new "Please logout First!" redirect is immediately overridden by a following unconditional redirect, so it does not actually break the login loop.</comment>
<file context>
@@ -860,13 +860,15 @@ GM_registerMenuCommand("重置数据", () => {
//otherwise CurrentUsername might be undefined
if (UtilityEnabled("AutoLogin") && document.querySelector("body > a:nth-child(1)") != null && document.querySelector("body > a:nth-child(1)").innerText == "请登录后继续操作") {
+ if (document.querySelector('a').innerText == "Please logout First!") location.href = localStorage.getItem("UserScript-LastPage");
localStorage.setItem("UserScript-LastPage", location.pathname + location.search);
location.href = "https://www.xmoj.tech/loginpage.php";
</file context>
Contributor
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="package.json">
<violation number="1" location="package.json:3">
P2: Do not manually bump `package.json` version; this project’s release workflow manages version updates automatically and manual edits can desynchronize version sources.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Signed-off-by: zsTree <wa2025666@gmail.com>
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="package.json">
<violation number="1" location="package.json:3">
P1: Version was updated only in `package.json`, causing version desynchronization with `XMOJ.user.js` and `Update.json`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| { | ||
| "name": "xmoj-script", | ||
| "version": "3.3.5", | ||
| "version": "3.4.4", |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Signed-off-by: zsTree <wa2025666@gmail.com>
2 tasks
2 tasks
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
1 participant
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.
What does this PR aim to accomplish?
Fix loginpage.php loop.
How does this PR accomplish the above?
Check
Please logout First!and go back.By submitting this pull request, I confirm the following:
git rebase)Summary by Sourcery
Bug Fixes:
Summary by cubic
Prevents an infinite redirect loop on
loginpage.phpby detecting the “Please logout First!” message and sending users back toUserScript-LastPageinstead of re-triggering AutoLogin. Updates release metadata: adds 3.4.4 toUpdate.json(with corrected/v1route text) and bumps versions inXMOJ.user.jsandpackage.jsonto 3.4.4.Written for commit 2a8b158. Summary will update on new commits.