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.

Sign up for GitHub

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

Jump to bottom

[CHANGED] Enforce max_control_line for client connections only #1850

Merged
merged 2 commits into from Jan 26, 2021
Merged

[CHANGED] Enforce max_control_line for client connections only #1850

merged 2 commits into from Jan 26, 2021

Conversation

kozlovic
Copy link
Member

Only check limit for CLIENT connection types, however, the check
is done for CLIENT in all conditions, not only in the case of a
split buffer as it was the case so far.

Signed-off-by: Ivan Kozlovic ivan@synadia.com

@kozlovic
[CHANGED] Enforce max_control_line for client connections only
dce8140 
Only check limit for CLIENT connection types, however, the check
is done for CLIENT in all conditions, not only in the case of a
split buffer as it was the case so far.

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
wallyqs
wallyqs reviewed Jan 26, 2021
View reviewed changes
server/const.go Outdated Show resolved Hide resolved
@kozlovic
Fixed typo in new const name
b07e176 
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
derekcollison
derekcollison reviewed Jan 26, 2021
View reviewed changes
Copy link
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

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

In general looks good, wondering if we can just access via server instance versus having to put it into every client which are already in need of a diet.

server/client.go Show resolved Hide resolved
@derekcollison derekcollison self-requested a review January 26, 2021 23:20
derekcollison
derekcollison approved these changes Jan 26, 2021
View reviewed changes
Copy link
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

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

LGTM

@kozlovic kozlovic merged commit 3369476 into master Jan 26, 2021
@kozlovic kozlovic deleted the max_control_line branch January 26, 2021 23:25
@wubumihuo wubumihuo mentioned this pull request Jan 17, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wallyqs wallyqs wallyqs left review comments

@derekcollison derekcollison derekcollison approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@kozlovic @wallyqs @derekcollison