PR#26: More generalized license matching - librejs

librejs

#26 More generalized license matching

Closed 5 years ago by quidam. Opened 5 years ago by quidam.

quidam/librejs match-by-link into master

Ruben Rodriguez • 5 years ago

2cdb441

main_background.js

file modified

+11 -3

		`@@ -644,15 +644,23 @@`


		`function validateLicense(matches) {`
		`- if (!(Array.isArray(matches) && matches.length === 4)){`
		`+ if (!(Array.isArray(matches) && matches.length >= 4)){`
		`return [false, "Malformed or unrecognized license tag."];`
		`}`
		`let [all, tag, link, id] = matches;`
		`- let license = licenses[id];`
		`+ let license = null;`
		`+ if (licenses[id])`
		`+ license = licenses[id];`
		`+ for (var key in licenses){`
		`+ if (licenses[key]["Magnet link"]==link)`
		`+ license=licenses[key];`
		`+ if (licenses[key]["URL"]==link)`
		`+ license=licenses[key];`
		`+ }`
		`if(!license){`
		return [false, `Unrecognized license "${id}"`];
		`}`
		`- if(license["Magnet link"] != link){`
		`+ if (!(license["Magnet link"] == link \|\| license["URL"] == link)){`
		return [false, `License magnet link does not match for "${id}".`];
		`}`
		return [true, `Recognized license: "${id}".`];

quidam commented 5 years ago

I still found some issues with the emailselfdefense.org example, in which the license is stated as
// @license magnet:?xt=urn:btih:1f739d935676111cfff4b4693e3816e664797050&dn=gpl-3.0.txt GPL-v3-or-Later

My suggested changes are:
* Allow for length >= 4 since somebody may write "GPL2.0 or later" or something like that. It should still work if the tag follows the new recommendations.
* Match the link to the URL or Magnet Link fields in the licenses table. Currently it matches only by id, and the link field is only used to fail the match if the link is not the same as in the table.

My reasoning comes from the documentation stating that the id is not used for matching, only the magnet link is. My changes should accommodate for people who use a custom link, as long as the license identifier matches the one in our license table, or for people who use a custom identifier, as long as the link is in our table.

I'd like an opinion on both the logic and my code, since it is very likely that I may be doing something stupid.

gioma1 commented 5 years ago

Clever, this should catch even the sloppiest cases.

Just a few very minor nits about the code:
1. Please use "let" instead of "var" whenever possible to narrow the variable scope, especially in loops like for (let key in licenses).
2. Please use strict (in)equality operators (=== and !==) unless you've got actual reasons not too.
3. Please put spaces around operators (licenses[key]["URL"] === link).

Nevertheless It would work as it is, and the logic looks good!