Skip to content

Conversation

@faretek1
Copy link
Collaborator

@faretek1 faretek1 commented May 30, 2025

Decode session_id to reduce request count (solves #395)

Upon logging in, scratchattach fetches the xtoken separately from the session id, although the session id, when decoded, actually contains the xtoken and some other useful information.

This PR implements

  • utils.commons.b62_decode - this decodes the base 62 timestamp (part 2 of the session id) into a unix timestamp
  • site.session.decode_session_id - this decodes the session id into a dictionary containing various pieces of data (part 1 of the session id), and a datetime object generated from the base62 encoded timestamp
  • site.session.Session._process_session_id - this sets attributes of the Session object using data which is collected from the session id. Note the use of the word process, which implies that no web requests are made. This method is automatically called upon __init__ if a session id is provided

note: the ip is not collected from the session id. This is because this information is much more likely to be harmful than be helpful. Do tell if you want this changed

todo:

  • simplify login functions which do not need to fetch the xtoken if the session id is already present
  • testing

@faretek1
Copy link
Collaborator Author

faretek1 commented May 30, 2025

Is assert_never supposed to be imported from typing? I changed it to typing_extensions because it was giving me an error. I am using python 3.10

Marked as duplicate because this is now mentioned in the comment below

@faretek1
Copy link
Collaborator Author

faretek1 commented Jun 1, 2025

@TheCommCraft
There are a couple of things to consider here.

  1. With the use of the session id to decode the xtoken and other data, the session.update call within login_by_id has been removed, which could cause some compatability issues with any code that assumes that certain information is already available, e.g. Whether you are banned
  2. The login-ip value from the session id has not been saved, considering it probably harms more than helps (although it is easy for users to fetch this by themselves if they know what they are doing, using commons.decode_session_id)
  3. When setting values such as Session.xtoken and Session.username, one has to make sure that they are setting certain other values too (Session._headers["X-token"] and Session._username), which is easy to forget - should @property or special setter functions be used?
    • in Session._process_session_id, a new attribute Session.language is set, but there is a corresponding value in the cookies: Session._cookies["scratchlanguage"] - should this be set, or will this interfere with certain HTML parsing?
    • similarly, values such as the session id has to be transferred to Session._cookies. Should @property be used for this? Should we consider using requests.Session?
  4. Should the session string value be generated on Session initialisation, or should it be only generated on demand, e.g. using @property?
  5. Should the xtoken parameter be removed from login_by_id, because it is completely redundant when decoding it from the session_id, which is a required parameter of that function?
  6. I changed from typing import assert_never to from typing_extensions import assert_never because it was causing me issues. Is this a problem?

Also, since merging this PR will cause niche incompatibility issues, this probably calls for a change in the Semantic Version number. If one is to change the version to 3.0.0, it might also be worth making more significant changes at the same time - e.g. shifting to dataclasses, and considering what to do with project_json_capabilities.py/scratchattach.editor

@faretek1 faretek1 marked this pull request as ready for review June 1, 2025 12:51
@TheCommCraft
Copy link
Collaborator

If it's an incompatible change, it should be a major version change. https://tom.preston-werner.com/2022/05/23/major-version-numbers-are-not-sacred.html

Although, if we decide to make incompatible changes, we should probably fix some important problems directly with it.

@TheCommCraft
Copy link
Collaborator

properties should definitely be used

@TheCommCraft
Copy link
Collaborator

maybe the xtoken could be used to check against

@faretek1
Copy link
Collaborator Author

faretek1 commented Jun 1, 2025

i checked and @property is compatible all the way back to python 3.2 (any maybe more), so there's not much reason to not use them

Signed-off-by: faretek <107722825+FAReTek1@users.noreply.github.com>
@faretek1
Copy link
Collaborator Author

faretek1 commented Jun 2, 2025

it still appears to work, so should this be merged?

@faretek1 faretek1 requested a review from TheCommCraft June 2, 2025 21:54
@TheCommCraft
Copy link
Collaborator

Do you know why there is a _username attribute?

@faretek1
Copy link
Collaborator Author

faretek1 commented Jun 3, 2025

apparently it is for backwards compatibility

@TheCommCraft
Copy link
Collaborator

But it is a protected attribute? We could make it a property

@faretek1 faretek1 merged commit f388112 into TimMcCool:main Jun 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants