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

Add http_set_curlopt facility for setting strange curl options. #44

Merged
merged 5 commits into from Jun 21, 2017

Conversation

Projects
None yet
2 participants
@pramsey
Owner

pramsey commented Jun 16, 2017

No description provided.

@robe2

This comment has been minimized.

Show comment
Hide comment
@robe2

robe2 Jun 20, 2017

Contributor

This doesn't compile for me. I get this error. I confirmed current head compiles and installs fine for me, but not after I apply this patch. I have CURL 7.44.0 in case that matters.

`
http.c:117:28: error: 'CURLOPT_PRE_PROXY' undeclared here (not in a function)
{ "CURLOPT_PRE_PROXY", CURLOPT_PRE_PROXY, CURLOPT_STRING, false },
^
http.c:124:41: error: 'CURLOPT_PROXY_TLSAUTH_USERNAME' undeclared here (not in a function)
{ "CURLOPT_PROXY_TLSAUTH_USERNAME", CURLOPT_PROXY_TLSAUTH_USERNAME, CURLOPT_STRING, false },
^
http.c:125:41: error: 'CURLOPT_PROXY_TLSAUTH_PASSWORD' undeclared here (not in a function)
{ "CURLOPT_PROXY_TLSAUTH_PASSWORD", CURLOPT_PROXY_TLSAUTH_PASSWORD, CURLOPT_STRING, false },
^
http.c:127:37: error: 'CURLOPT_PROXY_TLSAUTH_TYPE' undeclared here (not in a function)
{ "CURLOPT_PROXY_TLSAUTH_TYPE", CURLOPT_PROXY_TLSAUTH_TYPE, CURLOPT_STRING, false },
^
: recipe for target 'http.o' failed
make: *** [http.o] Error 1
gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -g -O2 -I/projects/curl/rel-curl-7.44.0w64gcc48/include -I/projects/pcre/rel-8.33w64gcc48/include -I/projects/libxml/rel-libxml2-2.7.8w64gcc48/include/libxml2 -I. -I./ -IC:/MING641/projects/POSTGR1/rel/PG10W62/include/server -IC:/MING641/projects/POSTGR1/rel/PG10W62/include/internal -I./src/include/port/win32 -DEXEC_BACKEND -I/projects/libxml/rel-libxml2-2.7.8w64gcc48/include/libxml2 -IC:/MING641/projects/POSTGR1/rel/PG10W6~2/include/server/port/win32 -c -o http.o http.c
http.c:117:28: error: 'CURLOPT_PRE_PROXY' undeclared here (not in a function)
{ "CURLOPT_PRE_PROXY", CURLOPT_PRE_PROXY, CURLOPT_STRING, false },
^
http.c:124:41: error: 'CURLOPT_PROXY_TLSAUTH_USERNAME' undeclared here (not in a function)
{ "CURLOPT_PROXY_TLSAUTH_USERNAME", CURLOPT_PROXY_TLSAUTH_USERNAME, CURLOPT_STRING, false },
^
http.c:125:41: error: 'CURLOPT_PROXY_TLSAUTH_PASSWORD' undeclared here (not in a function)
{ "CURLOPT_PROXY_TLSAUTH_PASSWORD", CURLOPT_PROXY_TLSAUTH_PASSWORD, CURLOPT_STRING, false },
^
http.c:127:37: error: 'CURLOPT_PROXY_TLSAUTH_TYPE' undeclared here (not in a function)
{ "CURLOPT_PROXY_TLSAUTH_TYPE", CURLOPT_PROXY_TLSAUTH_TYPE, CURLOPT_STRING, false },
^
: recipe for target 'http.o' failed
make: *** [http.o] Error 1

`

Contributor

robe2 commented Jun 20, 2017

This doesn't compile for me. I get this error. I confirmed current head compiles and installs fine for me, but not after I apply this patch. I have CURL 7.44.0 in case that matters.

