api: lower default for MaxHeaderBytes.

[winder]

Summary

We only require an API token in the header, so a much lower default for MaxHeaderBytes can be used. The gossip network port already uses 4096 as the default.

[codecov]

Codecov Report

Merging #5171 (10c63f4) into master (9220f7b) will increase coverage by 0.02%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #5171      +/-   ##
==========================================
+ Coverage   53.60%   53.63%   +0.02%     
==========================================
  Files         439      439              
  Lines       54934    54935       +1     
==========================================
+ Hits        29447    29463      +16     
+ Misses      23208    23194      -14     
+ Partials     2279     2278       -1     

Impacted Files	Coverage Δ
daemon/algod/server.go	3.92% <0.00%> (-0.02%)	⬇️
data/transactions/verify/streamverifier.go	90.24% <0.00%> (-2.44%)	⬇️
network/wsNetwork.go	69.08% <0.00%> (+0.17%)	⬆️
ledger/testing/randomAccounts.go	56.88% <0.00%> (+0.61%)	⬆️
catchup/service.go	71.05% <0.00%> (+1.41%)	⬆️
network/wsPeer.go	71.26% <0.00%> (+1.83%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[cce]

To better conform with best practices it would be nice to put constants like at the top of the file with names like

go-algorand/network/wsNetwork.go

Lines 75 to 79 in 9220f7b

	// MaxHeaderBytes controls the maximum number of bytes the
	// server will read parsing the request header's keys and
	// values, including the request line. It does not limit the
	// size of the request body.
	const httpServerMaxHeaderBytes = 4096

[cce]

https://en.wikipedia.org/wiki/Magic_number_(programming)
https://github.com/tommy-muehle/go-mnd

[winder]

According to your wikipedia link, this is not a magic number:

A unique value with unexplained meaning or multiple occurrences which could (preferably) be replaced with a named constant

The number is assigned to a clearly defined value and there is exactly 1 occurance.

A constant numerical or text value used to identify a file format or protocol; for files, see List of file signatures

This is not some cross system value, just a number of bytes.

A distinctive unique value that is unlikely to be mistaken for other meanings (e.g., Globally Unique Identifiers)

It is not distinctive, just a number of bytes.

It seems to me that this suggestion is about the letter of the rule but completely misses the spirit. The constant is used in exactly one place to assign a value to a variable with nice documentation.

I'll add a comment above the assignment that describes why the value was chosen. I did wonder why it was chosen for wsNetwork.

[winder]

The wsNetwork comment is copy/pasted from the http.MaxHeaderBytes comment. It would be nice if the comment were changed to describe why the value is 4096.

[cce]

This is the convention used extensively in the network package, and that much of go-algorand follows — similarly, const httpServerMaxHeaderBytes is used in one place in wsNetwork.go — and a pretty common convention, I thought? I am assuming we wanted to follow this convention as practiced in go-algorand whenever new consts are introduced... You can similar usage at the top of api/server/v2/handlers.go (just opening an API related file) and other places..

[winder]

I don't care enough to debate this point, so I've made the change. As I mentioned, while this suggestion may indeed follow the letter of the rule, it misses the spirit.