Code review comment for ~bryce/ubuntu/+source/nginx:fix-lp1981457-jammy

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

+1

patch is ok, headers are ok (bug is against openssl, not nginx, but it has the info we need)

In the changelog, I just wouldn't say "older clients". It's any client really that misbehaves. So perhaps just say "for clients that don't close ...". Totally up to you.

I was able to confirm the fix even, with the test case from the SRU.

I would just suggest to clarify that a bit. Here is what I did:

- install nginx-full and ssl-cert
- edit /etc/nginx/sites-enabled/default and uncomment both "listen" lines for 443, and the "include" line for snakeoil.conf
- restart nginx
- run the script:
#!/bin/bash
URL="https://localhost"
while :; do
  timeout -s KILL 0.2s curl -v -k -K <(echo verbose;for i in {1..2000}; do echo url = "$URL"; echo -o /dev/null; done)
done

- in another terminal, tail -f /var/log/nginx/error.log and observe tons of SSL_READ() errors.
- with the updated package, these errors are gone

review: Approve

« Back to merge proposal