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

Epic: merge nokogiri-xmlsec into Nokogiri #2902

Open
23 tasks
maths22 opened this issue Jun 6, 2023 · 0 comments
Open
23 tasks

Epic: merge nokogiri-xmlsec into Nokogiri #2902

maths22 opened this issue Jun 6, 2023 · 0 comments

Comments

@maths22
Copy link

maths22 commented Jun 6, 2023

The maintainers of Nokogiri and nokogiri-xmlsec-instructure are planning to merge the two gems together so that Nokogiri assimilates xmlsec's signing and encryption features.

This issue is intended to be a "parent" issue which can be followed to understand the plan and how it's progressing, as the work will likely take O(weeks) and will be on a feature branch driven by multiple PRs.

This description will be edited as we go to reflect current state and progress.

Background

This work has previously been discussed:

As indicated in the first discussion, the underlying motivation is to allow nokogiri to stop exporting the symbols of the underlying native libraries when compiled with libxml2, libxslt, and friends. (Question: to facilitate potential future innovation, do we want to continue to export nokogiri's own symbols when compiled to use system libraries where the symbol export issues are irrelevant?)

Goals

Here's what success looks like:

  • Nokogiri v1.xy supports xmlsec encryption/decrption and signing/validation using the code from nokogiri-xmlsec-instructure.=
  • Upgrade paths for users are easy to navigate, and users don't get "stuck" on old versions.
  • Xmlsec and a crypto dependency (probably openssl) ships as a precompiled library in all the native platform gems supported by Nokogiri v1.xx.
  • The end-state of nokogiri-xmlsec-instructure is a (mostly) empty gem that prompts users to replace their dependency on nokogiri-xmlsec-instructure.

For more specific objectives, see the "Punchlist" section below.

Note: No JRuby support at this time

⚠️ JRuby support is not going to be addressed as part of this work. However, we hope to work on this in the future.

The nokogiri-xmlsec code relies on an xmlsec implementation in C, and a C extension that is tightly coupled to libxml2. As a result, the xmlsec methods will not be immediately available on JRuby, which uses Xerces in place of libxml2. We should be able to use https://santuario.apache.org/ in place of xmlsec1, but that exploration will be in a future release.

We ask that all downstream libraries be aware of this platform limitation as they consider using HTML5 parsing methods post-merger.

Risks

This work doesn't feel very risky, but if I had to name the riskiest bits:

  • Precompiling xmlsec's crypto dependnecies for all our platforms might be challenging.

Some things that could have been risky but aren't:

  • License compatibility.
    • The nokogiri-xmlsec-instructure license is MIT. Since nokogiri is also MIT, we don't need to worry about compatiblity, just including the necessary copyright notice (the upstream repo just says Copyright (c) 2013 TODO: Write your name, so maybe we don't need to carry that forward?).
    • The xmlsec license is MIT. Since nokogiri is also MIT, we don't need to worry about compatiblity, just including the necessary notice for binary distributions.
    • The openssl license is APL2.0. APL2.0 is compatible with MIT, so we just need to include the necessary notice for binary distributions.
    • (we could theoretically explore using a different crypto provider instead, but all seem worse license-wise)

Punchlist

Some finer-grained objectives (which will be modified over time as we discover new work to be done):

Pre-merger

  • Release nokogiri-xmlsec-instructure version which no-ops if Nokogiri is providing xmlsec functionality (not sure if we need this for this initiative or not)

Team Merger

@flavorjones (this section was copy-pasted from nokogumbo, and I don't know if it actually applies to this work; please edit appropriately)

  • Nokogumbo contributors are added to the Nokogiri gemspec, README, and copyright declarations.
  • Nokogumbo contributors are added to @sparklemotion/nokogiri-core
  • Send some welcome gifts to the Nokogumbo maintainers.

Legal/License Merger

  • All nokogiri-xmlsec files should possibly include a header with their original copyright (see note above re: current copyright)
  • Update LICENSE-DEPENDENCIES.md to include xmlsec under MIT
  • Update LICENSE-DEPENDENCIES.md to include openssl under Apache License 2.0 (APL2.0 clause 4.a)

Functional Merger

  • Commit history for nokogiri-xmlsec-instructure is preserved in the Nokogiri repository. (given scale I don't know if this is actually valuable?)
  • Unit tests include an integration contract for nokogiri-xmlsec equivalent of feat: Nokogumbo detects Nokogiri's HTML5 API rubys/nokogumbo#171
  • nokogiri-xmlsec-instructure unit tests integrated into rake test
  • nokogiri-xmlsec-instructure rb files moved into lib/nokogiri and C file moved into ext/nokogiri
  • extconf.rb builds xmlsec/openssl into nokogiri.so
  • nokogiri-xmlsec-instructure unit tests are green on a dev system
  • Valgrind memory testing is green on xmlsec functionality
  • xmlsec/openssl can be precompiled for linux 64-bit and 32-bit.
  • xmlsec/openssl can be precompiled for windows 64-bit and 32-bit.
  • xmlsec/openssl can be precompiled for darwin x64-64 and arm64.
  • xmlsec/openssl can be compiled from the vanilla gem on platforms without a native gem.

Pre-release

  • HTML5 rdocs should note that the API is available only starting in v1.xy, and only in CRuby (not JRuby).
  • Update README and Tutorials

Release candidate

  • :shipit:
  • solicit feedback in a Discussion thread

Final release

  • :shipit:
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

No branches or pull requests

1 participant