Skip to content

Comments

Warn on MaximumReceiveMessageSize#27014

Merged
guardrex merged 8 commits intomainfrom
guardrex-patch-1
Sep 27, 2022
Merged

Warn on MaximumReceiveMessageSize#27014
guardrex merged 8 commits intomainfrom
guardrex-patch-1

Conversation

@guardrex
Copy link
Collaborator

Fixes #27013

@guardrex
Copy link
Collaborator Author

guardrex commented Sep 16, 2022

Let's hold off on review. We're discussing this on the PU issue. We might do something different ... an INCLUDE used in the Blazor doc and the SignalR doc, for example. We might do this, but we might only do it in the SignalR doc because the Blazor doc already links to the SignalR doc for the configuration details.

@guardrex guardrex marked this pull request as draft September 16, 2022 10:29
@guardrex guardrex self-assigned this Sep 16, 2022
@guardrex
Copy link
Collaborator Author

guardrex commented Sep 17, 2022

The PU issue was closed right in the middle of the discussion on this.

Let's go with Rick's Gambit™ ... i.e. ... Call for review! 😆

Brennan, which way would you like to go?

@guardrex guardrex marked this pull request as ready for review September 17, 2022 23:12
@BrennanConroy
Copy link
Member

  • I can place it in the Description in the table at ...

Sure, similar to how ApplicationMaxBufferSize in a later table mentions memory usage can increase if the setting is changed.

@guardrex
Copy link
Collaborator Author

@BrennanConroy ... Added a remark to the table. How's it look?

Copy link
Contributor

@TanayParikh TanayParikh left a comment

Choose a reason for hiding this comment

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

LGTM, will leave the final sign-off for @BrennanConroy

Co-authored-by: Tanay Parikh <TanayParikh@users.noreply.github.com>
@guardrex guardrex closed this Sep 20, 2022
@guardrex guardrex reopened this Sep 20, 2022
Copy link
Contributor

@TanayParikh TanayParikh left a comment

Choose a reason for hiding this comment

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

LGTM, but I'd await @BrennanConroy's signoff.

@guardrex
Copy link
Collaborator Author

guardrex commented Sep 20, 2022

I will ... and btw ... I've talked myself out of using the explicit byte conventions for abbreviations, as you recommended, on the issue that I opened ...

#27056

I wish MS/.NET foundation would adopt the standard. What I can do is place a remark on it in the Blazor Fundamentals topic and just use "KB"/"MB"/"GB" everywhere.

I'm going to perform a universal update on that per that issue, so let's not sweat it on this PR. UPDATE: I fixed it on this PR and will address it elsewhere on another PR.

@guardrex
Copy link
Collaborator Author

@BrennanConroy ... Are you OOF? No rush ... just checking.

@guardrex
Copy link
Collaborator Author

@BrennanConroy ... Ok ... ready for another 👁️.

@guardrex guardrex merged commit 1eb1c97 into main Sep 27, 2022
@guardrex guardrex deleted the guardrex-patch-1 branch September 27, 2022 23:51
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.

Warn on MaximumReceiveMessageSize size (DOS risk)

3 participants