Repository navigation
Bound a circuit's reliable list, keep a secure login on TLS, give up on an unanswered local teleport - #637
Merged
Conversation
…packets The list of reliable packets kept for duplicate suppression shrinks only when the peer's StartPingCheck names its oldest unacked. A peer that sends reliable packets and no pings grew it by a map node per number, with 2^24 numbers before they repeat. It now holds 65536. The one that goes is the oldest number, the first past the highest seen, since numbers wrap, or else the lowest. A number forgotten this way is handled again if a resend of it comes, which takes 65536 newer reliable packets arriving with no ping between them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A login's next attempt sends the user's credentials to wherever the last answer's next_url says, and a login over https would follow it to http. It now ends with a failure that says why (LoginFailedInsecureRedirect). A plain login, as on a local grid, isn't held to this. The same could happen a level down. libcurl follows redirects for XML-RPC, and a 307 or 308 sends a POST's body on as it is, so one from an https login server to http sent the credentials in the clear. XML-RPC over https now sets the new HttpOptions::setRedirectsHttpsOnly, which limits libcurl's redirects to https and fails the request with CURLE_UNSUPPORTED_PROTOCOL otherwise. Other requests follow redirects as before. The test peer has no TLS, so the tests run the refusal and the default against a redirect to http. A redirect to https under the option isn't run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A teleport inside the agent's region sets TELEPORT_LOCAL, and gTeleportDisplay stays false until TeleportLocal comes, so update_tp_display, which gives up on a teleport past its time, doesn't run for it. If neither TeleportLocal nor TeleportFailed came, the state stayed for the rest of the session, and teleportCore refused every later teleport as one made while teleporting. The timer now starts as the teleport is asked for, and display gives it up after the same time as any other: 15 s, and 3 s per attachment. An answer that comes later is handled as one with no teleport under way, which still moves the agent. No test covers this, since newview has no harness for the teleport state. To check it in world, add TeleportLocal under messageBans/trusted in app_settings/message.xml and teleport within the region: "Giving up on a teleport inside the region" is logged after 15 s, and the next teleport is accepted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 26 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
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
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.
Summary
Three separate client-side defects, one commit each.
A peer that never pings grew a circuit's list of reliable packets without limit.
mRecentlyReceivedReliablePacketsshrinks only when the peer'sStartPingChecknames its oldest unacked. It now holds 65536, and the oldest number goes first: the first past the highest seen, since numbers wrap, or else the lowest. A number forgotten this way is handled again if a resend of it comes, which takes 65536 newer reliable packets arriving with no ping between them.A login over https could be redirected to http, carrying the credentials. This could happen two ways, and both are closed:
next_url.loginCororesends the same parameters to wherever it says. It now ends with a failure that says why (LoginFailedInsecureRedirect).HttpOptions::setRedirectsHttpsOnly, which limits libcurl's redirects to https. Other requests follow redirects as before.A plain http login, as on a local grid, isn't held to this. A grid whose https login server redirects to http will now fail to log in, which is the point, but it is a visible change for such a grid.
A teleport inside the region with no answer blocked every later teleport.
TELEPORT_LOCALleavesgTeleportDisplayfalse untilTeleportLocalcomes, soupdate_tp_display, which gives up on a late teleport, doesn't run for it. If neitherTeleportLocalnorTeleportFailedcame,teleportCorerefused every later teleport for the rest of the session. The timer now starts as the teleport is asked for, and it's given up on after the same wait as any other: 15 s, and 3 s per attachment. An answer that comes later still moves the agent.Testing
llcircuit_test,lllogin_testandllcorehttp_testsgain cases. With each fix withheld, the cases that should catch it failed, and mutants of the fixes were caught.ctest -L "llmessage|lllogin|llcorehttp" -LE manualpasses 44 of 44 on Windows. That ran on the branch before A face an object gains takes its own texture entry, material and override #633, Object updates and kills keep to the objects they name, and two patch code fixes #635 and A packet number past 24 bits no longer hangs the message system #636 merged; it merges cleanly over them, and macOS, Linux and the merged result are left to CI.newview's three changed files were compiled alone. The viewer wasn't linked.newviewhas no harness for the teleport state. To check it, addTeleportLocalundermessageBans/trustedinapp_settings/message.xmland teleport within the region: "Giving up on a teleport inside the region" is logged after 15 s plus 3 s per attachment, and the next teleport is accepted.🤖 Generated with Claude Code