`
http.c:117:28: error: 'CURLOPT_PRE_PROXY' undeclared here (not in a function)
{ "CURLOPT_PRE_PROXY", CURLOPT_PRE_PROXY, CURLOPT_STRING, false },
^
http.c:124:41: error: 'CURLOPT_PROXY_TLSAUTH_USERNAME' undeclared here (not in a function)
{ "CURLOPT_PROXY_TLSAUTH_USERNAME", CURLOPT_PROXY_TLSAUTH_USERNAME, CURLOPT_STRING, false },
^
http.c:125:41: error: 'CURLOPT_PROXY_TLSAUTH_PASSWORD' undeclared here (not in a function)
{ "CURLOPT_PROXY_TLSAUTH_PASSWORD", CURLOPT_PROXY_TLSAUTH_PASSWORD, CURLOPT_STRING, false },
^
http.c:127:37: error: 'CURLOPT_PROXY_TLSAUTH_TYPE' undeclared here (not in a function)
{ "CURLOPT_PROXY_TLSAUTH_TYPE", CURLOPT_PROXY_TLSAUTH_TYPE, CURLOPT_STRING, false },
^
: recipe for target 'http.o' failed
make: *** [http.o] Error 1
gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -g -O2 -I/projects/curl/rel-curl-7.44.0w64gcc48/include -I/projects/pcre/rel-8.33w64gcc48/include -I/projects/libxml/rel-libxml2-2.7.8w64gcc48/include/libxml2 -I. -I./ -IC:/MING641/projects/POSTGR1/rel/PG10W62/include/server -IC:/MING641/projects/POSTGR1/rel/PG10W62/include/internal -I./src/include/port/win32 -DEXEC_BACKEND -I/projects/libxml/rel-libxml2-2.7.8w64gcc48/include/libxml2 -IC:/MING641/projects/POSTGR1/rel/PG10W6~2/include/server/port/win32 -c -o http.o http.c
http.c:117:28: error: 'CURLOPT_PRE_PROXY' undeclared here (not in a function)
{ "CURLOPT_PRE_PROXY", CURLOPT_PRE_PROXY, CURLOPT_STRING, false },
^
http.c:124:41: error: 'CURLOPT_PROXY_TLSAUTH_USERNAME' undeclared here (not in a function)
{ "CURLOPT_PROXY_TLSAUTH_USERNAME", CURLOPT_PROXY_TLSAUTH_USERNAME, CURLOPT_STRING, false },
^
http.c:125:41: error: 'CURLOPT_PROXY_TLSAUTH_PASSWORD' undeclared here (not in a function)
{ "CURLOPT_PROXY_TLSAUTH_PASSWORD", CURLOPT_PROXY_TLSAUTH_PASSWORD, CURLOPT_STRING, false },
^
http.c:127:37: error: 'CURLOPT_PROXY_TLSAUTH_TYPE' undeclared here (not in a function)
{ "CURLOPT_PROXY_TLSAUTH_TYPE", CURLOPT_PROXY_TLSAUTH_TYPE, CURLOPT_STRING, false },
^
: recipe for target 'http.o' failed
make: *** [http.o] Error 1

`

@robe2

This comment has been minimized.

Show comment
Hide comment
@robe2

robe2 Jun 20, 2017

Contributor

Okay I was able to get this to work by remarking out all the TLS variables. I guess I don't have my curl compiled with TLS support. Not sure how easy it is to check that. I suspect people who didn't compile with SSL support will have similar issues with anything SSL.

The thing I don't like about this is I have to run the function for each session I want to use the setting, as opposed to just setting it once in System or in database GUC.

Stupid thought, you think you can create a table to store these settings and read them so they are maintained at least for the database. Otherwise got to change my code everywhere I use this to include this extra function call. Which at the moment is a PITA.

Contributor

robe2 commented Jun 20, 2017

Okay I was able to get this to work by remarking out all the TLS variables. I guess I don't have my curl compiled with TLS support. Not sure how easy it is to check that. I suspect people who didn't compile with SSL support will have similar issues with anything SSL.

The thing I don't like about this is I have to run the function for each session I want to use the setting, as opposed to just setting it once in System or in database GUC.

