Java 8/Junit 5 Migration #45
Conversation
… 16 bytes per RFCs.
…-ipv6prefixattribute
* 'master' of https://github.com/ctran/TinyRadius: Bump ipaddress from 5.3.1 to 5.3.3 (ctran#38) Bumped commons-logging to 1.2 (ctran#42) Run CI action on PR changes (ctran#44)
|
Appreciate the effort. I'll need some time to review all the changes. |
* 'master' of https://github.com/ctran/TinyRadius: Update ci.yml Update ci.yml Update ci.yml
![sonatype-lift[bot]](https://avatars.githubusercontent.com/in/9843?s=60&v=4){kind=small}
| @@ -234,7 +234,7 @@ public static RadiusAttribute createRadiusAttribute(Dictionary dictionary, int v | |||
| AttributeType at = dictionary.getAttributeTypeByCode(vendorId, attributeType); | |||
| if (at != null && at.getAttributeClass() != null) { | |||
| try { | |||
| attribute = (RadiusAttribute) at.getAttributeClass().newInstance(); | |||
| attribute = at.getAttributeClass().newInstance(); | |||
There was a problem hiding this comment.
ClassNewInstance: Class.newInstance() bypasses exception checking; prefer getDeclaredConstructor().newInstance() (details)
(at-me in a reply with help or ignore)
| LinkedList result = new LinkedList(); | ||
| for (Iterator i = subAttributes.iterator(); i.hasNext();) { | ||
| RadiusAttribute a = (RadiusAttribute) i.next(); | ||
| LinkedList<RadiusAttribute> result = new LinkedList<>(); |
There was a problem hiding this comment.
JdkObsolete: It is very rare for LinkedList to out-perform ArrayList or ArrayDeque. Avoid it unless you're willing to invest a lot of time into benchmarking. Caveat: LinkedList supports null elements, but ArrayDeque does not. (details)
(at-me in a reply with help or ignore)
| LinkedList result = new LinkedList(); | ||
| for (Iterator i = attributes.iterator(); i.hasNext();) { | ||
| RadiusAttribute a = (RadiusAttribute) i.next(); | ||
| LinkedList<RadiusAttribute> result = new LinkedList<>(); |
There was a problem hiding this comment.
JdkObsolete: It is very rare for LinkedList to out-perform ArrayList or ArrayDeque. Avoid it unless you're willing to invest a lot of time into benchmarking. Caveat: LinkedList supports null elements, but ArrayDeque does not. (details)
(at-me in a reply with help or ignore)
| for (Iterator i = attributes.iterator(); i.hasNext();) { | ||
| RadiusAttribute a = (RadiusAttribute) i.next(); | ||
| public List<VendorSpecificAttribute> getVendorAttributes(int vendorId) { | ||
| LinkedList<VendorSpecificAttribute> result = new LinkedList<>(); |
There was a problem hiding this comment.
JdkObsolete: It is very rare for LinkedList to out-perform ArrayList or ArrayDeque. Avoid it unless you're willing to invest a lot of time into benchmarking. Caveat: LinkedList supports null elements, but ArrayDeque does not. (details)
(at-me in a reply with help or ignore)
| @@ -22,9 +21,10 @@ | |||
| public class TestServer { | |||
|
|
|||
| /** | |||
| * @throws IOException | |||
| * @throws Exception | |||
There was a problem hiding this comment.
MissingSummary: A summary line is required on public/protected Javadocs. (details)
(at-me in a reply with help or ignore)
| @@ -192,12 +192,11 @@ private static void includeDictionaryFile(WritableDictionary dictionary, StringT | |||
| * string|octets|integer|date|ipaddr|ipv6addr|ipv6prefix | |||
| * @return RadiusAttribute class or descendant | |||
| */ | |||
| private static Class getAttributeTypeClass(int attributeType, String typeStr) { | |||
| Class type = RadiusAttribute.class; | |||
| private static Class<? extends RadiusAttribute> getAttributeTypeClass(int attributeType, String typeStr) { | |||
There was a problem hiding this comment.
UnusedVariable: The parameter 'attributeType' is never read. (details)
(at-me in a reply with help or ignore)
| @@ -368,6 +366,7 @@ public static RadiusPacket communicate(RadiusEndpoint remoteServer, RadiusPacket | |||
| * | |||
| * @return local socket | |||
| * @throws SocketException | |||
| * Socket Exception | |||
| */ | |||
| protected DatagramSocket getSocket() throws SocketException { | |||
| if (serverSocket == null || serverSocket.isClosed()) { | |||
There was a problem hiding this comment.
THREAD_SAFETY_VIOLATION: Read/Write race. Non-private method RadiusClient.getSocket() reads without synchronization from this.serverSocket. Potentially races with write in method RadiusClient.authenticate(...).
Reporting because another access to the same memory occurs on a background thread, although this access may not.
(at-me in a reply with help or ignore)
| */ | ||
| public void setSocketTimeout(int socketTimeout) throws SocketException { | ||
| if (socketTimeout < 1) | ||
| throw new IllegalArgumentException("socket tiemout must be positive"); | ||
| throw new IllegalArgumentException("socket timeout must be positive"); | ||
| this.socketTimeout = socketTimeout; | ||
| if (serverSocket != null) |
There was a problem hiding this comment.
THREAD_SAFETY_VIOLATION: Read/Write race. Non-private method RadiusClient.setSocketTimeout(...) reads without synchronization from this.serverSocket. Potentially races with write in method RadiusClient.authenticate(...).
Reporting because another access to the same memory occurs on a background thread, although this access may not.
(at-me in a reply with help or ignore)
…ft warning for race conditions)
| @@ -312,8 +315,7 @@ public RadiusPacket communicate(RadiusPacket request, int port) throws IOExcepti | |||
| DatagramPacket packetIn = new DatagramPacket(new byte[RadiusPacket.MAX_PACKET_LENGTH], RadiusPacket.MAX_PACKET_LENGTH); | |||
| DatagramPacket packetOut = makeDatagramPacket(request, port); | |||
|
|
|||
| DatagramSocket socket = getSocket(); | |||
| try { | |||
| try (DatagramSocket socket = getSocket()) { | |||
There was a problem hiding this comment.
THREAD_SAFETY_VIOLATION: Read/Write race. Non-private method RadiusClient.communicate(...) indirectly reads with synchronization from this.socketTimeout. Potentially races with unsynchronized write in method RadiusClient.setSocketTimeout(...).
Reporting because another access to the same memory occurs on a background thread, although this access may not.
(at-me in a reply with help or ignore)
| */ | ||
| protected DatagramSocket getSocket() throws SocketException { | ||
| protected synchronized DatagramSocket getSocket() throws SocketException { | ||
| if (serverSocket == null || serverSocket.isClosed()) { | ||
| serverSocket = new DatagramSocket(); | ||
| serverSocket.setSoTimeout(getSocketTimeout()); |
There was a problem hiding this comment.
THREAD_SAFETY_VIOLATION: Read/Write race. Non-private method RadiusClient.getSocket() indirectly reads with synchronization from this.socketTimeout. Potentially races with unsynchronized write in method RadiusClient.setSocketTimeout(...).
Reporting because this access may occur on a background thread.
(at-me in a reply with help or ignore)
| */ | ||
| public void setSocketTimeout(int socketTimeout) throws SocketException { | ||
| if (socketTimeout < 1) | ||
| throw new IllegalArgumentException("socket tiemout must be positive"); | ||
| throw new IllegalArgumentException("socket timeout must be positive"); | ||
| this.socketTimeout = socketTimeout; |
There was a problem hiding this comment.
THREAD_SAFETY_VIOLATION: Unprotected write. Non-private method RadiusClient.setSocketTimeout(...) writes to field this.socketTimeout outside of synchronization.
Reporting because this access may occur on a background thread.
(at-me in a reply with help or ignore)
…ft warning for race conditions)
|
Merged recent CI changes and fixed sonatype lift warnings (the ones marked new anyway). |
ctran
force-pushed
the
master
branch
3 times, most recently
from
67019ad to
5526820
Compare
Was having a fair number of warnings/issues compiling for Java 8/11 so thought I'd update some of the code for use with current supported JVMs. Mainly updates to migrate toward Java 8/11 code style.
I only use library with RADIUS client so server code not tested. Feel free to merge changes or toss as see fit. Happy to make additional changes if needed.