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

update source links to https where possible #2391

Merged
merged 1 commit into from
Dec 20, 2021

Conversation

jmartin-tech
Copy link
Contributor

  • Update the source link for libiconv due to redirect not followed on windows
  • Update zlib source link to https to be consistent where possible
  • xmlsoft.org is left as http due to what looks like SNI hosting without https service for xmlsoft.org

What problem is this PR intended to solve?

Native build on windows seems to be failing due to redirects related to libiconv package retrieval.

---------- IMPORTANT NOTICE ----------
Building Nokogiri with a packaged version of libiconv-1.15.
Configuration options: --host\=x86_64-w64-mingw32 --enable-static
--disable-shared
--libdir\=C:/embedded/lib/ruby/gems/3.0.0/gems/nokogiri-1.12.5/ports/x86_64-w64-mingw32/libiconv/1.15/lib
--disable-dependency-tracking --disable-shared --enable-static CPPFLAGS\=-Wall
CFLAGS\=-IC:/embedded/include\ -m64\ -O3\ -march\=x86-64\
-O2\ -U_FORTIFY_SOURCE\ -g\ -fPIC
CXXFLAGS\=-IC:/metasploit-framework/embedded/include\ -m64\ -O3\ -march\=x86-64\
-O2\ -U_FORTIFY_SOURCE\ -g LDFLAGS\=

The Nokogiri maintainers intend to provide timely security updates, but if
this is a concern for you and want to use your OS/distro system library
instead, then abort this installation process and install nokogiri as
instructed at:

https://nokogiri.org/tutorials/installing_nokogiri.html#installing-using-standard-system-libraries

2 retrie(s) left for libiconv-1.15.tar.gz
1 retrie(s) left for libiconv-1.15.tar.gz
0 retrie(s) left for libiconv-1.15.tar.gz
Failed to open TCP connection to ftp.gnu.org:80 (A connection attempt failed
because the connected party did not properly respond after a period of time, or
established connection failed because connected host has failed to respond. -
connect(2) for "ftp.gnu.org" port 80)
*** extconf.rb failed ***

Have you included adequate test coverage?

No added tests as no code changes, only updates transport protocol or retrieval of external source.

Does this change affect the behavior of either the C or the Java implementations?

No

* Update the source link for libiconv due to redirect not followed on windows
* Update zlib source link to `https` to be consistent where possible
* xmlsoft.org is left as `http` due to what looks like SNI hosting without `https` service for xmlsoft.org
@flavorjones
Copy link
Member

@jmartin-r7 I'm not seeing any problems related to downloading libiconv. Can you help me understand what you're seeing and what version of mini_portile2 you're using? I would like to reproduce this problem before proceeding to a pull request.

@flavorjones
Copy link
Member

Specifically, I'd like to understand what you're seeing that's leading you to conclude "redirect not followed on windows". The code used to download is ruby code in mini_portile2 that handles redirects, so I would like to understand if this is simply a connectivity issue or if we truly need to update the URL.

@flavorjones
Copy link
Member

While we're here, I'd love to do some user research with you: Is there a specific reason you're building from source (using the vanilla "ruby" platform gem) rather than the native precompiled gem for your platform?

@jmartin-tech
Copy link
Contributor Author

@flavorjones, it is possible we are experiencing some connectivity issues in automation, however multiple runs, hours apart, resulted in the same failure, This leads me to believe the http request process is having trouble with redirects.

Another reason for offering this update is, while maintaining other projects, I have seen a recent trend towards http resources being shifted onto https without any redirection available.

The reason the gem is built from source is related to common usage of Gemfile.lock across multiple platforms (Linux on x86/x64/armhf/arm64, macOS x64, Windows x64). I am aware this is not a standard practice since Bundler 2.2.x however work is still ongoing to find a way forward that we find maintainable.

@flavorjones
Copy link
Member

This leads me to believe the http request process is having trouble with redirects

I'm still not sure I see how you're arriving at this conclusion, and so I'm worried that updating these URLs will not help. That said, I'd be happy to merge this change since the upstream servers support HTTPS.

Once this goes green I'll hit merge.

@flavorjones
Copy link
Member

Just as additional information for you to consider: I don't see any redirects when I hit this URL:

$ wget http://ftp.gnu.org/pub/gnu/libiconv/libiconv-1.15.tar.gz
--2021-12-20 12:21:54--  http://ftp.gnu.org/pub/gnu/libiconv/libiconv-1.15.tar.gz
Resolving ftp.gnu.org (ftp.gnu.org)... 209.51.188.20, 2001:470:142:3::b
Connecting to ftp.gnu.org (ftp.gnu.org)|209.51.188.20|:80... connected.
HTTP request sent, awaiting response... 200 OK
Length: 5264188 (5.0M) [application/x-gzip]
Saving to: ‘libiconv-1.15.tar.gz.1’

libiconv-1.15.tar.gz.1            100%[============================================================>]   5.02M  10.2MB/s    in 0.5s    

2021-12-20 12:21:55 (10.2 MB/s) - ‘libiconv-1.15.tar.gz.1’ saved [5264188/5264188]


@jmartin-tech
Copy link
Contributor Author

Thank you for the additional check, the issue in automation could simply be connectivity then. I suspect the issue will resolve on its own then.

I had verified the https call was available and the redirection may simply have been due to some tooling locally in my testing before offering this change.

If the project would rather stick to http feel free to close this a not needed.

@flavorjones
Copy link
Member

Preferring https to http is a good policy, even if it might not solve your problem. Happy to merge, thank you!

@flavorjones flavorjones merged commit 0578700 into sparklemotion:main Dec 20, 2021
flavorjones added a commit that referenced this pull request Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants