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.

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

Windows, Curl_socket_check function will incorrectly report an error on sockets that have data available to read. #10501

Closed
opensslonzos-github opened this issue Feb 13, 2023 · 4 comments
Labels
needs-info Windows Windows-specific

Comments

@opensslonzos-github
Copy link

On Windows, the Curl_socket_check function will incorrectly report an error on sockets that have data available to read.
This is because on Windows, the value for POLLIN is 768 and the value for POLLRDBAND is 512.
In the function Curl_socket_check in select.c, curl checks [pollfd.revents & POLLRDBAND] to determine whether the socket is broken. But, if pollfd.revents = POLLIN, pollfd.revents & POLLRDBAND != 0. This leads to many connections being wrongfully terminated.

The bug can be fixed by changing lines 233 and 240 in lib/select.c from
if(pfd[num].revents & (POLLRDBAND|POLLPRI|POLLNVAL))
to
if(pfd[num].revents & (POLLPRI|POLLNVAL)) // (removing the POLLRDBAND check).

Those lines check whether the socket is dead based on a call to poll.
If poll sets POLLIN, curl identifies that as an error because on Windows POLLIN = POLLRDNORM | POLLRDBAND.

curl/libcurl version

7.86

[curl -V output]
curl 7.86.0 (x86_64-pc-win32) libcurl/7.86.0 OpenSSL/1.1.1n zlib/1.2.12 WinIDN nghttp2/1.42.0
Release-Date: 2022-10-26
Protocols: http https smtp smtps
Features: alt-svc AsynchDNS HSTS HTTP2 HTTPS-proxy IDN IPv6 Largefile libz NTLM SSL threadsafe UnixSockets

operating system

Windows 10

@bagder bagder added the Windows Windows-specific label Feb 13, 2023
@bagder
Copy link
Member

bagder commented Feb 13, 2023

Can you describe or show the logs of a concrete use case/scenario of such a wrongfully terminated connection?

@opensslonzos-github
Copy link
Author

opensslonzos-github commented Feb 14, 2023

#include "include/curl/easy.h"
#include <iostream>
using namespace std;

extern "C" {
  static size_t debug_http_write_data(void* buffer, size_t size, size_t nmemb, void* userp) {
    size_t n_bytes = size * nmemb;
    return n_bytes;
  }

  static int my_trace(CURL *handle, curl_infotype type,
               char *data, size_t size,
               void *userp)
  {
    const char *text;
    int num = *(int*)userp;

    switch (type) {
    case CURLINFO_TEXT:
      cout << "== Info: [" << num << "] " << data;
      return 0;
    default:
      return 0;
    }
  }
}

const char* message = "12345,";

CURL* get_easy_handle(int* num) {
  CURL* handle = curl_easy_init();

  curl_easy_setopt(handle, CURLOPT_URL, "http://localhost:12345");

  curl_easy_setopt(handle, CURLOPT_POST, 1);
  curl_easy_setopt(handle, CURLOPT_POSTFIELDS, message);
  curl_easy_setopt(handle, CURLOPT_POSTFIELDSIZE, 6);

  curl_easy_setopt(handle, CURLOPT_WRITEFUNCTION, debug_http_write_data);

  curl_easy_setopt(handle, CURLOPT_DEBUGFUNCTION, my_trace);
  curl_easy_setopt(handle, CURLOPT_DEBUGDATA, num);
  curl_easy_setopt(handle, CURLOPT_VERBOSE, 1);

  curl_easy_setopt(handle, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_2);

  return handle;
}

int main (int argc, char **argv) {
  CURLM* multi_handle = curl_multi_init();

  curl_multi_setopt(multi_handle, CURLMOPT_PIPELINING, CURLPIPE_MULTIPLEX);

  int n1 = 1;
  int n2 = 2;
  int n3 = 3;
  int n4 = 4;
  int n5 = 5;
  int n6 = 6;
  int n7 = 7;
  int n8 = 8;
  int n9 = 9;
  int n10 = 10;
  int n11 = 11;
  CURL* handle1 = get_easy_handle(&n1);
  CURL* handle2 = get_easy_handle(&n2);
  CURL* handle3 = get_easy_handle(&n3);
  CURL* handle4 = get_easy_handle(&n4);
  CURL* handle5 = get_easy_handle(&n5);
  CURL* handle6 = get_easy_handle(&n6);
  CURL* handle7 = get_easy_handle(&n7);
  CURL* handle8 = get_easy_handle(&n8);
  CURL* handle9 = get_easy_handle(&n9);
  CURL* handle10 = get_easy_handle(&n10);
  CURL* handle11 = get_easy_handle(&n11);

  int still_running = 10;

  curl_multi_add_handle(multi_handle, handle1);
  curl_multi_add_handle(multi_handle, handle2);
  curl_multi_add_handle(multi_handle, handle3);
  curl_multi_add_handle(multi_handle, handle4);
  curl_multi_add_handle(multi_handle, handle5);
  curl_multi_add_handle(multi_handle, handle6);
  curl_multi_add_handle(multi_handle, handle7);
  curl_multi_add_handle(multi_handle, handle8);
  curl_multi_add_handle(multi_handle, handle9);
  curl_multi_add_handle(multi_handle, handle10);

  curl_multi_perform(multi_handle, &still_running);


  while (still_running > 9) {
    CURLMcode mcode = curl_multi_perform(multi_handle, &still_running);
    if (mcode != CURLM_OK && mcode != CURLM_CALL_MULTI_PERFORM) {
      cerr << "Bad multi perform";
      return 1;
    }
  }

  curl_multi_add_handle(multi_handle, handle11);

  still_running = 1;
  while (still_running > 0) {
    CURLMcode mcode = curl_multi_perform(multi_handle, &still_running);
    if (mcode != CURLM_OK && mcode != CURLM_CALL_MULTI_PERFORM) {
      cerr << "Bad multi perform";
      return 1;
    }
  }

  return 0;
}

