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(api): Add option to set custom fiber ReadBufferSize param #637

Closed
wants to merge 2 commits into from

Conversation

arturkasperek
Copy link

Summary

This PR adds option to setup custom parameter GATUS_API_READ_BUFFER_SIZE. This param tweaks fiber ReadBufferSize that control max header size (read more here https://github.com/gofiber/fiber/blob/master/docs/api/fiber.md#config).

The problem occurred in my company cause we use gatus on the same domain as app and this causes sending large cookies.

Issue: #636

Checklist

  • Tested and/or added tests to validate that the changes work as intended, if applicable.
  • Updated documentation in README.md, if applicable.

@arturkasperek
Copy link
Author

@TwiN please review

api/api.go Outdated Show resolved Hide resolved
Copy link
Owner

@TwiN TwiN left a comment

Choose a reason for hiding this comment

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

Do you think the default value (4096) is too low, given that you ran into this issue?

Comment on lines +38 to +50
func getReadBufferSize() int {
bufferSizeStr, exists := os.LookupEnv("GATUS_API_READ_BUFFER_SIZE")
if !exists {
return 4096 // Default value
}
bufferSize, err := strconv.Atoi(bufferSizeStr)
if err != nil {
log.Printf("Error converting GATUS_API_READ_BUFFER_SIZE to integer: %s", err.Error())
return 4096 // Default value in case of conversion error
}
return bufferSize
}

Copy link
Owner

Choose a reason for hiding this comment

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

Could you move this block at the bottom of the file?

@TwiN TwiN changed the title feat(api): Add option to set custom fiber ReadBufferSize param fix(api): Add option to set custom fiber ReadBufferSize param Dec 25, 2023
@TwiN TwiN added bug Something isn't working area/api Related to the REST API labels Dec 25, 2023
@codecov-commenter
Copy link

codecov-commenter commented Dec 25, 2023

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (3c246f0) 78.49% compared to head (c73a959) 78.41%.

Files Patch % Lines
api/api.go 45.45% 6 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #637      +/-   ##
==========================================
- Coverage   78.49%   78.41%   -0.08%     
==========================================
  Files          58       58              
  Lines        4705     4716      +11     
==========================================
+ Hits         3693     3698       +5     
- Misses        827      833       +6     
  Partials      185      185              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines +112 to +113
- [Troubleshooting](#troubleshooting)
- [How to solve `Request Header Fields Too Large` error?](#how-to-solve-request-header-fields-too-large-error)
Copy link
Owner

Choose a reason for hiding this comment

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

I feel like there's a bit of an overlap between Troubleshooting and the current use of the FAQ section 🤔

@lifez
Copy link

lifez commented Jan 18, 2024

I got this error as well while I put Gatus behind the Cloudflare Access. Can we merge this ?

@TwiN
Copy link
Owner

TwiN commented Jan 24, 2024

@lifez As soon as @arturkasperek addresses my comment and resolves the conflict in the PR, I can merge this :)

@lifez
Copy link

lifez commented Jan 24, 2024

@arturkasperek Let me know if you don't have bandwidth to address this. I can done this for us cc @TwiN

@arturkasperek
Copy link
Author

Dont have time - please handle that

@lifez
Copy link

lifez commented Jan 25, 2024

@TwiN @arturkasperek Since I cannot edit this PR, this is the new one #663

@TwiN
Copy link
Owner

TwiN commented Feb 1, 2024

Thank you both @arturkasperek @lifez!

@TwiN TwiN closed this Feb 1, 2024
TwiN added a commit that referenced this pull request Feb 7, 2024
This fixes the `431 Request Header Fields Too Large` error

By default, the read-buffer-size is 8192, up from fiber's default of 4096.

Fixes #674

Fixes #636

Supersedes #637

Supersedes #663
@TwiN
Copy link
Owner

TwiN commented Feb 7, 2024

Superseded by #675

TwiN added a commit that referenced this pull request Feb 7, 2024
This fixes the `431 Request Header Fields Too Large` error

By default, the read-buffer-size is 8192, up from fiber's default of 4096.

Fixes #674

Fixes #636

Supersedes #637

Supersedes #663
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api Related to the REST API bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants