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

#11917 fix: Possibility to handle long URLs #11918

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

p24-max
Copy link

@p24-max p24-max commented Apr 4, 2024

Fixes #11917

@Seldaek
Copy link
Member

Seldaek commented Apr 12, 2024

I'd rather have some additional processing that removes the username/password from the URL before normalizing it, but keeps existing URLs working as they do now. It's much more debuggable than having md5s of URLs in the cache, plus doing this change would blast everyone's cache which is also a bit of a waste.

@Seldaek Seldaek added this to the 2.7 milestone Apr 12, 2024
@p24-max
Copy link
Author

p24-max commented Apr 15, 2024

@Seldaek yes I do understand, the thing is if we just remove the credentials it would fix my problem for sure. But it will not fix other long URLs which are too long for whatever reason, e.g. #7313

From my point of view - whenever possible - it should be avoided to pass external data to file-system operations. You may encounter different issues on different systems (encoding, limitations) and it tends to security flaws. In this case, a git process is spawned containing the sanitised URL as an argument. If sanitisation is not working properly, it may allow to execute arbitrary code. (I'm not saying that this is possible at the moment).

plus doing this change would blast everyone's cache which is also a bit of a waste.

True.

@Seldaek
Copy link
Member

Seldaek commented Apr 15, 2024

Yeah I don't really see super long name as a valid use case. This showed up once. I see the problem with credentials as that is much more likely to be long, but that really should not be part of the repository URL as a cache identifier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bitbucket] [AccessTokenAuth] File name too long
2 participants