Skip to content

Clear data after negotiation #6947

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

Merged
merged 1 commit into from
Dec 13, 2024
Merged

Clear data after negotiation #6947

merged 1 commit into from
Dec 13, 2024

Conversation

lrm29
Copy link
Contributor

@lrm29 lrm29 commented Nov 25, 2024

Fixes #6889.

I'm not sure why this works with https and not ssh. There are claims it works with the ssh "exec" backend but that wasn't the case for me.

Because we don't clear the data buffer, the "wants" get sent twice to the server. The second time is the call to git_smart__negotiation_step in line 568 of smart_protocol.c

@lrm29
Copy link
Contributor Author

lrm29 commented Nov 25, 2024

Just as well we have tests for this... I think I'm on the right lines here though.

@lrm29 lrm29 marked this pull request as draft November 25, 2024 22:24
@@ -433,6 +433,7 @@ int git_smart__negotiate_fetch(

if ((error = git_smart__negotiation_step(&t->parent, data.ptr, data.size)) < 0)
goto on_error;
git_str_clear(&data);
Copy link
Member

Choose a reason for hiding this comment

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

You are indeed - I think that you need to differentiate the clearing of data based on whether we're in RPC mode (HTTP) or not (SSH).

Suggested change
git_str_clear(&data);
if (!t->rpc)
git_str_clear(&data);

?

@ethomson ethomson merged commit eb84531 into libgit2:main Dec 13, 2024
3 of 19 checks passed
@ethomson ethomson added the bug label Dec 16, 2024
@lrm29 lrm29 deleted the ssh branch January 7, 2025 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

depth option does not seem to work with libssh2
2 participants