Closed Bug 1631341 Opened 4 years ago Closed 2 years ago

Random form body loss for concurrent HTTP/2.0 POST requests

Categories

(Web Compatibility :: Site Reports, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: danny0838, Unassigned)

Details

(Keywords: webcompat:site-wait, Whiteboard: [necko-triaged])

Attachments

(1 file)

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:75.0) Gecko/20100101 Firefox/75.0

Steps to reproduce:

Send many HTTP/2.0 POST requests at once. Here's a sample code:

async function postRequests(urlPrefix, num = 500, minKB = 0.5, maxKB = 15) {
function getRandomString(length) {
const possible = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz012345678-_';
const array = new Uint8Array(length);
return [].map.call(crypto.getRandomValues(array), x => possible[x % 64]).join('');
}

function getRandomInt(min, max) {
min = Math.ceil(min);
max = Math.floor(max);
return Math.floor(Math.random() * (max - min)) + min; //The maximum is exclusive and the minimum is inclusive
}

const requests = new Array(num)
.fill(0)
.map(x => ({
name: getRandomString(8) + '.html',
data: new Blob([getRandomString(getRandomInt(minKB * 1024, maxKB * 1024))], {type: 'text/html'}),
}));

return await Promise.all(requests.map(async ({name, data}) => {
const token = await fetch(
${urlPrefix}/${name}?a=token,
{
method: 'GET',
credentials: 'include',
cache: 'no-cache',
}).then(r => r.text());

const formData = new FormData();
formData.append('token', token);
formData.append('upload', data);

return await fetch(
  `${urlPrefix}/${name}?a=save&f=json`,
  {
    method: 'POST',
    credentials: 'include',
    cache: 'no-cache',
    body: formData,
  });

}));
}

postRequests('https://example.com/app', 500, 0.5, 15)
.then(r => console.warn(r.map(x => x.status)));

Actual results:

The server randomly receives no form body for some of the requests. This never happens in Chrome or when the requests are sent via HTTP/1.1

Expected results:

The server should receive form body for all requests.

Fix the sample script:

async function postRequests(urlPrefix, num = 500, minKB = 0.5, maxKB = 15) {
  function getRandomString(length) {
    const possible = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz012345678-_';
    const array = new Uint8Array(length);
    return [].map.call(crypto.getRandomValues(array), x => possible[x % 64]).join('');
  }

  function getRandomInt(min, max) {
    min = Math.ceil(min);
    max = Math.floor(max);
    return Math.floor(Math.random() * (max - min)) + min; //The maximum is exclusive and the minimum is inclusive
  }

  const requests = new Array(num)
      .fill(0)
      .map(x => ({
        name: getRandomString(8) + '.html',
        data: new Blob([getRandomString(getRandomInt(minKB * 1024, maxKB * 1024))], {type: 'text/html'}),
      }));

  return await Promise.all(requests.map(async ({name, data}) => {
    const token = await fetch(
      `${urlPrefix}/${name}?a=token`,
      {
        method: 'GET',
        credentials: 'include',
        cache: 'no-cache',
      }).then(r => r.text());

    const formData = new FormData();
    formData.append('token', token);
    formData.append('upload', data);

    return await fetch(
      `${urlPrefix}/${name}?a=save&f=json`,
      {
        method: 'POST',
        credentials: 'include',
        cache: 'no-cache',
        body: formData,
      });
  }));
}

postRequests('https://example.com/app', 500, 0.5, 15)
  .then(r => console.warn(r.map(x => x.status)));

Additional information: in my case the server is a Python APP run behind a reverse proxy using nginx. Not sure whether it's related or not.

Component: Untriaged → Networking: HTTP
Product: Firefox → Core

I cannot reproduce it locally with apache. Can you try to run the test without nginx to see if it's needed to reproduce the problem?

Flags: needinfo?(danny0838)

Not reproduced using hypercorn - asgiref.wsgi.WsgiToAsgi - same Python WSGI application.

After increasing the requests to postRequests('https://127.0.0.1:8080/http2-bug/', 1000, 1, 40), still not reproduced.

Maybe nginx is somehow related with this issue...

Flags: needinfo?(danny0838)

I can reproduce it with nginx reverse proxy. I'll investigate it whether it's a bug in Firefox or nginx.

Assignee: nobody → michal.novotny
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Priority: -- → P1
Whiteboard: [necko-triaged]

