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

Jump to bottom

Save altsvc cache in curl_easy_reset #4898

Closed
candrews opened this issue Feb 9, 2020 · 2 comments · Fixed by sthagen/curl-curl#77
Closed

Save altsvc cache in curl_easy_reset #4898

candrews opened this issue Feb 9, 2020 · 2 comments · Fixed by sthagen/curl-curl#77
Labels
HTTP libcurl API

Comments

@candrews
Copy link

candrews commented Feb 9, 2020

I did this

I expect this block to save the altsvc cache to "altsvc.txt" but it does not.
When the curl_easy_reset line (noted with a comment) is removed, "altsvc.txt" is saved as expected.

/********* Sample code generated by the curl command line tool **********
 * All curl_easy_setopt() options are documented at:
 * https://curl.haxx.se/libcurl/c/curl_easy_setopt.html
 ************************************************************************/
#include <curl/curl.h>

int main(int argc, char *argv[])
{ 
  CURLcode ret;
  CURL *hnd;

  hnd = curl_easy_init();
  curl_easy_setopt(hnd, CURLOPT_BUFFERSIZE, 102400L);
  curl_easy_setopt(hnd, CURLOPT_URL, "https://integralblue.com");
  curl_easy_setopt(hnd, CURLOPT_NOPROGRESS, 1L);
  curl_easy_setopt(hnd, CURLOPT_USERAGENT, "curl/7.68.0");
  curl_easy_setopt(hnd, CURLOPT_MAXREDIRS, 50L);
  curl_easy_setopt(hnd, CURLOPT_HTTP_VERSION, (long)CURL_HTTP_VERSION_2TLS);
  curl_easy_setopt(hnd, CURLOPT_TCP_KEEPALIVE, 1L);
  curl_easy_setopt(hnd, CURLOPT_ALTSVC, "altsvc.txt");

  /* Here is a list of options the curl code used that cannot get generated
     as source easily. You may select to either not use them or implement
     them yourself.

  CURLOPT_WRITEDATA set to a objectpointer
  CURLOPT_INTERLEAVEDATA set to a objectpointer
  CURLOPT_WRITEFUNCTION set to a functionpointer
  CURLOPT_READDATA set to a objectpointer
  CURLOPT_READFUNCTION set to a functionpointer
  CURLOPT_SEEKDATA set to a objectpointer
  CURLOPT_SEEKFUNCTION set to a functionpointer
  CURLOPT_ERRORBUFFER set to a objectpointer
  CURLOPT_STDERR set to a objectpointer
  CURLOPT_HEADERFUNCTION set to a functionpointer
  CURLOPT_HEADERDATA set to a objectpointer

  */

  ret = curl_easy_perform(hnd);

  curl_easy_reset(hnd); // line added
  curl_easy_cleanup(hnd);
  hnd = NULL;

  return (int)ret;
}
/**** End of sample code ****/

Currently, the alt svc cache is only saved in curl_close which is called by curl_easy_cleanup:
https://github.com/curl/curl/blob/curl-7_68_0/lib/url.c#L383
https://github.com/curl/curl/blob/curl-7_68_0/lib/easy.c#L738

It should also be saved when curl_easy_reset is called.

I believe that adding

#ifdef USE_ALTSVC
  Curl_altsvc_save(data->asi, data->set.str[STRING_ALTSVC]);
#endif

at https://github.com/curl/curl/blob/curl-7_68_0/lib/easy.c#L941 would address this issue.

I expected the following

curl_easy_reset should save the altsvc cache file.

curl/libcurl version

$ curl --version
curl 7.68.0 (x86_64-pc-linux-gnu) libcurl/7.68.0 OpenSSL/1.1.1d zlib/1.2.11 brotli/1.0.7 libidn2/2.3.0 libssh2/1.9.0_DEV nghttp2/1.40.0 quiche/0.2.0 librtmp/2.3
Release-Date: 2020-01-08
Protocols: dict file ftp ftps gopher http https imap imaps pop3 pop3s rtmp rtsp scp sftp smtp smtps telnet tftp 
Features: alt-svc AsynchDNS brotli HTTP2 HTTP3 HTTPS-proxy IDN IPv6 Largefile libz Metalink NTLM SSL TLS-SRP UnixSockets

operating system

$ uname -a
Linux irrational 5.4.0-rc6 #2 SMP Fri Nov 8 19:09:49 EST 2019 x86_64 Intel(R) Core(TM) i5-6500 CPU @ 3.20GHz GenuineIntel GNU/Linux
@bagder
Copy link
Member

bagder commented Feb 9, 2020

I don't think it should save the file in the reset call, but the reset should not impact the alt-svc cache so it should then be saved correctly by curl_easy_cleanup(). I take it you're saying that's not what's happening?

@bagder
Copy link
Member

bagder commented Feb 9, 2020

Ah, the reason it doesn't save the alt-svc cache is because the reset cleared the file name so it doesn't know where to store them any longer...

bagder added a commit that referenced this issue Feb 9, 2020
@bagder
altsvc: keep a copy of the file name to survive handle reset
3b7b972 
The alt-svc cache survives a call to curl_easy_reset fine, but the file
name to use for saving the cache was cleared. Now the alt-svc cache has
a copy of the file name to survive handle resets.

Added test 1908 to verify.

Reported-by: Craig Andrews
Fixes #4898
@bagder bagder mentioned this issue Feb 9, 2020
Closed
@bagder bagder closed this as completed in 02f8de6 Feb 9, 2020
@sthagen sthagen mentioned this issue Feb 9, 2020
Merged
@lock lock bot locked as resolved and limited conversation to collaborators May 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
HTTP libcurl API
Milestone
No milestone
2 participants
@bagder @candrews