mina-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Guillaume Nodet <gno...@apache.org>
Subject Re: mina-sshd git commit: [SSHD-805] Return a clone of the RFC8268 modulu number bytes to avoid inadvertent modification of the shared instance
Date Mon, 05 Mar 2018 13:24:43 GMT
2018-03-05 14:18 GMT+01:00 <lgoldstein@apache.org>:

> Repository: mina-sshd
> Updated Branches:
>   refs/heads/master f38e14df3 -> 2d43f6fe2
>
>
> [SSHD-805] Return a clone of the RFC8268 modulu number bytes to avoid
> inadvertent modification of the shared instance
>
>
> Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
> Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/2d43f6fe
> Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/2d43f6fe
> Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/2d43f6fe
>
> Branch: refs/heads/master
> Commit: 2d43f6fe23ffdb6c18a56e54202c70813b411938
> Parents: f38e14d
> Author: Goldstein Lyor <lyor@c-b4.com>
> Authored: Mon Mar 5 15:18:34 2018 +0200
> Committer: Goldstein Lyor <lyor@c-b4.com>
> Committed: Mon Mar 5 15:18:34 2018 +0200
>
> ----------------------------------------------------------------------
>  .../org/apache/sshd/common/kex/DHGroupData.java | 20 +++++++++++---------
>  .../sshd/common/kex/BuiltinDHFactoriesTest.java |  4 ++--
>  2 files changed, 13 insertions(+), 11 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/
> 2d43f6fe/sshd-core/src/main/java/org/apache/sshd/common/
> kex/DHGroupData.java
> ----------------------------------------------------------------------
> diff --git a/sshd-core/src/main/java/org/apache/sshd/common/kex/DHGroupData.java
> b/sshd-core/src/main/java/org/apache/sshd/common/kex/DHGroupData.java
> index ee0be8a..8570a6d 100644
> --- a/sshd-core/src/main/java/org/apache/sshd/common/kex/DHGroupData.java
> +++ b/sshd-core/src/main/java/org/apache/sshd/common/kex/DHGroupData.java
> @@ -19,6 +19,7 @@
>  package org.apache.sshd.common.kex;
>
>  import java.io.BufferedReader;
> +import java.io.FileNotFoundException;
>  import java.io.IOError;
>  import java.io.IOException;
>  import java.io.InputStream;
> @@ -122,21 +123,23 @@ public final class DHGroupData {
>          return readOakleyGroup("group18.prime");
>      }
>
> -    static byte[] readOakleyGroup(String name) {
> -        return OAKLEY_GROUPS.computeIfAbsent(name, DHGroupData::
> doReadOakleyGroup);
> +    public static byte[] readOakleyGroup(String name) {
>

The method was private, but making it public would need just a bit of
javadoc explaining the input/output.
I don't really see the need, since we have the getP16() and similar methods
that are already public though...


> +        byte[] value = OAKLEY_GROUPS.computeIfAbsent(name, DHGroupData::
> doReadOakleyGroup);
> +        return (value == null) ? null : value.clone();
>

value can't be null, so this check is unnecessary.


>      }
>
> -    private static byte[] doReadOakleyGroup(String name) {
> +    public static byte[] doReadOakleyGroup(String name) {
>

I don't think that one should be public given readOakleyGroup has been made
public already.
If you really want to keep that one public, it should be renamed as doXxx
are usually meant to be private methods.


>          try (InputStream is = DHGroupData.class.getResourceAsStream(name))
> {
>              if (is == null) {
> -                throw new IOException("Resource not found: " + name);
> +                throw new FileNotFoundException("Resource not found: " +
> name);
>              }
> +
>              try (BufferedReader br = new BufferedReader(new
> InputStreamReader(is, StandardCharsets.UTF_8))) {
>                  String str = br.lines()
> -                        .filter(s -> !s.startsWith("#"))
> -                        .map(s -> s.replaceAll("\\s", ""))
> -                        .collect(Collectors.joining());
> -                byte[] group = new byte[str.length() / 2 + 1];
> +                    .filter(s -> !s.startsWith("#"))
> +                    .map(s -> s.replaceAll("\\s", ""))
> +                    .collect(Collectors.joining());
> +                byte[] group = new byte[(str.length() / 2) + 1];
>                  group[0] = 0;
>                  for (int l = 1; l < group.length; l++) {
>                      group[l] = (byte) Integer.parseInt(str.substring(l *
> 2 - 2, l * 2), 16);
> @@ -147,5 +150,4 @@ public final class DHGroupData {
>              throw new IOError(e);
>          }
>      }
> -
>  }
>
> http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/
> 2d43f6fe/sshd-core/src/test/java/org/apache/sshd/common/
> kex/BuiltinDHFactoriesTest.java
> ----------------------------------------------------------------------
> diff --git a/sshd-core/src/test/java/org/apache/sshd/common/kex/BuiltinDHFactoriesTest.java
> b/sshd-core/src/test/java/org/apache/sshd/common/kex/
> BuiltinDHFactoriesTest.java
> index 989968a..77b712e 100644
> --- a/sshd-core/src/test/java/org/apache/sshd/common/kex/
> BuiltinDHFactoriesTest.java
> +++ b/sshd-core/src/test/java/org/apache/sshd/common/kex/
> BuiltinDHFactoriesTest.java
> @@ -174,7 +174,7 @@ public class BuiltinDHFactoriesTest extends
> BaseTestSupport {
>
>      @Test
>      public void testDHGRead() throws Exception {
> -        assertArrayEquals(DHGroupData.getP1(),
> DHGroupData.readOakleyGroup("group2.prime"));
> -        assertArrayEquals(DHGroupData.getP14(),
> DHGroupData.readOakleyGroup("group14.prime"));
> +        assertArrayEquals("P1", DHGroupData.getP1(),
> DHGroupData.readOakleyGroup("group2.prime"));
> +        assertArrayEquals("P14", DHGroupData.getP14(),
> DHGroupData.readOakleyGroup("group14.prime"));
>      }
>  }
>
>


-- 
------------------------
Guillaume Nodet

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message