Stupid thought, you think you can create a table to store these settings and read them so they are maintained at least for the database. Otherwise got to change my code everywhere I use this to include this extra function call. Which at the moment is a PITA.

@pramsey

This comment has been minimized.

Show comment
Hide comment
@pramsey

pramsey Jun 20, 2017

Owner

Yeah, but the whole point of this is to avoid GUCs. The only alternatives in my mind are (a) including options in every http_() call and (b) this session-based thing, which happens to work because we keep the CURL lying around in a global. No, there will not be a magic table. These fixes are for the 2% of the population who need options beyond the defaults, so nothing will get uglier in the default use case. That's actually why I didn't do the option-every-time approach, since it involves mucking with all the function signatures. Though I could be persuaded to do it, still, since it's programatically the cleanest and most consistent with HTTP as a stateless protocol, and it's also consistent with the CURL API.

Owner

pramsey commented Jun 20, 2017

Yeah, but the whole point of this is to avoid GUCs. The only alternatives in my mind are (a) including options in every http_() call and (b) this session-based thing, which happens to work because we keep the CURL lying around in a global. No, there will not be a magic table. These fixes are for the 2% of the population who need options beyond the defaults, so nothing will get uglier in the default use case. That's actually why I didn't do the option-every-time approach, since it involves mucking with all the function signatures. Though I could be persuaded to do it, still, since it's programatically the cleanest and most consistent with HTTP as a stateless protocol, and it's also consistent with the CURL API.

@pramsey

This comment has been minimized.

Show comment
Hide comment
@pramsey

pramsey Jun 20, 2017

Owner

I built/tested against curl 7.54.0. It may well be that there are options unavailable there. I guess I'll have to hide these in levels of curl version testing. I wonder if there's a dictionary of what version each curlopt was added in.

Owner

pramsey commented Jun 20, 2017

I built/tested against curl 7.54.0. It may well be that there are options unavailable there. I guess I'll have to hide these in levels of curl version testing. I wonder if there's a dictionary of what version each curlopt was added in.

@robe2

This comment has been minimized.

Show comment
Hide comment
@robe2

robe2 Jun 20, 2017

Contributor

I said it was a stupid thought :). Anyway your function approach I prefer over the adding args to every call.
I suppose I could get around the issue by putting a wrapper around all the functions I use that does the set before calling the function. I'll have to give that a try.

Contributor

robe2 commented Jun 20, 2017

I said it was a stupid thought :). Anyway your function approach I prefer over the adding args to every call.
I suppose I could get around the issue by putting a wrapper around all the functions I use that does the set before calling the function. I'll have to give that a try.

@pramsey

This comment has been minimized.

Show comment
Hide comment
@pramsey

pramsey Jun 20, 2017

Owner
Owner

pramsey commented Jun 20, 2017

@robe2

This comment has been minimized.

Show comment
Hide comment
@robe2

robe2 Jun 20, 2017

Contributor

Not exactly. my function is taking no additional args (same signature as your existing), but has the settings I need hard-coded in the function so I can pretend I'm in the 98% of the population that doesn't need anything set.

Contributor

robe2 commented Jun 20, 2017

Not exactly. my function is taking no additional args (same signature as your existing), but has the settings I need hard-coded in the function so I can pretend I'm in the 98% of the population that doesn't need anything set.

@robe2

This comment has been minimized.

Show comment
Hide comment
@robe2

robe2 Jun 20, 2017

Contributor

As an extra datapoint I compiled against my curl 7.48.0 and had the same issue. I tested against 7.54.1 and it was fine.

I wonder if the change happened around 7.52, based on this note here:

https://curl.haxx.se/docs/sslcerts.html

HTTPS proxy

Since version 7.52.0, curl can do HTTPS to the proxy separately from the connection to the server. This TLS connection is handled separately from the server connection so instead of --insecure and --cacert to control the certificate verification, you use --proxy-insecure and --proxy-cacert. With these options, you make sure that the TLS connection and the trust of the proxy can be kept totally separate from the TLS connection to the server.

