Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: uncaught exception due to second response with digest auth #530

Merged
merged 1 commit into from
Sep 14, 2024
Merged

fix: uncaught exception due to second response with digest auth #530

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/HttpClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,8 @@ export class HttpClient extends EventEmitter {
// FIXME: merge exists cookie header
requestOptions.headers.cookie = response.headers['set-cookie'].join(';');
}
// Ensure the previous response is consumed as we re-use the same variable
await response.body.arrayBuffer();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@damiengrandi Please add an associated test case for this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, are you asking for an example to reproduce this issue? With an example URL and additional details?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, a test example to reproduce the problem.

Copy link
Contributor Author

@damiengrandi damiengrandi Sep 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In some specific cases where APIs are slow to respond and DigestAuth is used, an uncaught UND_ERR_BODY_TIMEOUT exception can occur. Below is a source code example illustrating this issue:

import { request, RequestOptions } from 'urllib';

try {
  const username = "someUsername";
  const password = "somePassword";
  const deviceHost = "123.456.789.123";
  const deviceWebPort = 80;
  const path = "/axis-cgi/com/ptz.cgi?camera=1&query=position";

  const requestUrl = `http://${deviceHost}:${deviceWebPort}${path}`;

  const requestOptions: RequestOptions = {
    digestAuth: `${username}:${password}`,
    method: 'GET',
    timeout: [1000, 1], // 1ms to force UND_ERR_BODY_TIMEOUT to happen more often for testing purpose
  };

  // This line may provoke an uncaught exception in some cases
  const res = await request(requestUrl, requestOptions);

  const data = res.data?.toString();
  console.log(data);
} catch (error) {
  console.error(error);
}

I’ve observed this issue when querying PTZ position from an AXIS camera. The device can be slow to respond, which makes it a good candidate for testing this issue. You can find more details on the relevant endpoint in AXIS's documentation here: https://www.axis.com/vapix-library/subjects/t10175981/section/t10036011/display?section=t10036011-t10004639

Unfortunately, due to GDPR restrictions, I can't provide a camera with dummy credentials.
However, you may reproduce the issue with other devices that use DigestAuth and are slow to respond to API requests.

Copy link
Contributor Author

@damiengrandi damiengrandi Sep 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found a way for you to reproduce in local.
Using the following PHP script to simulate slow response during the first DigestAuth request :

<?php

function sendDigestAuthChallenge() {
    $realm = 'Restricted Area';
    $nonce = uniqid();
    header('HTTP/1.1 401 Unauthorized');
    header('WWW-Authenticate: Digest realm="' . $realm . '",qop="auth",nonce="' . $nonce . '",opaque="' . md5($realm) . '"');

    // Simulate slow response time
    for ($i = 0; $i < 10000000; $i++) {
        echo ".";
    }

    echo "Authentication required";
    exit;
}

if (empty($_SERVER['PHP_AUTH_DIGEST'])) {
    sendDigestAuthChallenge();
}
echo "OK";

And using this code to test it from NodeJS:

do {
  try {
    console.log('HERE');
    const host = 'http://localhost/test_digest/test.php';
    const options: RequestOptions = {
      digestAuth: 'root:root',
      method: 'GET',
      timeout: [3000, 1],
    };
    const response = await request(host, options);
    console.log(response.data?.toString());
  } catch (error) {
    console.error(error);
  }

  // Wait 1 sec
  await new Promise((resolve) => setTimeout(resolve, 1000));
} while (true);

I got the crash very quickly:
image

response = await undiciRequest(requestUrl, requestOptions as UndiciRequestOption);
}
}
Expand Down
Loading