Setting an header of empty strings (~headers:[ "", "" ]) causes hang
Closed this issue · 15 comments
dream~alpha4, specifically Dream.json.
The workaround is laughable (don't do that), so I didn't spend any time digging to see if it affects other helpers etc.
One possible clue is that if I dune exec /.app.exe locally on macos 12.4 (arm64) I don't see any problems, only when built + deployed on alpine:3.12 (x86).
If I figure out anything more I'll update, but this is super low priority just want this to be searchable.
Thanks for Dream!
taking a look
i'll look to create a separate repo and link here for reproduction
Testing with z-fly and z-docker-opam examples, and I'm encountering build issues within just the docker compose up process even before running a request with the empty key-value header.
(* app.ml *)
let () =
Dream.run ~interface:"0.0.0.0"
@@ Dream.logger
@@ Dream.router [
Dream.get "/" (fun _ -> Dream.html ~headers:[("", "")] "Dream deployed on Fly.io!");
]
Local tests first on an m1 mac:
z-docker-opam local error with docker compose up
z-docker-opam-web-1 | Error loading shared library libssl.so.3: No such file or directory (needed by /bin/app)
z-docker-opam-web-1 | Error loading shared library libcrypto.so.3: No such file or directory (needed by /bin/app)
z-docker-opam-web-1 | Error relocating /bin/app: SSL_CTX_get_cert_store: symbol not found
z-docker-opam-web-1 | Error relocating /bin/app: SSL_load_client_CA_file: symbol not found
...I truncated because we get the picture
z-fly local error with docker compose up
> [build 5/12] RUN npm install esy:
#9 15.35 npm ERR! code 1
#9 15.35 npm ERR! path /build/node_modules/esy
#9 15.35 npm ERR! command failed
#9 15.35 npm ERR! command sh -c node -e "process.env['OCAML_VERSION'] = process.platform == 'linux' ? '4.12.0-musl.static.flambda': '4.12.0'; process.env['OCAML_PKG_NAME'] = 'ocaml'; require('./postinstall.js')"
#9 15.35 npm ERR! internal/fs/utils.js:269
#9 15.35 npm ERR! throw err;
#9 15.35 npm ERR! ^
#9 15.35 npm ERR!
#9 15.35 npm ERR! Error: ENOENT: no such file or directory, scandir '/build/node_modules/esy/platform-linux-arm64/bin'
To test a remote build, I've deployed both examples to fly.io.
z-docker-opam fails with the same missing missing ssl files
z-fly deploys successfully because it uses debian, and we finally get a failure on a request due to the missing header. Since fly proxies requests, they're already interrupting the request before it makes it back to the client.
2023-04-20T14:03:56.181 proxy[2c233da5] ewr [error] could not make HTTP request to instance: invalid HTTP header parsed
Unfortunate i'm unable to reproduce locally exactly what @pckilgore found, but we should still filter empty header keys regardless of linux image or whether it works locally because certain hosting platforms will block such http responses.
@dangdennis Are you able to test locally in just a regular Dream build, without Docker? Or is Docker required for OCaml on M1?
The output during container builds does look like additional other issues worth looking at separately, however.
@dangdennis Are you able to test locally in just a regular Dream build, without Docker? Or is Docker required for OCaml on M1?
For what it's worth, I'm using an M1 Mac with OCaml without docker and without any problems, but I run iTerm under Rosetta so my development environment is emulating Intel. Haven't done development at all for straight ARM.
All fine with local opam and dune, no docker.
@pckilgore did you not run into any ssl dep issues related to alpine?
Two options from what I can see
- We filter our
""headers at each of the response helpers, like at Dream.json line link - Add a middleware that always runs that cleans the headers to the middleware chain. I assume it's the
built_in_middleware
2 seems correct, and leaves the functionality all optional via ~builtins.
Option 2 is not possible anymore before we only run builtin middlewares before the users' middlewares, as well as before the user's dream handlers are called too.
Guess I'll just update Message.response
@pckilgore did you not run into any ssl dep issues related to alpine?
FROM node:16-alpine AS node
FROM ocaml/opam:alpine-ocaml-4.14 as build
# Grab Node & Friends
COPY --from=node /usr/lib /usr/lib
COPY --from=node /usr/local/share /usr/local/share
COPY --from=node /usr/local/lib /usr/local/lib
COPY --from=node /usr/local/include /usr/local/include
COPY --from=node /usr/local/bin /usr/local/bin
# Install system dependencies
RUN sudo apk add --update \
libev-dev \
openssl-dev \
sqlite-dev \
zlib-dev \
gmp-dev \
linux-headers
WORKDIR /home/opam
# Install dependencies
ADD blog.opam .
RUN opam install --deps-only --with-test .
RUN sudo npm install yarn --force --location=global
# Build project
ADD . .
RUN opam exec -- dune build @css
RUN opam exec -- dune build --profile=release -j 4
ADD https://github.com/benbjohnson/litestream/releases/download/v0.3.8/litestream-v0.3.8-linux-amd64-static.tar.gz /litestream.tar.gz
USER root
RUN ls -l / && chmod 0755 /litestream.tar.gz
RUN tar -C /bin -xzf /litestream.tar.gz
## Runtime
FROM alpine:3.12 as run
RUN apk add --update \
bash \
libev-dev \
openssl-dev \
sqlite-dev \
zlib-dev \
gmp-dev \
linux-headers
COPY --from=build /home/opam/posts /posts
COPY --from=build /home/opam/style.build.css /style.build.css
COPY --from=build /home/opam/_build/default/app.exe /bin/app
COPY --from=build /home/opam/entrypoint.sh /bin/entrypoint.sh
COPY --from=build /home/opam/litestream.yml /etc/litestream.yml
COPY --from=build /bin/litestream /bin/litestream
CMD ["bash", "/bin/entrypoint.sh"]
But this is deployed to Fly, so maybe the issue was never dokcer/arch? It's been so long since I filed this I don't really have the context anymore, sorry!
@dangdennis and @pckilgore, thanks for all the info!
I was able to reproduce this by modifying 2-middleware into this:
let () =
Dream.run
@@ Dream.logger
@@ fun _ ->
Dream.html ~headers:["", ""] "Good morning, world!"after that,
$ printf "GET / HTTP/1.1\r\nConnection: close\r\n\r\n" | nc localhost 8080
HTTP/1.1 200 OK
:
Content-Type: text/html; charset=utf-8
Content-Length: 20
Good morning, world!
So Dream does indeed send a line containing only :
This is not correct HTTP. Per RFC 7230 §3.2, a message header field-name is a token, and a token per RFC 7230 §3.2.6 is at least one character long. Chrome and HTTPie seem to be fine with this -- they both silently drop it and parse the rest of the response.
However, as I understand it, the issue is that some hosting services swallow up responses like this and don't emit them. If I understood correctly, @pckilgore, your Web app was setting these headers "unintentionally," and some of your time was spent diagnosing the problem.
If that's the case, I think it's best for Dream to warn about such headers, but maybe still emit them. @pckilgore, would that have helped you to deal with this issue efficiently in your development?
@dangdennis I think that Dream probably shouldn't drop "headers" with empty names like in #262. But regardless, even if there is an argument for automatically dropping them, it probably shouldn't do so silently, as this is some kind of programming error on the Dream user's part -- I think we need to help the user find their error quickly.
@aantron You tagged this with docs.
Is the idea to warn users of empty header keys as well as add some snippet related to this in the docs?
I think a warning is enough. Some of Dream's warnings are docs :) I don't think we need to document things that we can warn about, and don't have some other reason for documenting ahead of time. For little semi-obscure problems that can be detected by Dream a nice and clear warning is probably the best documentation.
@aantron Correct -- My application was wrong for setting the header, it just took a long time to figure out that was causing my issues.
A warning from Dream would have short-circuited debugging time on my end, either with or without other side effects like dropping the header.
Great, thanks @pckilgore and @dangdennis!