Problem/Motivation

I am not a lawyer (IANAL), all opinions given are my own. Please contact a licensed attorney that is qualified to opine on the subject matters contained herein should you have any questions.

I stumbled upon this code while looking at implementing in contrib a resolution on specific callables to see if there was any preexisting work under GPLv2+ in core that could be called instead of duplicated in contrib.

ExpectDeprecationTrait::getCallableName() has an @see record in its doclblock pointing to https://stackoverflow.com/questions/34324576/print-name-or-definition-of....

Unless otherwise stated answers published on StackOverflow appear to be licensed under licenses determined by publication date.

From history we can see the code sample started out as CC BY-SA 3.0, was edited by its original author converting it to CC BY-SA 4.0 and it was later extended upon in a new answer under CC BY-SA 4.0. I was unable to find a comment co-releasing the code under a diffrent license.

Googling for a solution to a problem I found some code 99% matching what I needed on stackoverflow, c/p'ed it, adjusted and made a reference to the source for posterity.
- @mondrake (Source: Slack Thread in #core-development)

CC BY-SA 3.0 is 100% incompatible with all revisions of GPL, CC BY-SA 4.0 can be merged with GPLv3(only) (Source: Creative Commons - Compatible Licenses).

The code in question is small, and could possibly be developed independently in the same manner, however as we have an annotation directly refereeing the StackOverflow answer, and a comment in Slack implying a copy/paste with modification (deriviative) as such we have reason to belive that this was directly taken rather which raises questions on the validity of inclusions on GPLv2+ application.

A cursory review getCallableName() appears to only be called inside tearDownErrorHandler() howeve the Trait is included as low as \Drupal\Tests\UnitTestCase. I am unsure where tearDownErrorHandler() is called

Setting to Criticial as this involves Legal Compliance(Licensing) and the scope should be identified before any further code is impacted.

Tagging for Framework manager and Release Manager as I believe this involves their scope.

This issue may need to become a Meta at some point.

Steps to reproduce

Review source.

Proposed resolution

Resolve concern (if any exists)
Implement measures to prevent re-occurrence.

Remaining tasks

  1. Determine if this is actually CC BY-SA 4.0 only or if some other clause allows it to be classified as GPLv2+
  2. Confirm this means that the code could only be GPLv3 in cotext of Drupal.
  3. Determine spread of 'taint':
  • Is the entiere commit GPLv3 (even if so @mondrake could resolve much of this by re-granting/affirming a GPLv2+ license for all other code once this is replaced)
  • Does this taint apply to any other code extending the UntiTestCase class or higher (are all Core tests now tainted to GPLv3?)
  • Does this impact Contrib licensing.

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3517614

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

cmlara created an issue. See original summary.

cmlara’s picture

cmlara
he/him
commented
Issue summary: View changes

Edit: Correction on the provenance, it started as CC BY-SA 3.0 not 2.5 (no change in statements above regarding compatibility concerns)

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Yep we need to re-write this code.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

I;ve looked for other links to stackoverflow in our code base. There are a few the others seem to involve much less code but I guess need to be reviewed as well.

cmlara’s picture

cmlara
he/him
commented

Saw blog post that the next Alpha will be released in the near future.

Should this be classified as an Alpha blocker and release blocker?

The monthly security window I believe is tomorrow, doss this have any impact on that window?

mstrelan’s picture

mstrelan commented

Do we even need this function? It's only used when throwing this exception:

throw new \RuntimeException(sprintf('%s registered its own error handler (%s) without restoring the previous one before or during tear down. This can cause unpredictable test results. Ensure the test cleans up after itself.',
  $this->name(),
  self::getCallableName($handler),
));

Wouldn't it be enough without self::getCallableName($handler)?

quietone’s picture

quietone commented

Should we not seek an opinion from a the licensing working group?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

The monthly security window I believe is tomorrow, doss this have any impact on that window?

I don't think so. This is an existing issue that has already been released. If it would be the first release with it in maybe but that's not the case.

I agree with #6 let's just remove the code. We can open an issue to reimplement if we like.

longwave made their first commit to this issue’s fork.

longwave opened merge request !11850

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Active » Needs review
Issue tags: -Needs release manager review

Agree, let's just remove the offending code for now, we don't strictly need it here. If we want to reintroduce it again by rewriting it in the future we can do that in another issue.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Makes sense to just remove it, the error message is decent without it.

cmlara’s picture

cmlara
he/him
commented

Please take that into account that while the comments below are not intended to be an 'alarmist' reaction, I am well aware that this particular incident is minor at the moment (and overall is one of the least significant I've encountered in my history with software), the goal is to ensure it does not become more significant and to ensure any future incidents also have a well grounded base in place to avoid them possibly becoming significant.

Should we not seek an opinion from a the licensing working group?

I would suggest no matter what the Core Team/The Project Lead (Dries) will want to run this past legal to ensure they properly unwind this to ensure no future legal entanglement.

I don't think so. This is an existing issue that has already been released. If it would be the first release with it in maybe but that's not the case.

The phrase "Knowingly, willfully, and repeatedly" could be used to describe publishing additional releases or commits after the core team is made aware of the issue. It is one thing for something like this to happen without knowledge there is some room for compassion and more lenient mitigation.

The Linksys GPLv2 case is one of the most notable example of case law on this subject. The platform went from being Closed Source to Open Source GPLv2. An equivalent scenario for the Drupal Project in this case could be that the code no longer qualify as GPLv2 and would be GPLv3 only, not the worst situation that could happen, however I also suspect this is not desired by the Core Team.

Agree, let's just remove the offending code for now, we don't strictly need it here.

Seems reasonable.

benjifisher’s picture

benjifisher
he/him or they/them
Primary language English
Location Boston area
commented

In the migrate_plus module, I credited StackOverflow: https://git.drupalcode.org/project/migrate_plus/-/blob/6.0.x/src/Plugin/...

    // @see https://stackoverflow.com/a/47718734/3130080
    return array_map(NULL, ...$table);

Does that seem problematic? Once you know to look for it, that usage of array_map() is also documented in the PHP docs: https://www.php.net/array_map#example-5199. Unlike the code in this issue, it is a one-liner.

catch’s picture

catch
he/him
Primary language English
commented

For me personally, Creative Commons licenses are not software licenses at all. That means we should treat StackOverflow as documentation (or a discussion forum etc.), and if code follows that documentation and references the documentation it comes from, that is compatible with with a Creative Commons license and does not imply that using the code itself adopts that license.

Code on stack overflow is not released as code, it's embedded in a text document as an example, it's not even like a gist which might be a small standalone program.

Obviously I am not a lawyer, and also I have zero interest in whether this argument would stand up in court, and doubt it would ever need to.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 11.x-dev » 11.0.x-dev
Status: Reviewed & tested by the community » Fixed

Discussed with @longwave and we agreed to put this back to 11.0.x in case we do a security release on 11.0.x.

Committed and pushed 405ac9dad3c to 11.x and 7769a956fbe to 11.1.x and 902e7a8c57a to 11.0.x. Thanks!

  • alexpott committed 902e7a8c on 11.0.x 
    Issue #3517614 by alexpott, longwave, cmlara, catch, quietone, mstrelan...

  • alexpott committed 7769a956 on 11.1.x 
    Issue #3517614 by alexpott, longwave, cmlara, catch, quietone, mstrelan...

  • alexpott committed 405ac9da on 11.x 
    Issue #3517614 by alexpott, longwave, cmlara, catch, quietone, mstrelan...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

quietone closed merge request !11850