Max request header size? about uwebsockets

[atkaper]

@janos-mands I have altered my PR to add your wish; #1661
Now you only need to get @uNetworkingAB to merge it some time...

We also needed the increase of the max values for graphql-mesh in our enterprise cloud environment. However it seemed the graphql-mesh we were using (possibly the same one? not sure) had multiple issues, among which some memory leak. We moved to try just stitching, instead of using mesh, which seems much more stable.

[atkaper]

As additional information; I think more than 4K is big, but... we have no control over this size where we are in the development chain. The infrastructure adds multiple big load-balancer cookies, and we have an authorization token with a JWT user context in them. We can not make the request size smaller.

[gillsoftab]

Hi @gillsoftab, thanks. But my question remains. The 4k is an arbitrary limit, and we can not make our code go under the 4k. I see no reason why we could not go to the more modern value of 8K, which is supported by many servers. None of the other components in our landscape have an issue with our headers.

If you ask us, we would have forced max header size to 2k or even 1k. If larger max header size is needed it's an indication the implementer(s) did something wrong or constructed a suboptimal solution. It might be a good idea for them to talk to the yellow rubber duck until progress is made.

[atkaper]

Hi @gillsoftab ok, no doubling of the setting then. How about we make it configurable via for example an environment variable (with default to 4K)? Then you can lower it to 1K, and we can increase it to 5K. Would that be ok with you?

[ddevienne]

Wondering if [...] the same kind of change for MAX_HEADERS as well.

No, since a compile-time constant used to size an array.

Making MAX_FALLBACK_SIZE configurable at runtime via getenv() cannot be done in this case,
w/o changing the allocation of the headers array to be dynamic (i.e. heap allocated), which affect performance,
and this make is less likely for @uNetworkingAB to accept it. Just a guess though. My $0.02. --DD

[atkaper]

@atkaper thanks for adding MAX_HEADERS to your PR. I can see that your primary concern of MAX_FALLBACK_SIZE has been addressed and so the PR is closed. I'll propose the same for MAX_HEADERS, but wondering if you guys do not get errors when the max header count is exceeded or it is just never the case for you. Worth noting that when the max size is exceeded, we are getting 431 back, but when the max count is exceeded, we are getting 500 from our cloud service, that is, based on our investigation by checking the process directly, caused by socket hung up/ timeout.

@janos-mands The same solution is not possible, as the header array size is a compile time thing/constant. Simplest is to just double the max in the source, and rebuild. The best solution would be to get the code changed to do dynamic header allocation (without extra limit apart from the existing total header bytes). I might have a look at it some day (no promises), but it's probably much faster if @uNetworkingAB could do it as my C has gotten a bit rusty (I moved to java around 2001) ;-)

As for the question if we ran into the max # of headers; I'm actually not sure, it could quite likely indeed have been the case without us knowing this. A lot of requests worked fine, but a part of them did fail. The number of request headers differs in our case depending on if end-users are anonymous, returning recognized users, or logged on users, and it depends on what browsers or proxies or providers they use. Perhaps we indeed went over the 50 headers in some cases. And our infra indeed adds quite some headers as well. So probably yes, this might have been one of our issues also.

What we did for building this graphql-mesh, is use a multi stage "Dockerfile", like this (all in one file):

# Temporary setup, we do a full checkout+build of the uWebSockets.js server project, to increase buffer size.
FROM ubuntu as uWebSocketsJsBuilder

RUN apt update && apt install -y  git build-essential clang zlib1g-dev libuv1-dev libssl-dev curl cmake golang
WORKDIR /app
RUN git clone --recursive https://github.com/uNetworking/uWebSockets.js.git --depth=1 --shallow-submodules
WORKDIR /app/uWebSockets.js
RUN sed -i 's/MAX_FALLBACK_SIZE = .*/MAX_FALLBACK_SIZE = 1024 * 8;/g' uWebSockets/src/HttpParser.h && grep 'MAX_FALLBACK_SIZE = 1024 \* 8;' uWebSockets/src/HttpParser.h
RUN make

# Normal build of mesh container
FROM node:18

WORKDIR /app
COPY package.json /app
RUN yarn install
COPY .meshrc.ts forward-headers-plugin.ts version-resolver.ts /app

# Overwrite uWebSockets.js server with results of uWebSocketsJsBuilder
RUN rm -rf /app/node_modules/uWebSockets.js/uws*
COPY --from=uWebSocketsJsBuilder /app/uWebSockets.js/dist/uws* /app/node_modules/uWebSockets.js/
RUN ls -la /app/node_modules/uWebSockets.js/uws*

CMD ["yarn", "start"]

Of course you could do something similar to override the max headers setting. Just add an SED line to above example.
The use of the multi stage docker file allowed us to do the build in a single step in our build pipelines (in github action). Although it was much slower than needed due to doing the extra build in stage 1.

[janos-mands]