(In reply to Michal Novotny [:michal] from comment #5)

I can reproduce it with nginx reverse proxy. I'll investigate it whether it's a bug in Firefox or nginx.

Can we check if this is problem on our side, there is a lot of request.
I would leave it as p1.

Attached file dumps.tar.xz

I've modified the test so the data is easier to follow in Wireshark. The pcap dumps contain 30 requests where data of request "/bug1631341/test/xxx_023.html" is corrupted. The pcap dump between Firefox and Nginx (dump_ff.pcap) doesn't contain garbled data, whereas the data is garbled between Nginx and Apache (dump_apache.pcap). I don't know why this cannot be reproduced with Chrome, but I don't see anything strange in dump_ff.pcap. I'm closing the bug as invalid, because this seems to be a bug in Nginx.

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → INVALID

Nhi, do you know who we can ping to contact nginx or file a bug on their for this issue? See comment 7.

Flags: needinfo?(nhnguyen)

Yeah, we should get this bug filed upstream.

Michael, which version of nginx were you able to reproduce on? And do you have a config you can share?

Actually, the bug report form wants the following:

nginx -V
uname -a

Status: RESOLVED → REOPENED
Component: Networking: HTTP → Desktop
Flags: needinfo?(michal.novotny)
Product: Core → Web Compatibility
Resolution: INVALID → ---
Version: 75 Branch → unspecified
Priority: P1 → P2
Flags: needinfo?(nhnguyen)

(In reply to Mike Taylor [:miketaylr] from comment #9)

Yeah, we should get this bug filed upstream.

Michael, which version of nginx were you able to reproduce on? And do you have a config you can share?

This is a content of /etc/nginx/sites-enabled/reverse-proxy-ssl.conf

server {
  listen 443 ssl http2;
  listen [::]:443 ssl http2;

  ssl_certificate /etc/ssl/certs/nginx-selfsigned.crt;
  ssl_certificate_key /etc/ssl/private/nginx-selfsigned.key;

  access_log /var/log/nginx/reverse-access-ssl.log;
  error_log /var/log/nginx/reverse-error-ssl.log;

  location / {
    proxy_pass https://192.168.192.10;
    proxy_ssl_certificate     /etc/ssl/certs/nginx-selfsigned.crt;
    proxy_ssl_certificate_key /etc/ssl/private/nginx-selfsigned.key;
  }
}

Actually, the bug report form wants the following:

nginx -V

nginx version: nginx/1.10.3
built with OpenSSL 1.1.0f  25 May 2017 (running with OpenSSL 1.1.0j  20 Nov 2018)
TLS SNI support enabled
configure arguments: --with-cc-opt='-g -O2 -fdebug-prefix-map=/build/nginx-0TiIP5/nginx-1.10.3=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2' --with-ld-opt='-Wl,-z,relro -Wl,-z,now' --prefix=/usr/share/nginx --conf-path=/etc/nginx/nginx.conf --http-log-path=/var/log/nginx/access.log --error-log-path=/var/log/nginx/error.log --lock-path=/var/lock/nginx.lock --pid-path=/run/nginx.pid --modules-path=/usr/lib/nginx/modules --http-client-body-temp-path=/var/lib/nginx/body --http-fastcgi-temp-path=/var/lib/nginx/fastcgi --http-proxy-temp-path=/var/lib/nginx/proxy --http-scgi-temp-path=/var/lib/nginx/scgi --http-uwsgi-temp-path=/var/lib/nginx/uwsgi --with-debug --with-pcre-jit --with-ipv6 --with-http_ssl_module --with-http_stub_status_module --with-http_realip_module --with-http_auth_request_module --with-http_v2_module --with-http_dav_module --with-http_slice_module --with-threads --with-http_addition_module --with-http_geoip_module=dynamic --with-http_gunzip_module --with-http_gzip_static_module --with-http_image_filter_module=dynamic --with-http_sub_module --with-http_xslt_module=dynamic --with-stream=dynamic --with-stream_ssl_module --with-mail=dynamic --with-mail_ssl_module --add-dynamic-module=/build/nginx-0TiIP5/nginx-1.10.3/debian/modules/nginx-auth-pam --add-dynamic-module=/build/nginx-0TiIP5/nginx-1.10.3/debian/modules/nginx-dav-ext-module --add-dynamic-module=/build/nginx-0TiIP5/nginx-1.10.3/debian/modules/nginx-echo --add-dynamic-module=/build/nginx-0TiIP5/nginx-1.10.3/debian/modules/nginx-upstream-fair --add-dynamic-module=/build/nginx-0TiIP5/nginx-1.10.3/debian/modules/ngx_http_substitutions_filter_module

uname -a

Linux debian 4.20.5-200.fc29.x86_64 #1 SMP Mon Jan 28 19:29:17 UTC 2019 x86_64 GNU/Linux
Flags: needinfo?(michal.novotny)

Thanks. 1.10 is old-ish, I wonder if it reproduces in newer versions.

Danny, which version of nginx are you able to reproduce on?

Flags: needinfo?(danny0838)

(In reply to Mike Taylor [:miketaylr] from comment #11)

Thanks. 1.10 is old-ish, I wonder if it reproduces in newer versions.

Danny, which version of nginx are you able to reproduce on?

I had 1.10.3, too. It's the latest version in Ubuntu 16 package.

Flags: needinfo?(danny0838)

Seems to be an issue in older nginx. Resolved after upgrading to latest nginx.

The bug assignee didn't login in Bugzilla in the last 7 months.
:karlcow, could you have a look please?
For more information, please visit auto_nag documentation.

Assignee: michal.novotny → nobody
Flags: needinfo?(kdubost)

This was fixed in nginx. See their response to the ticket.
https://trac.nginx.org/nginx/ticket/1973

Flags: needinfo?(kdubost)
Status: REOPENED → RESOLVED
Closed: 4 years ago2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: