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 avoidable exception when data length is too long #823

Merged
merged 4 commits into from Nov 19, 2023

Conversation

se006
Copy link
Contributor

@se006 se006 commented May 13, 2021

Fix #713
Fix #1243

Data lengths that are both longer than int.MaxValue and data bytes are now ignored and do not throw an exception.

This would come up when connecting to a Tectia SFTP server running on a mainframe.

Credit to @edmlin for the fix. This is for issue #713

…r than int.maxvalue are ignored and do not throw an exception
Copy link
Collaborator

@IgorMilavec IgorMilavec left a comment

Choose a reason for hiding this comment

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

You should change/remove test Load_ShouldThrowNotSupportedExceptionWhenDataLengthIsGreaterThanInt32MaxValue for this PR to pass.

@se006
Copy link
Contributor Author

se006 commented May 14, 2021

I have removed the unit test as I cannot think of a way to enter into the middle condition,

 else if (dataLength > int.MaxValue)
            {
                throw new NotSupportedException(string.Format(CultureInfo.CurrentCulture, "Data longer than {0} is not supported.", int.MaxValue));
            }

Perhaps this condition should be removed entirely.

@IgorMilavec
Copy link
Collaborator

With the current implementation of SshData I believe you cannot test this case.
This exception provides a more explicit error information than the OverflowException that would happen anyway, and should stay in the code IMO.

@IgorMilavec
Copy link
Collaborator

@se006 can you please edit the PR description (first comment) to include text "Fix #713" so this PR will be linked to the issue? Tnx!

@se006
Copy link
Contributor Author

se006 commented Feb 21, 2022

Fix #713 added to first comment as requested

Copy link
Collaborator

@WojciechNagorski WojciechNagorski left a comment

Choose a reason for hiding this comment

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

LGTM!
I wish there was a test, but I know it will be difficult.

@WojciechNagorski WojciechNagorski merged commit daa1acc into sshnet:develop Nov 19, 2023
1 check passed
@WojciechNagorski WojciechNagorski added this to the 2023.0.1 milestone Nov 19, 2023
@WojciechNagorski
Copy link
Collaborator

The 2023.0.1 version has been released to Nuget: https://www.nuget.org/packages/SSH.NET/2023.0.1

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