SSLApp and chunked encoding malformed

[porsager]

When using SSL and sending chunked the response will sometimes be malformed - mostly when there's backpressure.

I've attached a small sample that shows a repro of the issue when running bash run.sh.

uws-ssl-chunked-bug.zip

It's just the code below with self signed cert, file and a bash.run script that uses curl to show we end up with a curl: (56) Malformed encoding found in chunked-encoding error.

I know this sample could easily be changed to use tryEnd, but the point is showing the chunked issue.

import uws from 'uWebSockets.js'
import fs from 'fs'

const https = uws.SSLApp({ cert_file_name: 'localhost.cert', key_file_name: 'localhost.key' })
https.get('/*', handler)
https.listen(8443, () => console.log('https on 8443'))

const http = uws.App()
http.get('/*', handler)
http.listen(8000, () => console.log('http on 8000'))

function handler(res) {
  const stream = fs.createReadStream('100mb.txt')

  res.onAborted(() => {
    res.aborted = true
    stream.removeAllListeners()
  })

  stream.on('data', x => {
    res.cork(() => {
      const ok = res.write(x)
      if (ok)
        return

      stream.pause()
      res.onWritable(() => {
        stream.resume()
        return true
      })
    })
  })

  stream.on('end', () => res.cork(() => res.end()))
}

[uNetworkingAB]

It's a good test, I can reproduce. Something like this should be run in CI. You don't see this for tryEnd? And it works for non-SSL?

[uNetworkingAB]

If I remove the res.cork it works with SSL, so something is wrong there

[porsager]

I haven't been able to reproduce it with tryEnd, but I've had cases in prod where I couldn't locate an issue with similar symptoms, so I can't rule it out, but it's definately not as prevalent.

And yeah, I've only been able to reproduce with SSL. As seen in the sample, no SSL works fine

[porsager]

If I remove the res.cork it works with SSL, so something is wrong there

Oh - interesting. I'll give it a try, even though it's not the intended way!

[uNetworkingAB]

Nah that's not it. It happens whenever pause triggers, but you can replace onWritable with any setTimeout and it still happens. Something is off, maybe it is something with the data chunk given, maybe the lib does not properly start from byteOffset or something like that, really odd issue. Must reproduce it in raw C++ first

[uNetworkingAB]

This also triggers it without files:

import uws from 'uWebSockets.js'
import fs from 'fs'

const https = uws.SSLApp({ cert_file_name: 'localhost.cert', key_file_name: 'localhost.key' })
https.get('/*', handler)
https.listen(8443, () => console.log('https on 8443'))

const http = uws.App()
http.get('/*', handler)
http.listen(8000, () => console.log('http on 8000'))

function streamData(res, chunk) {

  if (res.aborted) {
    return;
  }

  if (chunk < 1600) {
    res.cork(() => {
      let ok = res.write(new Uint8Array(65536).fill(97));

      if (ok) {
        setImmediate(() => {
          streamData(res, chunk + 1);
        });
        return;
      }

      setTimeout(() => {
        streamData(res, chunk + 1);
      }, 50);
    });

  } else {
    res.cork(() => {
      res.end();
    });
  }

}

function handler(res) {
  streamData(res, 0);

  res.onAborted(() => {
    res.aborted = true;
  })
}

[uNetworkingAB]

Did this happen only with a recent version or always?

[uNetworkingAB]

This triggers the issue almost 100% of the time:

import uws from 'uWebSockets.js'
import fs from 'fs'

const https = uws.SSLApp({ cert_file_name: 'localhost.cert', key_file_name: 'localhost.key' })
https.get('/*', handler)
https.listen(8443, () => console.log('https on 8443'))

const http = uws.App()
http.get('/*', handler)
http.listen(8000, () => console.log('http on 8000'))

function streamData(res, chunk) {

  if (res.aborted) {
    return;
  }

  if (chunk < 1600) {
    res.cork(() => {
      let ok = res.write(new Uint8Array(65536).fill(97));
      if (ok) {
        streamData(res, chunk + 1);
        return;
      }

      setTimeout(() => {
        streamData(res, chunk + 1);
      }, 1);
    });
  } else {
    res.cork(() => {
      res.end();
    });
  }
}

function handler(res) {
  streamData(res, 0);

  res.onAborted(() => {
    res.aborted = true;
  })
}

[uNetworkingAB]

This C++ example triggers it also: https://github.com/uNetworking/uWebSockets/commit/96814394152f6491ad74898f0ea0f7ca2f4ef2de

[uNetworkingAB]

This seems to fix it: https://github.com/uNetworking/uWebSockets/commit/1cc86054902a25c91edfe50bc4b9a5590c175c33

That's the only difference between SSL and non-SSL. You are hitting a case that has been untested for a long time, very few seem to use the SSL support in uWS and use proxies.

[uNetworkingAB]

Whenever latest commit builds, can you test it again to make sure?

npm install uNetworking/uWebSockets.js#binaries

[uNetworkingAB]

Yes it is fixed now, I can't trigger it anymore

[porsager]

This seems to fix it: uNetworking/uWebSockets@1cc8605

Fantastic! Can't repro here anymore - Do you think it's worth investigating if the optimized path for SSL can actually get to work?

That's the only difference between SSL and non-SSL. You are hitting a case that has been untested for a long time, very few seem to use the SSL support in uWS and use proxies.

Or perhaps they're accepting the odd error here and there, if they're even doing chunked transfer at all. Can only speak for myself, but it's such a breeze having less moving parts and simply expose uws directly.

Thanks a lot for the quick fix!

[uNetworkingAB]

Thanks for the stellar reproducer. I have to go back to basics on that SSL special case and figure out what the heck I was trying to do