Skip to content
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

The RE used to split tokens is too narrow #10771

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

asterbini
Copy link

I noticed that when using conda WITH tokens sometimes the token part is not recognized if it contains also the char '_' (underscore).
This patch enlarge regular expression to split the token to all chars different from '/'

@asterbini asterbini requested a review from a team as a code owner July 5, 2021 13:04
@anaconda-issue-bot
Copy link

We require contributors to sign our Contributor License Agreement, and we don't have one on file for @asterbini. In order for us to review and merge your code, please e-sign the PDF at https://conda.io/en/latest/contributing.html#conda-contributor-license-agreement. We then need to manually verify your signature. We will ping the bot to refresh the PR status when we have confirmed your signature.

@jezdez
Copy link
Member

jezdez commented Jul 5, 2021

@anaconda-issue-bot check

@anaconda-issue-bot anaconda-issue-bot added the cla-signed [bot] added once the contributor has signed the CLA label Jul 5, 2021
@jezdez jezdez added the type::bug describes erroneous operation, use severity::* to classify the type label Jul 5, 2021
@wagnerpeer
Copy link

wagnerpeer commented Jul 7, 2021

I doubt that the regex [^/] is fully correct, as it would allow URLs/token to contain ? and other special characters.
I haven't found a reference to the parts where token are generated, but I assume that token will always be alphanumeric with dividers of blocks like - and _. Hence, extending the regex to include _ seems like the better option to me.

I am also missing the same changes to conda/conda/gateways/logging.py#L32

Copy link

@wagnerpeer wagnerpeer left a comment

Choose a reason for hiding this comment

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

What about similar changes to conda/conda/gateways/logging.py#L32?

@asterbini
Copy link
Author

What about similar changes to conda/conda/gateways/logging.py#L32?

Yes, it makes sense. I did not notice that RE, I just was fixing mamba/conda .

@wolfv
Copy link
Contributor

wolfv commented Jul 8, 2021

@wagnerpeer the regex you linked has teh same prolbme for the token matching though (r'/t/[a-z0-9A-Z-]+/' # token) as in not matching a token with an underscore.

The problem here is that the first two letters of a token generated by anaconda.org are the first two letters of the username.

Apparently anaconda.org allows underscores in usernames.

So when your username is x_blabla your tokens will look like x_-uuid4123-1231-1231-1321-aaaaaa which breaks these regex's.

@wagnerpeer
Copy link

@wagnerpeer the regex you linked has teh same prolbme for the token matching though (r'/t/[a-z0-9A-Z-]+/' # token) as in not matching a token with an underscore.

Correct, that was the intention. Making aware of another location with the same issues.

The problem here is that the first two letters of a token generated by anaconda.org are the first two letters of the username.

Apparently anaconda.org allows underscores in usernames.

So when your username is x_blabla your tokens will look like x_-uuid4123-1231-1231-1321-aaaaaa which breaks these regex's.

That explains the issue, but can you give a reference (documentation) on how a token is defined? Where did you find, that the first two characters are copied from the username?
If the statement holds true, then a solution could look like r'/t/.{2}[a-z0-9A-Z-]+/' # token?

@asterbini
Copy link
Author

asterbini commented Jul 19, 2021

That explains the issue, but can you give a reference (documentation) on how a token is defined? Where did you find, that the first two characters are copied from the username?
If the statement holds true, then a solution could look like r'/t/.{2}[a-z0-9A-Z-]+/' # token?

I just noticed that my username on anaconda starts with 'a_' and the token start with the same two chars
.{2} seems to me too wide, perhaps [^/]{2} or something even narrower ?

@chenghlee chenghlee self-assigned this Jul 20, 2021
@chenghlee chenghlee added this to the Release TBD milestone Sep 20, 2021
@jezdez jezdez mentioned this pull request Jan 5, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed [bot] added once the contributor has signed the CLA type::bug describes erroneous operation, use severity::* to classify the type
Projects
Status: 🆕 New
Development

Successfully merging this pull request may close these issues.

6 participants