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
| 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)
|
|
||
| /** | ||
| * @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)
| */ | ||
| 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)
| * 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)
| 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)
|
|
||
| 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 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)
| 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). |
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.