From d9843a71ced5fce4da8ab714c6791cd24cda09f2 Mon Sep 17 00:00:00 2001 From: Paul Millar Date: Tue, 5 Jul 2016 13:24:52 +0200 Subject: [PATCH] dcap: add support for clients presenting more version metadata Motivation: A patch for dcap library adds more version information; in principle this allows dCache to apply version-specific behaviour such as banning badly behaved versions. Unfortunately, this cannot be applied retrospectively, but we can provide this in the future. Modification: Add support for parsing client-supplied version information. Backwards compatibility is provided for older clients. Result: The dcap door is aware of the exact dcap version, if the dcap client provides this information. Target: master Request: 2.16 Request: 2.15 Request: 2.14 Request: 2.13 Requires-notes: yes Requires-book: no Patch: https://rb.dcache.org/r/9510 Acked-by: Tigran Mkrtchyan Conflicts: modules/dcache-dcap/src/main/java/diskCacheV111/doors/DCapDoorInterpreterV3.java modules/dcache-dcap/src/main/java/diskCacheV111/doors/DcapDoorSettings.java --- .../doors/DCapDoorInterpreterV3.java | 170 ++++++++++++++---- .../doors/DCapDoorInterpreterV3Tests.java | 121 +++++++++++++ 2 files changed, 252 insertions(+), 39 deletions(-) create mode 100644 modules/dcache-dcap/src/test/java/diskCacheV111/doors/DCapDoorInterpreterV3Tests.java diff --git a/modules/dcache-dcap/src/main/java/diskCacheV111/doors/DCapDoorInterpreterV3.java b/modules/dcache-dcap/src/main/java/diskCacheV111/doors/DCapDoorInterpreterV3.java index f77cf4ab5e5..e9975320c13 100644 --- a/modules/dcache-dcap/src/main/java/diskCacheV111/doors/DCapDoorInterpreterV3.java +++ b/modules/dcache-dcap/src/main/java/diskCacheV111/doors/DCapDoorInterpreterV3.java @@ -1,5 +1,8 @@ package diskCacheV111.doors; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ComparisonChain; +import com.google.common.collect.Ordering; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -182,8 +185,7 @@ static DcapCommand get(String s) { */ private int _gid = NameSpaceProvider.DEFAULT; - private int _majorVersion; - private int _minorVersion; + private Version _version; private Date _startedTS; private Date _lastCommandTS; @@ -383,44 +385,131 @@ private LoginReply login(String user) return login; } - private static class Version implements Comparable { + static class Version + { private final int _major; private final int _minor; - private Version( String versionString ){ + private final Integer _bugfix; + private final String _package; + + @VisibleForTesting + protected Version(int major, int minor) + { + this(major, minor, null, null); + } + + @VisibleForTesting + protected Version(int major, int minor, Integer bugfix, String pack) + { + _major = major; + _minor = minor; + _bugfix = bugfix; + _package = pack; + } + + /** + * Generate a predicate Version: 1.2 matches 1.2, 1.2.3 and 1.2.3-4. + * @param versionString + */ + Version(String versionString) + { StringTokenizer st = new StringTokenizer(versionString,"."); _major = Integer.parseInt(st.nextToken()); _minor = Integer.parseInt(st.nextToken()); + if (st.hasMoreTokens()) { + st = new StringTokenizer(st.nextToken(), "-"); + _bugfix = Integer.parseInt(st.nextToken()); + _package = st.hasMoreTokens() ? st.nextToken() : null; + } else { + _bugfix = null; + _package = null; + } } - @Override - public int compareTo( Version other ){ - return _major != other._major ? - Integer.compare(_major, other._major) : Integer.compare(_minor, other._minor); + + public int getMajor() + { + return _major; } - @Override - public String toString(){ return ""+_major+"."+_minor ; } - private Version( int major , int minor ){ - _major = major ; - _minor = minor ; + + public int getMinor() + { + return _minor; } - @Override - public boolean equals(Object obj ) { - if( obj == this ) { - return true; - } - if( !(obj instanceof Version) ) { - return false; - } - return ((Version)obj)._major == this._major && ((Version)obj)._minor == this._minor; + /** + * Similar to compareTo but use this Version as a predicate. For + * example if this is Version(1,2) then matches returns + * + * -1 for Version(1,1), Version(1,1,5) and Version(1,1,5,"3") + * + * 0 for Version(1,2), Version(1,2,5) and Version(1,2,5,"3") + * + * 1 for Version(1,3), Version(1,3,5) and Version(1,3,5,"3") + * + * Note that this method is asymmetric if the number of defined + * elements is the same; e.g., + * + * Version(1,2).matches(Version(1,3)) is 1 + * Version(1,3).matches(Version(1,2)) is -1 + * + * but not asymmetric if the number of defined elements differs; e.g., + * + * Version(1,2).matches(Version(1,2,3)) is 0 + * Version(1,2,3).matches(Version(1,2)) is 1 + */ + public int matches(Version other) + { + ComparisonChain cc = ComparisonChain.start() + .compare(_major, other._major) + .compare(_minor, other._minor); + if (_bugfix != null) { + cc = cc.compare(_bugfix, other._bugfix, Ordering.natural().nullsFirst()); + } + if (_package != null) { + cc = cc.compare(_package, other._package, Ordering.natural().nullsFirst()); + } + return cc.result(); } + @Override - public int hashCode() { - return _minor ^ _major; + public String toString() + { + StringBuilder sb = new StringBuilder(); + sb.append(_major).append('.').append(_minor); + if (_bugfix != null) { + sb.append('.').append(_bugfix); + } + if (_package != null) { + sb.append('-').append(_package); + } + return sb.toString(); } + @Override + public boolean equals(Object obj) + { + if (obj == this) { + return true; + } + + if (!(obj instanceof Version)) { + return false; + } + Version o = (Version) obj; + return Objects.equals(_major, o._major) + && Objects.equals(_minor, o._minor) + && Objects.equals(_bugfix, o._bugfix) + && Objects.equals(_package, o._package); + } + + @Override + public int hashCode() { + return Objects.hash(_major, _minor, _bugfix, _package); + } } + private void installVersionRestrictions( String versionString ){ // // majorMin.minorMin[:majorMax.minorMax] @@ -477,20 +566,25 @@ public String com_hello( int sessionId , int commandId , VspArgs args ) CommandExitException("Command Syntax Exception", 2); } - Version version; - try{ - _majorVersion = Integer.parseInt( args.argv(2) ) ; - _minorVersion = Integer.parseInt( args.argv(3) ) ; - }catch(NumberFormatException e ){ + try { + int major = Integer.parseInt(args.argv(2)); + int minor = Integer.parseInt(args.argv(3)); + Integer bugfix = null; + String patch = null; + if (args.argc() > 2) { + bugfix = Integer.parseInt(args.argv(4)); + patch = args.argv(5); + } + _version = new Version(major, minor, bugfix, patch); + } catch (NumberFormatException e) { _log.error("Syntax error in client version number : {}", e.toString()); throw new CommandException("Invalid client version number", e); } - version = new Version( _majorVersion , _minorVersion ) ; - _log.debug("Client Version : {}", version); - if (version.compareTo(_minClientVersion) < 0 || (version.compareTo(_maxClientVersion) > 0)) { - - String error = "Client version rejected : "+version ; + _log.debug("Client Version : {}", _version); + if (_minClientVersion.matches(_version) > 0 || + _maxClientVersion.matches(_version) > 0) { + String error = "Client version rejected : "+_version; _log.error(error); throw new CommandExitException(error , 1 ); @@ -507,7 +601,7 @@ public String com_hello( int sessionId , int commandId , VspArgs args ) _uid = args.getIntOption("uid", _uid); _gid = args.getIntOption("gid", _gid); - return "0 0 "+_ourName+" welcome "+_majorVersion+" "+_minorVersion ; + return "0 0 "+_ourName+" welcome "+_version.getMajor()+" "+_version.getMinor(); } public String com_byebye( int sessionId , int commandId , VspArgs args ) throws CommandException @@ -2505,10 +2599,8 @@ public void close() { public void getInfo( PrintWriter pw ){ pw.println( " ----- DCapDoorInterpreterV3 ----------" ) ; pw.println( " User = " + Subjects.getDisplayName(_authenticatedSubject)); - pw.println( " Version = "+_majorVersion+"/"+_minorVersion ) ; - pw.println( " VLimits = "+ - (_minClientVersion==null?"*":_minClientVersion.toString() ) +":" + - (_maxClientVersion==null?"*":_maxClientVersion.toString() ) ) ; + pw.println( " Version = " + (_version == null ? "unknown" : _version)); + pw.println( " VLimits = " + _minClientVersion + ":" + _maxClientVersion); pw.println( " Started = "+_startedTS ) ; pw.println( " Last at = "+_lastCommandTS ) ; diff --git a/modules/dcache-dcap/src/test/java/diskCacheV111/doors/DCapDoorInterpreterV3Tests.java b/modules/dcache-dcap/src/test/java/diskCacheV111/doors/DCapDoorInterpreterV3Tests.java new file mode 100644 index 00000000000..7a3da29fbe9 --- /dev/null +++ b/modules/dcache-dcap/src/test/java/diskCacheV111/doors/DCapDoorInterpreterV3Tests.java @@ -0,0 +1,121 @@ +/* dCache - http://www.dcache.org/ + * + * Copyright (C) 2001 - 2016 Deutsches Elektronen-Synchrotron + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * 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 Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +package diskCacheV111.doors; + +import org.junit.Test; + +import diskCacheV111.doors.DCapDoorInterpreterV3.Version; + +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.assertThat; + +/** + * Tests for parts of DCapDoorInterpreter + */ +public class DCapDoorInterpreterV3Tests +{ + @Test + public void versionShouldEqual() + { + Version v1_2 = new Version(1, 2); + Version v1_1 = new Version(1, 1); + Version v2_2 = new Version(2, 2); + Version v1_1_1 = new Version(1,1,1,null); + Version v1_1_1_1 = new Version(1,1,1,"1"); + + assertThat(v1_1, is(equalTo(v1_1))); + assertThat(v1_2, is(equalTo(v1_2))); + assertThat(v2_2, is(equalTo(v2_2))); + assertThat(v1_1_1, is(equalTo(v1_1_1))); + assertThat(v1_1_1_1, is(equalTo(v1_1_1_1))); + + assertThat(new Version(1,1), is(equalTo(v1_1))); + assertThat(new Version(1,2), is(equalTo(v1_2))); + assertThat(new Version(2,2), is(equalTo(v2_2))); + assertThat(new Version(1,1,1,null), is(equalTo(v1_1_1))); + assertThat(new Version(1,1,1,"1"), is(equalTo(v1_1_1_1))); + + assertThat(v1_1, not(equalTo(v1_2))); + assertThat(v1_2, not(equalTo(v1_1))); + + assertThat(v2_2, not(equalTo(v1_2))); + assertThat(v1_2, not(equalTo(v2_2))); + + assertThat(v2_2, not(equalTo(v1_1))); + assertThat(v1_1, not(equalTo(v2_2))); + + assertThat(v1_1, is(not(equalTo(v1_1_1)))); + assertThat(v1_1_1, is(not(equalTo(v1_1)))); + + assertThat(v1_1, is(not(equalTo(v1_1_1_1)))); + assertThat(v1_1_1_1, is(not(equalTo(v1_1)))); + + assertThat(v1_1_1, is(not(equalTo(v1_1_1_1)))); + assertThat(v1_1_1_1, is(not(equalTo(v1_1_1)))); + } + + @Test + public void versionShouldConstructFromString() + { + assertThat(new Version("1.2"), is(equalTo(new Version(1,2)))); + assertThat(new Version("1.2.3"), is(equalTo(new Version(1,2,3,null)))); + assertThat(new Version("1.2.3-4"), is(equalTo(new Version(1,2,3,"4")))); + } + + @Test + public void versionShouldMatchTo() + { + Version v1_2 = new Version(1, 2); + Version v1_1 = new Version(1, 1); + Version v2_1 = new Version(2, 1); + Version v1_1_1 = new Version(1, 1, 1, null); + Version v1_1_2 = new Version(1, 1, 2, null); + Version v1_1_1_1 = new Version(1, 1, 1, "1"); + Version v1_1_1_2 = new Version(1, 1, 1, "2"); + + assertThat(v1_1.matches(v1_1), is(equalTo(0))); + assertThat(v1_2.matches(v1_2), is(equalTo(0))); + assertThat(v2_1.matches(v2_1), is(equalTo(0))); + assertThat(v1_1_1.matches(v1_1_1), is(equalTo(0))); + assertThat(v1_1_2.matches(v1_1_2), is(equalTo(0))); + assertThat(v1_1_1_1.matches(v1_1_1_1), is(equalTo(0))); + assertThat(v1_1_1_2.matches(v1_1_1_2), is(equalTo(0))); + assertThat(v1_1.matches(v1_1_1), is(equalTo(0))); + assertThat(v1_1.matches(v1_1_1_1), is(equalTo(0))); + assertThat(v1_1_1.matches(v1_1_1_1), is(equalTo(0))); + + assertThat(v1_1.matches(v1_2), is(equalTo(-1))); + assertThat(v1_2.matches(v1_1), is(equalTo(1))); + + assertThat(v1_1.matches(v1_2), is(equalTo(-1))); + assertThat(v2_1.matches(v1_1), is(equalTo(1))); + + assertThat(v1_2.matches(v2_1), is(equalTo(-1))); + assertThat(v2_1.matches(v1_2), is(equalTo(1))); + + assertThat(v1_1_1.matches(v1_1), is(equalTo(1))); + + assertThat(v1_1_1.matches(v1_1_2), is(equalTo(-1))); + assertThat(v1_1_2.matches(v1_1_1), is(equalTo(1))); + + assertThat(v1_1_1_1.matches(v1_1_1), is(equalTo(1))); + + assertThat(v1_1_1_1.matches(v1_1_1_2), is(equalTo(-1))); + assertThat(v1_1_1_2.matches(v1_1_1_1), is(equalTo(1))); + } +}