-
-
Notifications
You must be signed in to change notification settings - Fork 220
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Browse files
Browse the repository at this point in the history
Node 19 adds keep-alive header to requests by default. This makes the content-length header very significant and important because it enables the server to know when one request has been fully received and another one is sent through the same connection. When authenticating a user, we take all headers from the original user's request and pass them to a GET /_session request, presumably so we don't have to filter out all the ways through which the user can authenticate (cookie, authorization etc). This meant that if the original request was a POST, and it had a content-length header, this header was also forwarded. This meant that when the connection would later be reused, the server (haproxy in this case) would truncate the previous's request content-length number of characters from the next request, believing the previous request was not finished. Obviously, this new truncated request was invalid and generated a 400 status code. This only seems to happen when accessing API directly or through the AWS load balancer, and never when running requests through nginx. To avoid next request truncation, I am no longer forwarding content-length headers in the session request. #8868
- Loading branch information
1 parent
d0c7bc8
commit 831bd6e
Showing
6 changed files
with
184 additions
and
14 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| FROM alpine:3.19 | ||
|
|
||
| RUN apk add --update --no-cache curl | ||
|
|
||
| COPY cmd.sh / | ||
| RUN chmod +x /cmd.sh | ||
|
|
||
| CMD ["/cmd.sh"] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| #!/bin/sh | ||
|
|
||
| set -e | ||
|
|
||
| data='{"user":"'$USER'","password":"'$PASSWORD'"}' | ||
|
|
||
| curl -v POST 'http://api:5988/medic/login' -d $data -H 'Content-Type:application/json' | ||
|
|
15 changes: 15 additions & 0 deletions
15
tests/integration/haproxy/keep-alive-script/docker-compose.yml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| version: '3' | ||
|
|
||
| services: | ||
| login_call: | ||
| build: . | ||
| networks: | ||
| net: | ||
| environment: | ||
| - USER | ||
| - PASSWORD | ||
|
|
||
| networks: | ||
| net: | ||
| name: cht-net-e2e | ||
| external: true |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| const { spawn } = require('child_process'); | ||
| const path = require('path'); | ||
| const constants = require('@constants'); | ||
|
|
||
| const runDockerCommand = (command, params, env=process.env) => { | ||
| return new Promise((resolve, reject) => { | ||
| const cmd = spawn(command, params, { cwd: path.join(__dirname, 'keep-alive-script'), env }); | ||
| const output = []; | ||
| const log = (data) => output.push(data.toString()); | ||
| cmd.on('error', reject); | ||
| cmd.stdout.on('data', log); | ||
| cmd.stderr.on('data', log); | ||
| cmd.on('close', () => resolve(output.join(' '))); | ||
| }); | ||
| }; | ||
|
|
||
| const runScript = async () => { | ||
| const env = { ...process.env }; | ||
| env.USER = constants.USERNAME; | ||
| env.PASSWORD = constants.PASSWORD; | ||
| return await runDockerCommand('docker-compose', ['up', '--build', '--force-recreate'], env); | ||
| }; | ||
| const getLogs = async () => { | ||
| return await runDockerCommand('docker-compose', ['logs', '--no-log-prefix']); | ||
| }; | ||
|
|
||
| describe('logging in through API directly', () => { | ||
| after(async () => { | ||
| await runDockerCommand('docker-compose', ['down', '--remove-orphans']); | ||
| }); | ||
|
|
||
| it('should allow logins', async () => { | ||
| await runScript(); | ||
| const logs = await getLogs(); | ||
|
|
||
| console.log(logs); | ||
|
|
||
| expect(logs).to.not.include('HTTP/1.1 400 Bad Request'); | ||
|
|
||
| expect(logs).to.include('HTTP/1.1 302 Found'); | ||
| expect(logs).to.include('Connection: keep-alive'); | ||
| expect(logs).to.include('Set-Cookie: AuthSession='); | ||
| expect(logs).to.include('Set-Cookie: userCtx='); | ||
| }); | ||
| }); |