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

metalink: remove #7176

Closed
wants to merge 2 commits into from
Closed

metalink: remove #7176

wants to merge 2 commits into from

Conversation

@bagder
Copy link
Member

@bagder bagder commented Jun 2, 2021

Warning: this will make existing curl command lines that use metalink to
stop working.

Reasons for removal:

  1. We've found several security problems and issues involving the
    metalink support in curl. The issues are not detailed here. When
    working on those, it become apparent to the team that several of the
    problems are due to the system design, metalink library API and what
    the metalink RFC says. They are very hard to fix on the curl side
    only.

  2. The metalink usage with curl was only very briefly documented and was
    not following the "normal" curl usage pattern in several ways, making
    it surprising and non-intuitive which could lead to further security
    issues.

  3. The metalink library was last updated 6 years ago and wasn't so
    active the years before that either. An unmaintained library means
    there's a security problem waiting to happen. This is probably reason
    enough.

  4. Metalink requires an XML parsing library, which is complex code (even
    the smaller alternatives) and to this day often gets security
    updates.

  5. Metalink is not a widely used curl feature. In the 2020 curl user
    survey, only 1.4% of the responders said that they'd are using
    it. Searching the web also show very few traces of it being used,
    even with other tools.

  6. The torrent format and associated technology clearly won for
    downloading large files from multiple sources in parallel.

Warning: this will make existing curl command lines that use metalink to
stop working.

Reasons for removal:

1. We've found several security problems and issues involving the
   metalink support in curl. The issues are not detailed here. When
   working on those, it become apparent to the team that several of the
   problems are due to the system design, metalink library API and what
   the metalink RFC says. They are very hard to fix on the curl side
   only.

2. The metalink usage with curl was only very briefly documented and was
   not following the "normal" curl usage pattern in several ways, making
   it surprising and non-intuitive which could lead to further security
   issues.

3. The metalink library was last updated 6 years ago and wasn't so
   active the years before that either. An unmaintained library means
   there's a security problem waiting to happen. This is probably reason
   enough.

4. Metalink requires an XML parsing library, which is complex code (even
   the smaller alternatives) and to this day often gets security
   updates.

5. Metalink is not a widely used curl feature. In the 2020 curl user
   survey, only 1.4% of the responders said that they'd are using
   it. Searching the web also show very few traces of it being used,
   even with other tools.

6. The torrent format and associated technology clearly won for
   downloading large files from multiple sources in parallel.
@ytrezq
Copy link

@ytrezq ytrezq commented Jun 2, 2021

I disagree on point 6 though, torrent will never support e2dk or gnunet or ipts unlike MetaLink. But I recognize curl only supports servers protocols and not p2p ones.

@bagder
Copy link
Member Author

@bagder bagder commented Jun 3, 2021

Assuming no major obstacle, I will merge this on June 7th.

@kdudka
Copy link
Collaborator

@kdudka kdudka commented Jun 3, 2021

I would prefer if --with-libmetalink made the configure script fail rather than being silently ignored. If someone expects metalink to be available, they should learn about its removal rather sooner than later.

@bagder
Copy link
Member Author

@bagder bagder commented Jun 3, 2021

I would prefer if --with-libmetalink made the configure script fail rather than being silently ignored.

Good idea!

@danielgustafsson
Copy link
Member

@danielgustafsson danielgustafsson commented Jun 3, 2021

Another idea could be to move metalink support to being an experimental feature (even though experimental generally means moving the other way) requiring an explicit opt-in before removing it. Iff there is an outcry after the release then there is a way for powerusers to keep their systems running while we take the discussion on how to proceed. FWIW, I'm not sure I want this but I wanted to raise it in the discussion before the PR is merged.

@bagder
Copy link
Member Author

@bagder bagder commented Jun 3, 2021

I don't think we should merge even an experimental feature with known security problems so they would still need to be worked on first.

Power users can just revert this removal to get metalink back with a local patch. If there's a strong enough demand to get it back "for real", I expect developer time to get funded to fix the issues.

@danielgustafsson
Copy link
Member

@danielgustafsson danielgustafsson commented Jun 3, 2021

@xquery
Copy link
Member

@xquery xquery commented Jun 3, 2021

maybe this has already been discussed here or elsewhere - apologies

I have no sense of metalink usage ( I see very few respondents to survey use it) ... adhoc observation is that there is not a lot of adoption ... in the best of worlds (good developer support of exotic feature) my pref would be that said feature would not be part of the core ... considering we have less then good implementation - its a 'no brainer' to deprecate.

Alternately is it time to discuss (prob again) how to formalise pluggability beyond obvious routes .. it makes my brain hurt as its so easy to extend curl/libcurl and not warrant inclusion into core repo ... but is it getting to the point where we need more points of extensibility ?

@bagder
Copy link
Member Author

@bagder bagder commented Jun 3, 2021

is it getting to the point where we need more points of extensibility ?

Perhaps. I don't think this is the last straw that breaks the camel's back though because I don't think it is popular enough.

Extensibility, plugins and add-ons are mentioned every once in a while. It's just that nobody has yet even tried to write up a suggestion of how it could work - at any level of ambition - so I think we're still not quite there yet. Personally, I'm casually skeptic because I think both curl and libcurl are what they are partly because they provide feature-packed all-in-one packages.

bagder added a commit that referenced this pull request Jun 7, 2021
Warning: this will make existing curl command lines that use metalink to
stop working.

Reasons for removal:

1. We've found several security problems and issues involving the
   metalink support in curl. The issues are not detailed here. When
   working on those, it become apparent to the team that several of the
   problems are due to the system design, metalink library API and what
   the metalink RFC says. They are very hard to fix on the curl side
   only.

2. The metalink usage with curl was only very briefly documented and was
   not following the "normal" curl usage pattern in several ways, making
   it surprising and non-intuitive which could lead to further security
   issues.

3. The metalink library was last updated 6 years ago and wasn't so
   active the years before that either. An unmaintained library means
   there's a security problem waiting to happen. This is probably reason
   enough.

4. Metalink requires an XML parsing library, which is complex code (even
   the smaller alternatives) and to this day often gets security
   updates.

5. Metalink is not a widely used curl feature. In the 2020 curl user
   survey, only 1.4% of the responders said that they'd are using it. In
   2021 that number was 1.2%. Searching the web also show very few
   traces of it being used, even with other tools.

6. The torrent format and associated technology clearly won for
   downloading large files from multiple sources in parallel.

Cloes #7176
@bagder
Copy link
Member Author

@bagder bagder commented Jun 7, 2021

Closed by 265b14d

@bagder bagder closed this Jun 7, 2021
@bagder bagder deleted the bagder/rm-metalink branch Jun 7, 2021
D4v1dH03 added a commit to D4v1dH03/homebrew-core that referenced this pull request Jun 7, 2021
See upstream pull request curl/curl#7176 which removes metalink support

To build with metalink support is no longer available.
BrewTestBot added a commit to Homebrew/homebrew-core that referenced this pull request Jun 8, 2021
See upstream pull request curl/curl#7176 which removes metalink support

To build with metalink support is no longer available.

Closes #78895.

Signed-off-by: Carlo Cabrera <30379873+carlocab@users.noreply.github.com>
Signed-off-by: BrewTestBot <1589480+BrewTestBot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

5 participants