Thanks @ddevienne, @atkaper - you guys are right, I have missed that point about static -> dynamic array allocation. (You can tell I am not a C++ programmer, for a long time.) I understand the additional, possible performance cost of making it dynamic, but unfortunately the original problem still remains. I assume we are not the only one who can reach this limit now, and given the different trends in IT, I belive we will not be last either, so I'll make a separate issue for it (instead of hijacking this issue), to make it more visible, and debate it further there , whatever will be the outcome.

@atkaper Thanks a lot for the build example, it could help us to make it working in the meantime, faster.

[atkaper]

See also: #1660

[gillsoftab]

As additional information; I think more than 4K is big, but... we have no control over this size where we are in the development chain. The infrastructure adds multiple big load-balancer cookies, and we have an authorization token with a JWT user context in them. We can not make the request size smaller.

Read

[atkaper]

Hi @gillsoftab, thanks. But my question remains. The 4k is an arbitrary limit, and we can not make our code go under the 4k. I see no reason why we could not go to the more modern value of 8K, which is supported by many servers. None of the other components in our landscape have an issue with our headers.

[gillsoftab]

Hi @gillsoftab ok, no doubling of the setting then. How about we make it configurable via for example an environment variable (with default to 4K)? Then you can lower it to 1K, and we can increase it to 5K. Would that be ok with you?

Hi, It's not us, rather @uNetworkingAB who is the authority regarding such changes. We as an example use this repo as a base, then we apply security and compliance related changes in our codebase. You might be able to do the same to meet your requirements.

[atkaper]

@gillsoftab at the moment, we indeed do build the code ourselves before we include it in the final service, with the buffer increase, but it would be easier if we can skip doing search/replaces, and having it as just a config option.

@uNetworkingAB would you be OK with making the max header setting configurable via for example an environment variable? (with default to 4K). I don't mind having a go at a PR, but if it won't be considered an option, it's not worth the time.

[atkaper]

A quick test in a separate hello world.cpp file indicates this line would work fine:

const size_t MAX_FALLBACK_SIZE = getenv("MAX_HEADER_LENGTH") ? atoi(getenv("MAX_HEADER_LENGTH")) : 1024 *4;

But as I'm not a CPP programmer, you might have preferences/ways to make it look nicer. Or maybe you already have standard functions in the uWebSockets to solve this differently.

[janos-mands]

Thanks for raising it @gillsoftab , we have the same problem and getting more and more serious, affecting production.

@uNetworkingAB could the max header settings be configurable for both MAX_FALLBACK_SIZE and MAX_HEADERS?

We are using a 3rd party framework graphql-mesh that is using your webserver. As our consumer base grows they starting facing errors due to the current header limits, both randomly 500s or 431s in production and e.g. when they what to add a new feature that require aditional headers, they figure out that they cannot add more headers. It makes the situation worse that

• The cloud infrastructure also adds additional headers, without having much influence on it
• The client applications are also not in control of all headers, the browsers are sending many of them automatically
• Some headers have inheretly big value, like JWT
• At bigger companies there can be different teams that they need to add different headers for their on purpose to the same request.

You can imagine that in today's cloud based web apps, considering the end-to-end flow, it is easy to reach the 50 header and 4k size limit, and it is difficult, if not impossible, to solve it by keeping the header limits within these values, since it would require controlling and limiting the functionality of all the different parts.

SInce we use uWebSockets <- uWebSockets.js <- graphql-mesh <- our app, it would be also much more difficult to have our own version of all these dependencies, rather than e.g. having an environment variable for each of these limits as configuration option.

[uNetworkingAB]

The PR doesn't compile

[atkaper]

Ah, yes, sorry ;-) I didn't have my coffee yet perhaps. This would require a dynamic allocation. I'll have another look at it.

[janos-mands]

@atkaper thanks for adding MAX_HEADERS to your PR. I can see that your primary concern of MAX_FALLBACK_SIZE has been addressed and so the PR is closed. I'll propose the same for MAX_HEADERS, but wondering if you guys do not get errors when the max header count is exceeded or it is just never the case for you. Worth noting that when the max size is exceeded, we are getting 431 back, but when the max count is exceeded, we are getting 500 from our cloud service, that is, based on our investigation by checking the process directly, caused by socket hung up/ timeout.

[janos-mands]

@uNetworkingAB thanks for making MAX_FALLBACK_SIZE configurable. Really appreciate it. Wondering if you could agree with the same kind of change for MAX_HEADERS as well. That would be a big help for us, since after that we could use uWebSocket as is, and it could avoid a chain of custom builds for us. I am happy to make a separate issue and PR for that.

[atkaper]

Max header count issue has been moved to #1689 and max request header size (in bytes) has been fixed in master. You can now set environment variable UWS_HTTP_MAX_HEADERS_SIZE to override the default max of 4096 bytes.

Closing this issue.