[sword-devel] Module version numbers
DM Smith
dmsmith at crosswire.org
Tue Sep 25 18:51:54 EDT 2018
Initially, JSword only accepted Major.Minor, converting that into a decimal. That was based on a poor understanding of the CrossWire wiki. That was changed to match SWORD’s 4 part “number” with a minor modification.
JSword will reject any version that doesn’t match the pattern: ^(\d+)(?:.(\d+))?(?:.(\d+))?(?:.(\d+))?$
I think I encoded this to allow pretty much any single non-digit separator. The . will match any single character and \d+ is greedy. So it will accept Karl’s encoding.
The wiki on the conf clearly states: “Do not use non-numbers, such as 1.4a”
(See: http://wiki.crosswire.org/DevTools:conf_Files <http://wiki.crosswire.org/DevTools:conf_Files>)
The conf also documents that the third part should be incremented when only the conf is affected. The first or second when the module is different.
Here is JSword's code:
/**
* Distribution License:
* JSword is free software; you can redistribute it and/or modify it under
* the terms of the GNU Lesser General Public License, version 2.1 or later
* as published by the Free Software Foundation. This program is distributed
* in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even
* the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
* See the GNU Lesser General Public License for more details.
*
* The License is available on the internet at:
* http://www.gnu.org/copyleft/lgpl.html
* or by writing to:
* Free Software Foundation, Inc.
* 59 Temple Place - Suite 330
* Boston, MA 02111-1307, USA
*
* © CrossWire Bible Society, 2011 - 2016
*
*/
package org.crosswire.common.util;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
/**
* Version is an immutable representation of dotted "number" consisting of 1 to 4 parts.
*
* <p>
* Here is the grammar for version strings:
* </p>
* <pre>
* version ::= major('.'minor('.'micro('.'nano)?)?)?
* major ::= [0-9]+
* minor ::= [0-9]+
* micro ::= [0-9]+
* nano ::= [0-9]+
* </pre>
*
* @see gnu.lgpl.License The GNU Lesser General Public License for details.
* @author DM Smith
*/
public class Version implements Comparable<Version> {
public static final Pattern VERSION_PATTERN = Pattern.compile("^(\\d+)(?:.(\\d+))?(?:.(\\d+))?(?:.(\\d+))?$");
/**
* Created a version identifier from the specified string.
*
* @param version String representation of the version identifier.
* @throws IllegalArgumentException If <code>version</code> is improperly
* formatted.
*/
public Version(String version) {
if (version == null) {
throw new IllegalArgumentException("Null version not allowed.");
}
this.original = version;
this.parts = new int[] { -1, -1, -1, -1 };
Matcher matcher = VERSION_PATTERN.matcher(this.original);
if (matcher.matches()) {
int count = matcher.groupCount();
for (int i = 1; i <= count; i++) {
String part = matcher.group(i);
if (part == null) {
break;
}
parts[i - 1] = Integer.parseInt(part);
}
} else {
throw new IllegalArgumentException("invalid: " + version);
}
}
/**
* Returns the original string representation of this version identifier.
*
* @return The original string representation of this version identifier.
*/
@Override
public String toString() {
return original;
}
@Override
public int hashCode() {
return original.hashCode();
}
/**
* Compares this <code>Version</code> object to another object.
*
* <p>
* A version is considered to be equal to another version if all the
* parts are equal.</p>
*
* @param object The <code>Version</code> object to be compared.
* @return true if the two objects are equal.
*/
@Override
public boolean equals(Object object) {
if (!(object instanceof Version)) {
return false;
}
Version that = (Version) object;
if (that == this) {
return true;
}
for (int i = 0; i < parts.length; i++) {
if (parts[i] != that.parts[i]) {
return false;
}
}
return true;
}
/**
* Compares this <code>Version</code> object to another object.
*
* <p>
* The comparison considers each of the parts (major, minor, micro, nano) in turn,
* comparing like with like. At any point the comparison is not equal, a result is
* known.
* </p>
*
* @param object The <code>Version</code> object to be compared.
* @return A negative integer, zero, or a positive integer if this object is
* less than, equal to, or greater than the specified
* <code>Version</code> object.
* @throws ClassCastException If the specified object is not a <code>Version</code>.
*/
public int compareTo(Version object) {
if (object == this) {
return 0;
}
for (int i = 0; i < parts.length; i++) {
int result = parts[i] - object.parts[i];
if (result != 0) {
return result;
}
}
return 0;
}
private final String original;
private final int[] parts;
}
> On Sep 25, 2018, at 7:07 AM, David Haslam <dfhdfh at protonmail.com> wrote:
>
> Thanks Peter,
>
> We should therefore also encourage Karl to review and update any such modules in the Xiphos repository.
>
> The reason I added my aside was that version values are clearly used for comparison purposes to determine whether a module has been updated.
> Version is one of maybe only a few configuration keys upon which calculations are made.
> This logically led me to pose the question about potential buffer overflows, even though I'm not part of the SWORD coding team.
> cf. My awareness of what can be exploited from a buffer overflow or the like has been honed by 13 years of watching the Security Now! webcasts on twit.tv
>
> Further aside: How does JSword process the same key?
>
> David
>
> Sent from ProtonMail Mobile
>
>
> On Tue, Sep 25, 2018 at 11:54, Peter Von Kaehne <refdoc at gmx.net <mailto:refdoc at gmx.net>> wrote:
>> I think there are two aspects then - version numbers should be correct in current modules. I think the bulk of the modules you listed are Xiphos, not CrossWire, but at least one is issued by CrossWire, maybe more. I will fix CrossWire's set.
>>
>> The other is - wrong version numbers should not crash the engine or produce unreliable/undefined results. Or at least the engine should go down with a decent error message. In essence all places where wrong module conf file entries can cause grief in the engine, I guess some level of sanity checking should happen - Particularly if we want to encourage other publishers to take up offering repositories.
>>
>> Peter
>>
>>
>>
>> > Gesendet: Dienstag, 25. September 2018 um 10:56 Uhr
>> > Von: "Jaak Ristioja" <jaak at ristioja.ee>
>> > An: sword-devel at crosswire.org
>> > Betreff: Re: [sword-devel] Module version numbers
>> >
>> > > Aside: Are there any limits to the number of dot separators in the
>> > Version value, or to the number of digits in total or in any part?
>> > > Would SWORD crash with a buffer overflow were it to encounter an
>> > inordinately long Version?
>> >
>> > The relevant code to parse the version string is in the SWVersion
>> > constructor:
>> >
>> > SWVersion::SWVersion(const char *version) {
>> > char *buf = new char[ strlen(version) + 1 ];
>> > char *tok;
>> > major = minor = minor2 = minor3 = -1;
>> >
>> > strcpy(buf, version);
>> > tok = strtok(buf, ".");
>> > if (tok)
>> > major = atoi(tok);
>> > tok = strtok(0, ".");
>> > if (tok)
>> > minor = atoi(tok);
>> > tok = strtok(0, ".");
>> > if (tok)
>> > minor2 = atoi(tok);
>> > tok = strtok(0, ".");
>> > if (tok)
>> > minor3 = atoi(tok);
>> > delete [] buf;
>> > }
>> >
>> > Very long version strings can only crash it if this runs out of memory.
>> > Other than that, it will just return an incorrect version. There are no
>> > limits to the number of dot separators, but only up to 4 version
>> > components separated by dots are actually parsed. AFAIK, the behavior of
>> > atoi() is undefined for invalid input. On my system, the results are as
>> > follows:
>> >
>> > "9.1" -> 9.1
>> > "99.1" -> 99.1
>> > "999.1" -> 999.1
>> > "9999.1" -> 9999.1
>> > "99999.1" -> 99999.1
>> > "999999.1" -> 999999.1
>> > "9999999.1" -> 9999999.1
>> > "99999999.1" -> 99999999.1
>> > "999999999.1" -> 999999999.1
>> > "9999999999.1" -> 1410065407.1
>> > "99999999999.1" -> 1215752191.1
>> > "999999999999.1" -> -727379969.1
>> > "9999999999999.1" -> 1316134911.1
>> > "99999999999999.1" -> 276447231.1
>> > "999999999999999.1" -> -1530494977.1
>> > "9999999999999999.1" -> 1874919423.1
>> > "99999999999999999.1" -> 1569325055.1
>> > "999999999999999999.1" -> -1486618625.1
>> > "9999999999999999999.1" -> -1.1
>> > "99999999999999999999.1" -> -1.1
>> >
>> >
>> > J
>> >
>> > On 25.09.2018 12:03, David Haslam wrote:
>> > > Ignoring the spurious SwordVersion hit, it seems that the string after the dash is a date in six digit format.
>> > >
>> > > IMHO, these modules should simply be re-issued with the dates recorded in the respective History key.
>> > >
>> > > It's not worth the effort to make the API parse these as they are now.
>> > > The dash is a nonconformance to what should be in the Version key.
>> > >
>> > > Aside: Are there any limits to the number of dot separators in the Version value, or to the number of digits in total or in any part?
>> > > Would SWORD crash with a buffer overflow were it to encounter an inordinately long Version?
>> > >
>> > > Best regards,
>> > >
>> > > David
>> > >
>> > > Sent from ProtonMail Mobile
>> > >
>> > > On Tue, Sep 25, 2018 at 09:44, Jaak Ristioja <jaak at ristioja.ee> wrote:
>> > >
>> > >> Hello!
>> > >>
>> > >> Most modules include version numbers matching the regular expression
>> > >>
>> > >> ^[0-9]+(.[0-9]+)*$
>> > >>
>> > >> However, looking at the .conf files, there are version fields with
>> > >> values also containing dashes:
>> > >>
>> > >> ~/.sword/mods.d $ grep -E 'Version=.*-' *
>> > >> 2tgreek.conf:Version=2.7-120109
>> > >> invstrongsrealgreek.conf:Version=1.4-090107
>> > >> jesermons.conf:SwordVersion=2017-05-24
>> > >> strongsrealgreek.conf:Version=1.5-150704
>> > >> tischmorph.conf:Version=2.7-120109
>> > >>
>> > >> How should these be interpreted? Should 1.2-3.4 be interpreted as
>> > >> (1).(2-3).(4) or (1.2)-(3.4)? It seems that SWVersion interprets such as
>> > >> just 1.2.4 (without the -3 entirely).
>> > >>
>> > >> God bless!
>> > >> J
>> > >>
>> > >> _______________________________________________
>> > >> sword-devel mailing list: sword-devel at crosswire.org
>> > >> http://www.crosswire.org/mailman/listinfo/sword-devel
>> > >> Instructions to unsubscribe/change your settings at above page
>> > >>
>> > >>
>> > >> _______________________________________________
>> > >> sword-devel mailing list: sword-devel at crosswire.org
>> > >> http://www.crosswire.org/mailman/listinfo/sword-devel
>> > >> Instructions to unsubscribe/change your settings at above page
>> >
>> >
>> > _______________________________________________
>> > sword-devel mailing list: sword-devel at crosswire.org
>> > http://www.crosswire.org/mailman/listinfo/sword-devel
>> > Instructions to unsubscribe/change your settings at above page
>> >
>>
>> _______________________________________________
>> sword-devel mailing list: sword-devel at crosswire.org
>> http://www.crosswire.org/mailman/listinfo/sword-devel
>> Instructions to unsubscribe/change your settings at above page
> _______________________________________________
> sword-devel mailing list: sword-devel at crosswire.org
> http://www.crosswire.org/mailman/listinfo/sword-devel
> Instructions to unsubscribe/change your settings at above page
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.crosswire.org/pipermail/sword-devel/attachments/20180925/bf5f2d9c/attachment-0001.html>
More information about the sword-devel
mailing list