@opensslonzos-github
Copy link
Author

== Info: [1] Trying 127.0.0.1:12345...
== Info: [1] Connected to localhost (127.0.0.1) port 12345 (#0)
== Info: [2] Found bundle for host: 0x16b2ce80160 [serially]
== Info: [2] Server doesn't support multiplex (yet)
== Info: [2] Hostname localhost was found in DNS cache
== Info: [2] Trying 127.0.0.1:12345...
== Info: [2] Connected to localhost (127.0.0.1) port 12345 (#1)
== Info: [3] Found bundle for host: 0x16b2ce80160 [serially]
== Info: [3] Server doesn't support multiplex (yet)
== Info: [3] Hostname localhost was found in DNS cache
== Info: [3] Trying 127.0.0.1:12345...
== Info: [3] Connected to localhost (127.0.0.1) port 12345 (#2)
== Info: [4] Found bundle for host: 0x16b2ce80160 [serially]
== Info: [4] Server doesn't support multiplex (yet)
== Info: [4] Hostname localhost was found in DNS cache
== Info: [4] Trying 127.0.0.1:12345...
== Info: [4] Connected to localhost (127.0.0.1) port 12345 (#3)
== Info: [5] Found bundle for host: 0x16b2ce80160 [serially]
== Info: [5] Server doesn't support multiplex (yet)
== Info: [5] Hostname localhost was found in DNS cache
== Info: [5] Trying 127.0.0.1:12345...
== Info: [5] Connected to localhost (127.0.0.1) port 12345 (#4)
== Info: [6] Found bundle for host: 0x16b2ce80160 [serially]
== Info: [6] Server doesn't support multiplex (yet)
== Info: [6] Hostname localhost was found in DNS cache
== Info: [6] Trying 127.0.0.1:12345...
== Info: [6] Connected to localhost (127.0.0.1) port 12345 (#5)
== Info: [7] Found bundle for host: 0x16b2ce80160 [serially]
== Info: [7] Server doesn't support multiplex (yet)
== Info: [7] Hostname localhost was found in DNS cache
== Info: [7] Trying 127.0.0.1:12345...
== Info: [7] Connected to localhost (127.0.0.1) port 12345 (#6)
== Info: [8] Found bundle for host: 0x16b2ce80160 [serially]
== Info: [8] Server doesn't support multiplex (yet)
== Info: [8] Hostname localhost was found in DNS cache
== Info: [8] Trying 127.0.0.1:12345...
== Info: [8] Connected to localhost (127.0.0.1) port 12345 (#7)
== Info: [9] Found bundle for host: 0x16b2ce80160 [serially]
== Info: [9] Server doesn't support multiplex (yet)
== Info: [9] Hostname localhost was found in DNS cache
== Info: [9] Trying 127.0.0.1:12345...
== Info: [9] Connected to localhost (127.0.0.1) port 12345 (#8)
== Info: [10] Found bundle for host: 0x16b2ce80160 [serially]
== Info: [10] Server doesn't support multiplex (yet)
== Info: [10] Hostname localhost was found in DNS cache
== Info: [10] Trying 127.0.0.1:12345...
== Info: [10] Connected to localhost (127.0.0.1) port 12345 (#9)
revents=768
== Info: [1] Mark bundle as not supporting multiuse
== Info: [1] Received 101, Switching to HTTP/2
== Info: [1] Using HTTP2, server supports multiplexing
== Info: [1] Copying HTTP/2 data in stream buffer to connection buffer after upgrade: len=50
== Info: [1] Connection state changed (MAX_CONCURRENT_STREAMS == 4294967295)!
== Info: [1] Connection #0 to host localhost left intact
revents=768
== Info: [2] Mark bundle as not supporting multiuse
== Info: [2] Received 101, Switching to HTTP/2
== Info: [2] Using HTTP2, server supports multiplexing
== Info: [2] Copying HTTP/2 data in stream buffer to connection buffer after upgrade: len=21
== Info: [2] Connection state changed (MAX_CONCURRENT_STREAMS == 4294967295)!
== Info: [11] Found bundle for host: 0x16b2ce80160 [can multiplex]
revents=768
== Info: [11] Connection 0 seems to be dead
== Info: [11] Closing connection 0
== Info: [11] Multiplexed connection found
== Info: [11] Re-using existing connection #1 with host localhost

@opensslonzos-github
Copy link
Author

I modified select.c to print the revents that Curl_poll sets in Curl_socket_check? There are some extra lines in debug.txt that would be confusing if they don’t know that.

bagder added a commit that referenced this issue Feb 23, 2023
POLLRDBAND does not seem to be an general error and on Windows, the
value for POLLIN is 768 and the value for POLLRDBAND is 512.

Fixes #10501
Reported-by: opensslonzos-github on github
Closes #10592
@bagder bagder closed this as completed in 0242eba Feb 23, 2023
bch pushed a commit to bch/curl that referenced this issue Jul 19, 2023
POLLRDBAND does not seem to be an general error and on Windows the value
for POLLIN is 768 and the value for POLLRDBAND is 512.

Fixes curl#10501
Reported-by: opensslonzos-github on github
Closes curl#10592
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-info Windows Windows-specific
Development

Successfully merging a pull request may close this issue.

2 participants