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

Bugfix/add support curl8 #89

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Bugfix/add support curl8 #89

wants to merge 3 commits into from

Conversation

sprive
Copy link
Contributor

@sprive sprive commented Jul 16, 2024 •
edited
Loading

CHANGES:

  1. pull in remainder of Draft fixes from Fix/library not supporting curl version 8.0.x or higher, Closes #10 #85. (thanks @Laeri)
  2. minor cleanup, docstrings, and python typehints

IF MERGED:

TESTING:

  • I was able to go run various use cases from the examples directory, using curl 8, on macOS.
  • (I don't feel my level of testing here is adequate or production quality. This is an area that other users can help with)

@sprive sprive mentioned this pull request Jul 16, 2024
Draft
@sprive
Copy link
Contributor Author

sprive commented Jul 16, 2024

Hi @andelf
No rush but this fixes things for curl8. Thanks

@sprive
Copy link
Contributor Author

sprive commented Jul 17, 2024 •
edited
Loading

Some test results

curl used:

macmini-2018:curl sprive$ git describe --tags --abbrev=0
curl-8_8_0

go-curl examples:

$ go run ./examples/ftpget.go 
* Host ftp.gnu.org:21 was resolved.
* IPv6: (none)
* IPv4: 209.51.188.20
*   Trying 209.51.188.20:21...
* Connected to ftp.gnu.org (209.51.188.20) port 21
< 220 GNU FTP server ready.
> USER anonymous
< 230-NOTICE (Updated October 15 2021):
< 230-
< 230-If you maintain scripts used to access ftp.gnu.org over FTP,
< 230-we strongly encourage you to change them to use HTTPS instead.
< 230-
< 230-Eventually we hope to shut down FTP protocol access, but plan
< 230-to give notice here and other places for several months ahead
< 230-of time.
< 230-
< 230----
< 230-
< 230-Due to U.S. Export Regulations, all cryptographic software on this
< 230-site is subject to the following legal notice:
< 230-
< 230-    This site includes publicly available encryption source code
< 230-    which, together with object code resulting from the compiling of
< 230-    publicly available source code, may be exported from the United
< 230-    States under License Exception "TSU" pursuant to 15 C.F.R. Section
< 230-    740.13(e).
< 230-
< 230-This legal notice applies to cryptographic software only. Please see
< 230-the Bureau of Industry and Security (www.bxa.doc.gov) for more
< 230-information about current U.S. regulations.
< 230 Login successful.
> PWD
< 257 "/" is the current directory
* Entry path is '/'
* Request has same path as previous transfer
> EPSV
* Connect data stream passively
* ftp_perform ends with SECONDARY: 0
< 229 Entering Extended Passive Mode (|||29542|)
* Connecting to 209.51.188.20 (209.51.188.20) port 29542
*   Trying 209.51.188.20:29542...
* Connected to ftp.gnu.org (209.51.188.20) port 21
> TYPE I
< 200 Switching to Binary mode.
> SIZE README
< 213 2748
> RETR README
< 150 Opening BINARY mode data connection for README (2748 bytes).
* Maxdownload = -1
* Getting file with size: 2748
* Remembering we are in dir ""
< 226 Transfer complete.
* Connection #0 to host ftp.gnu.org left intact

$ go run ./examples/https.go 
<html>
<head><title>302 Found</title></head>
<body bgcolor="white">
<center><h1>302 Found</h1></center>
<hr><center>bfe/1.0.8.18</center>
</body>
</html>

$ go run ./examples/sendrecv.go 
* Host www.renren.com:80 was resolved.
* IPv6: (none)
* IPv4: 120.133.12.124, 120.133.12.123
*   Trying 120.133.12.124:80...
* Connected to www.renren.com (120.133.12.124) port 80
* Connection #0 to host www.renren.com left intact
recv num =  348
HTTP/1.1 200 OK
Server: nginx
Date: Wed, 17 Jul 2024 01:24:55 GMT
...

Some breakage found... but this could have been broken before I touched anything. Can't be sure as I did not setup curl7 to compare:

$ go run ./examples/misc.go 
# command-line-arguments
examples/misc.go:43:42: undefined: curl.OPT_RESOLVE

^--I'm not thrilled with finding a bug I can't fix. Overall since the PR gets most examples working on newer systems, it does seem the better outcome.

@sprive
Copy link
Contributor Author

sprive commented Jul 21, 2024 •
edited
Loading

**HELP WANTED:
If anyone who knows cgo, or knows libcurl. please feel free to jump in (or create a fresh PR from this) :-)
I'm not much of a C or cgo person.

Haven't given up but I am spinning wheels in mud..**

Current Error:

$ go run ./examples/misc.go 
# command-line-arguments
examples/misc.go:43:42: undefined: curl.OPT_RESOLVE

OK, not defined. So now I'm editing compat.h directly to work out the changes needed (to then put into compatgen.py once figured out..

Before my PR, repo's OPT_RESOVE definition looked like:

#if (LIBCURL_VERSION_MINOR == 21 && LIBCURL_VERSION_PATCH < 3) || LIBCURL_VERSION_MINOR < 21 
#define CURLOPT_RESOLVE 0

My assumption is that all the old #ifdefs were written assuming Curl version 7.
So the first thing I tried was:

#if (LIBCURL_VERSION_MAJOR == 7 && LIBCURL_VERSION_MINOR == 21 && LIBCURL_VERSION_PATCH < 3) || LIBCURL_VERSION_MAJOR == 7 && LIBCURL_VERSION_MINOR < 21 
#define CURLOPT_RESOLVE 0

...but same error -- in ./examples/misc.go. (Other examples were broken in curl8, but made working again with this PR).

Next, tried hardcoding it enabled:

#if (LIBCURL_VERSION_MAJOR == 8) 
#define CURLOPT_RESOLVE 1

Still same error. I don't think I introduced this error, but knowing it's there makes the PR harder to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@sprive