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

Avoid return new MemoryStream, instead return null and manage nulls i… #661

Merged
merged 11 commits into from
Sep 22, 2022

Conversation

gmilano
Copy link
Contributor

@gmilano gmilano commented Aug 9, 2022

Veracode was reporting several "Improper Resource Shutdown or Release", the root cause of this in our case was that we return MemoryStreams in many places just to avoid handling the nulls. The side effect of this practice is that we are spreading disposable objects out the class boundary and nobody is releasing them.

@gmilano gmilano requested a review from claudiamurialdo August 9, 2022 13:49
@genexusbot
Copy link
Collaborator

Cherry pick to beta failed, 2 conflicted files in commit f63c717
  • .gitignore
  • dotnet/src/dotnetframework/GxClasses/Core/Web/GxHttpServer.cs

@genexusbot genexusbot added the conflict Conflict merging to beta branch label Aug 9, 2022
@gmilano gmilano had a problem deploying to external-storage-tests August 9, 2022 13:53 Failure
@gmilano gmilano temporarily deployed to external-storage-tests August 9, 2022 15:14 Inactive
@gmilano gmilano removed the conflict Conflict merging to beta branch label Aug 9, 2022
@genexusbot
Copy link
Collaborator

Manual cherry pick to beta success

@genexusbot
Copy link
Collaborator

Cherry pick to beta success

@gmilano gmilano had a problem deploying to external-storage-tests August 9, 2022 17:18 Failure
@genexusbot
Copy link
Collaborator

Cherry pick to beta success

claudiamurialdo
claudiamurialdo previously approved these changes Aug 9, 2022
@genexusbot
Copy link
Collaborator

Cherry pick to beta failed, 0 conflicted files in commit a2b3e0f (warning: no changes merged)

@claudiamurialdo claudiamurialdo temporarily deployed to external-storage-tests August 9, 2022 21:09 Inactive
@genexusbot genexusbot added the conflict Conflict merging to beta branch label Aug 9, 2022
@genexusbot
Copy link
Collaborator

Manual cherry pick to beta success

@genexusbot genexusbot removed the conflict Conflict merging to beta branch label Aug 9, 2022
@claudiamurialdo claudiamurialdo temporarily deployed to external-storage-tests August 9, 2022 21:55 Inactive
@genexusbot
Copy link
Collaborator

Cherry pick to beta success

… the stream ReceiveStream is changed by a byte[] array to avoid potential Improper Resource Shutdown errors when used outside StandardClasses.
@genexusbot
Copy link
Collaborator

Cherry pick to beta success

@claudiamurialdo claudiamurialdo temporarily deployed to external-storage-tests August 11, 2022 18:53 Inactive
@genexusbot
Copy link
Collaborator

Cherry pick to beta success

@claudiamurialdo claudiamurialdo temporarily deployed to external-storage-tests August 12, 2022 14:20 Inactive
@claudiamurialdo claudiamurialdo temporarily deployed to external-storage-tests August 12, 2022 19:25 Inactive
@genexusbot
Copy link
Collaborator

Cherry pick to beta success

@claudiamurialdo claudiamurialdo temporarily deployed to external-storage-tests August 16, 2022 16:23 Inactive
@genexusbot
Copy link
Collaborator

Cherry pick to beta success

…HttpRequest.GetStringFromStream.

Default value for bufferSize argument in StreamReader constructor is 1024 for .NET Framework and -1 for .NET.
@genexusbot
Copy link
Collaborator

Cherry pick to beta success

@claudiamurialdo claudiamurialdo temporarily deployed to external-storage-tests September 1, 2022 14:42 Inactive
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.

3 participants