Contributor

robe2 commented Jun 20, 2017

As an extra datapoint I compiled against my curl 7.48.0 and had the same issue. I tested against 7.54.1 and it was fine.

I wonder if the change happened around 7.52, based on this note here:

https://curl.haxx.se/docs/sslcerts.html

HTTPS proxy

Since version 7.52.0, curl can do HTTPS to the proxy separately from the connection to the server. This TLS connection is handled separately from the server connection so instead of --insecure and --cacert to control the certificate verification, you use --proxy-insecure and --proxy-cacert. With these options, you make sure that the TLS connection and the trust of the proxy can be kept totally separate from the TLS connection to the server.

@pramsey

This comment has been minimized.

Show comment
Hide comment
@pramsey

pramsey Jun 20, 2017

Owner

Check if the new version guards help.

Owner

pramsey commented Jun 20, 2017

Check if the new version guards help.

@robe2

This comment has been minimized.

Show comment
Hide comment
@robe2

robe2 Jun 20, 2017

Contributor

Seems to work. It compiled at any rate didn't test it.

I do see some warnings, but those might have been there before and I just wasn't paying attention.
This shows both compiling against 7.44.0 and 7.54.1

C:/ming64gcc48/projects/curl/rel-curl-7.44.0w64gcc48/include/curl/typecheck-gcc.h:47:9: warning: call to '_curl_easy_setopt_err_long' declared with attribute warning: curl_easy_setopt expects a long argument for this option
_curl_easy_setopt_err_long();
^
http.c:232:8: note: in expansion of macro 'curl_easy_setopt'
err = curl_easy_setopt((handle), (opt), (value));
^
http.c:814:4: note: in expansion of macro 'CURL_SETOPT'
CURL_SETOPT(g_http_handle, CURLOPT_INFILESIZE, content_size);
^

Contributor

robe2 commented Jun 20, 2017

Seems to work. It compiled at any rate didn't test it.

I do see some warnings, but those might have been there before and I just wasn't paying attention.
This shows both compiling against 7.44.0 and 7.54.1

C:/ming64gcc48/projects/curl/rel-curl-7.44.0w64gcc48/include/curl/typecheck-gcc.h:47:9: warning: call to '_curl_easy_setopt_err_long' declared with attribute warning: curl_easy_setopt expects a long argument for this option
_curl_easy_setopt_err_long();
^
http.c:232:8: note: in expansion of macro 'curl_easy_setopt'
err = curl_easy_setopt((handle), (opt), (value));
^
http.c:814:4: note: in expansion of macro 'CURL_SETOPT'
CURL_SETOPT(g_http_handle, CURLOPT_INFILESIZE, content_size);
^

@robe2

This comment has been minimized.

Show comment
Hide comment
@robe2

robe2 Jun 20, 2017

Contributor

oops hold on spoke too soon. You put

{ "CURLOPT_CAINFO", CURLOPT_CAINFO, CURLOPT_STRING, false },

under 7.52. That isn't right. That has existed for quite some time and what I was using to override ssl bundle.

Contributor

robe2 commented Jun 20, 2017

oops hold on spoke too soon. You put

{ "CURLOPT_CAINFO", CURLOPT_CAINFO, CURLOPT_STRING, false },

under 7.52. That isn't right. That has existed for quite some time and what I was using to override ssl bundle.

@robe2

This comment has been minimized.

Show comment
Hide comment
@robe2

robe2 Jun 20, 2017

Contributor

@pramsey
Okay that last commit works with no errors or warnings and I can set the CURLOPT_CAINFO with my 7.44.0 again and access https sites.

Thanks

Contributor

robe2 commented Jun 20, 2017

@pramsey
Okay that last commit works with no errors or warnings and I can set the CURLOPT_CAINFO with my 7.44.0 again and access https sites.

Thanks

@pramsey pramsey merged commit 05f4c13 into master Jun 21, 2017

@pramsey pramsey deleted the setopt branch Aug 10, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment