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

fix newline characters in comment #1550

Merged
merged 1 commit into from
Dec 4, 2024
Merged

Conversation

Varorbc
Copy link
Contributor

@Varorbc Varorbc commented Dec 3, 2024

No description provided.

@Rob-Hague
Copy link
Collaborator

Thanks! are you able to modify one of the .ppk files in test/Data in order to demonstrate the problem and fix?

@Varorbc
Copy link
Contributor Author

Varorbc commented Dec 3, 2024

The current test case has this problem because it is a ppk file obtained from an embedded resource, so it works fine. This problem occurs if you read directly from disk. So how do I add test cases? I hope you can give me some advice.

@scott-xu
Copy link
Collaborator

scott-xu commented Dec 4, 2024

@Varorbc Just out of curiosity, what's the binary difference between embedded resource and loose file?

@Varorbc
Copy link
Contributor Author

Varorbc commented Dec 4, 2024

@scott-xu Sorry about that, I got the issue mixed up earlier. Let me correct it: The PPK files in the current test cases use \n for new lines, but the PPK files generated by the older PuTTYgen client use \r\n.

@Varorbc Varorbc changed the title Update PuTTYPrivateKeyPattern fix newline characters in comment Dec 4, 2024
@Rob-Hague
Copy link
Collaborator

right, because the comment is included in the MAC, so we don't want to greedily match the \r. Thanks!

FYI: the changed .ppk file still has LF line endings because we normalize these files:

test/Data/* eol=lf

but the problem is understood well enough

@Rob-Hague Rob-Hague merged commit ee054f4 into sshnet:develop Dec 4, 2024
3 checks passed
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.

3 participants