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

WIP: Libcurl backend #26

Merged
merged 14 commits into from Oct 4, 2018
Merged

Conversation

goetzk
Copy link
Member

@goetzk goetzk commented Sep 12, 2018

Hi,
I'm after feedback on this change. Its only required in cases where the OS doesn't support SNI - in my case RHEL6 but I expect a similarly aged Debian/Ubuntu would see the same problem.

The main thing I want to know is if I should try and if/else it out so (for example) if WWW::Curl is available that is used instead of HTTP::Headers and SSLeay.

Do you need this change? If you see this, you need the changes:

bash-4.1$ lwp-request -U -S -e -d  https://test.datacite.org:443
GET https://test.datacite.org:443
User-Agent: lwp-request/5.827 libwww-perl/5.833

GET https://test.datacite.org:443 --> 500 Can't connect to test.datacite.org:443 (SSL connect attempt failed with unknown errorerror:14077410:SSL routines:SSL23_GET_SERVER_HELLO:sslv3 alert handshake failure)
Content-Type: text/plain
Client-Date: Wed, 12 Sep 2018 22:54:39 GMT
Client-Warning: Internal response

If you see something like this, you should be fine unmodified:

bash-4.1$ lwp-request -U -S -e -d  https://researchdata.ands.org.au/api/doi/datacite
Enter username for ands at researchdata.ands.org.au:443: [username]
Password: 
GET https://researchdata.ands.org.au/api/doi/datacite
Authorization: Basic [big long string]
User-Agent: lwp-request/5.827 libwww-perl/5.833

GET https://researchdata.ands.org.au/api/doi/datacite --> 401 Unauthorized
GET https://researchdata.ands.org.au/api/doi/datacite --> 200 OK

We have used this code to mint several DOIs so the basic functionality is working.

Work in progress code to change from LWP to WWW::Curl. This has been forced
upon us by the age of LWP in RHEL6.10 which can't handle SNI servers.
See also
curl/curl#700 - Curl updates in RHEL 6.7 and 6.8
libwww-perl/LWP-Protocol-https#17 LWP SNI fix from 6.07
This branch requires use WWW::Curl not SSLeay
@goetzk goetzk added this to the v2.3 milestone Sep 18, 2018
@goetzk goetzk modified the milestones: v2.3, v2.2 Sep 18, 2018
@goetzk goetzk changed the base branch from master to v2.1.0 September 26, 2018 01:43
each request fills a page so turning down for now.
@goetzk
Copy link
Member Author

goetzk commented Sep 26, 2018

Looking at https://access.redhat.com/support/policy/updates/errata RHEL6 has 2 years of bug fix support left, so this code is unlikely to be used after that time.

From the Ubuntu side, only 14.04 is still supported but only for another ~7 months.
https://www.ubuntu.com/about/release-cycle

SNI fixes arrived in 6.0.7
https://metacpan.org/release/LWP-Protocol-https

I'm sure some other long term systems from different vendors are still out there, how many are running EPrints is another matter.

@goetzk goetzk modified the milestones: v2.2, v2.1 Sep 26, 2018
we don't have a repository instance at file top. moving includes down
is bad form WRT expectations but saves us another settings check and
doesn't meaningfully impact flow because the libraries are only used
in the one method.
HTTP::Headers needs two :, not one.
goetzk referenced this pull request Oct 2, 2018
As reported in #14, Crypt::SSLeay is no longer the recommended way of getting
HTTPS through LWP.

https://wiki.eprints.org/w/index.php?title=Installing_EPrints_on_RHEL/Fedora/CentOS&diff=prev&oldid=12408

This is also backed up by Crypt::SSLeay upstream who warn
```
At this point, Crypt::SSLeay is maintained to support existing software that
already depends on it. However, it is possible that your software does not
really depend on Crypt::SSLeay, only on the ability of LWP::UserAgent class to
communicate with sites over SSL/TLS.
```

https://metacpan.org/pod/Crypt::SSLeay#DO-YOU-NEED-Crypt::SSLeay?

Closes: #14
Closes: #23
This is a collection of whitespace, ordering and wording changes
All the bits were there, just needed to be moved around.
Having spent some time testing this now having everything packed in to
datacite_request was just a bit fiddly and there were issues with checking
configuration (among other things).

This "feels like" it reduces complexity and thats what I've been going for.
@goetzk goetzk merged commit 50d3eec into eprintsug:v2.1.0 Oct 4, 2018
@goetzk goetzk deleted the libcurl-backend branch October 4, 2018 00:09
@goetzk goetzk restored the libcurl-backend branch October 4, 2018 00:12
@goetzk goetzk deleted the libcurl-backend branch October 4, 2018 00:13
@goetzk goetzk restored the libcurl-backend branch March 13, 2019 04:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant