garydgregory commented on code in PR #331:
URL: https://github.com/apache/commons-net/pull/331#discussion_r1973829511
##########
src/main/java/org/apache/commons/net/tftp/TFTP.java:
##########
@@ -112,6 +112,12 @@ public static final String getModeName(final int mode) {
/** A datagram used to minimize memory allocation in bufferedSend() */
private DatagramPacket sendDatagram;
+ /** Internal packet size, which is the data octet size plus 4 (for TFTP
header octets) */
+ private int packetSize = TFTPPacket.SEGMENT_SIZE + 4;
Review Comment:
Don't duplicate the expression that defines `PACKET_SIZE`, reuse that
constant.
##########
src/main/java/org/apache/commons/net/tftp/TFTPErrorPacket.java:
##########
@@ -62,6 +62,12 @@ public final class TFTPErrorPacket extends TFTPPacket {
/** The no such user error code according to RFC 783, value 7. */
public static final int NO_SUCH_USER = 7;
+ /**
+ * The invalid options error code according to RFC 2347, value 8.
+ * @since 3.12.0
+ * */
Review Comment:
Fix formatting.
##########
src/main/java/org/apache/commons/net/tftp/TFTPOptionsAckPacket.java:
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.net.tftp;
+
+import java.net.DatagramPacket;
+import java.net.InetAddress;
+import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * A final class derived from TFTPPacket defining the TFTP OACK (Option
Acknowledgment) packet type.
+ * <p>
+ * Details regarding this packet type can be found in RFC 2347.
+ * </p>
+ *
+ * @since 3.12.0
+ */
+public final class TFTPOptionsAckPacket extends TFTPPacket {
+
+ private final Map<String, String> options;
+
+ /**
+ * Constructs an OACK packet informing which options are accepted.
+ *
+ * @param address The host to which the packet is going to be sent.
+ * @param port The port to which the packet is going to be sent.
+ * @param options The options accepted.
+ */
+ public TFTPOptionsAckPacket(InetAddress address, int port, Map<String,
String> options) {
+ super(OACK, address, port);
+ this.options = new HashMap<>(options);
+ }
+
+ /**
+ * Gets the options extensions being acknowledged.
+ *
+ * @return The options being acknowledged.
+ */
+ public Map<String, String> getOptions() {
+ return options;
+ }
+
+ /**
+ * Creates a UDP datagram containing all the accepted options data in the
proper format.
+ * <p>
+ * This is a method exposed to the programmer in case he
+ * wants to implement his own TFTP client instead of using the {@link
org.apache.commons.net.tftp.TFTPClient} class. Under normal circumstances, you
should
+ * not have a need to call this method.
+ * </p>
+ *
+ * @return A UDP datagram containing the TFTP OACK packet.
+ */
+ @Override
+ public DatagramPacket newDatagram() {
+ int optionsLength = 0;
+ for (Map.Entry<String, String> entry : options.entrySet()) {
+ optionsLength += entry.getKey().length() + 1 +
entry.getValue().length() + 1;
+ }
+
+ byte[] data = new byte[2 + optionsLength];
+ data[0] = 0;
+ data[1] = (byte) type;
+ writeOptionsData(data, 2);
+
+ return new DatagramPacket(data, data.length, getAddress(), getPort());
+ }
+
+ /**
+ * Creates a datagram with all the accepted options data in the proper
format.
+ * <p>
+ * This is a method only available within the package for implementing
efficient datagram transport by eliminating buffering. It takes a datagram as an
+ * argument, and a byte buffer in which to store the raw datagram data.
Inside the method, the data is set as the datagram's data and the datagram
returned.
+ * </p>
+ *
+ * @param datagram The datagram to create.
+ * @param data The buffer to store the packet and to use in the
datagram.
+ * @return The datagram argument.
+ */
+ @Override
+ DatagramPacket newDatagram(DatagramPacket datagram, byte[] data) {
+ int offset = 0;
+ data[offset++] = 0;
+ data[offset++] = (byte) type;
+
+ offset = writeOptionsData(data, offset);
+
+ datagram.setAddress(address);
+ datagram.setPort(port);
+ datagram.setData(data);
+ datagram.setLength(offset);
+
+ return datagram;
+ }
+
+ private int writeOptionsData(byte[] data, int offset) {
+ // all the options would get written to the data byte array in
following format
Review Comment:
Put this inline comment in a Javadoc comment using `<pre>` tags, _keep it
simple_, you don't need "~~"s...
##########
src/main/java/org/apache/commons/net/tftp/TFTPOptionsAckPacket.java:
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.net.tftp;
+
+import java.net.DatagramPacket;
+import java.net.InetAddress;
+import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * A final class derived from TFTPPacket defining the TFTP OACK (Option
Acknowledgment) packet type.
+ * <p>
+ * Details regarding this packet type can be found in RFC 2347.
+ * </p>
+ *
+ * @since 3.12.0
+ */
+public final class TFTPOptionsAckPacket extends TFTPPacket {
+
+ private final Map<String, String> options;
+
+ /**
+ * Constructs an OACK packet informing which options are accepted.
+ *
+ * @param address The host to which the packet is going to be sent.
+ * @param port The port to which the packet is going to be sent.
+ * @param options The options accepted.
+ */
+ public TFTPOptionsAckPacket(InetAddress address, int port, Map<String,
String> options) {
+ super(OACK, address, port);
+ this.options = new HashMap<>(options);
+ }
+
+ /**
+ * Gets the options extensions being acknowledged.
+ *
+ * @return The options being acknowledged.
+ */
+ public Map<String, String> getOptions() {
+ return options;
+ }
+
+ /**
+ * Creates a UDP datagram containing all the accepted options data in the
proper format.
+ * <p>
+ * This is a method exposed to the programmer in case he
+ * wants to implement his own TFTP client instead of using the {@link
org.apache.commons.net.tftp.TFTPClient} class. Under normal circumstances, you
should
+ * not have a need to call this method.
+ * </p>
+ *
+ * @return A UDP datagram containing the TFTP OACK packet.
+ */
+ @Override
+ public DatagramPacket newDatagram() {
+ int optionsLength = 0;
+ for (Map.Entry<String, String> entry : options.entrySet()) {
+ optionsLength += entry.getKey().length() + 1 +
entry.getValue().length() + 1;
+ }
+
+ byte[] data = new byte[2 + optionsLength];
+ data[0] = 0;
+ data[1] = (byte) type;
+ writeOptionsData(data, 2);
+
+ return new DatagramPacket(data, data.length, getAddress(), getPort());
+ }
+
+ /**
+ * Creates a datagram with all the accepted options data in the proper
format.
+ * <p>
+ * This is a method only available within the package for implementing
efficient datagram transport by eliminating buffering. It takes a datagram as an
+ * argument, and a byte buffer in which to store the raw datagram data.
Inside the method, the data is set as the datagram's data and the datagram
returned.
+ * </p>
+ *
+ * @param datagram The datagram to create.
+ * @param data The buffer to store the packet and to use in the
datagram.
+ * @return The datagram argument.
+ */
+ @Override
+ DatagramPacket newDatagram(DatagramPacket datagram, byte[] data) {
+ int offset = 0;
+ data[offset++] = 0;
+ data[offset++] = (byte) type;
+
Review Comment:
Remove extra vertical whitespace.
##########
src/main/java/org/apache/commons/net/tftp/TFTP.java:
##########
@@ -258,4 +266,32 @@ public final void send(final TFTPPacket packet) throws
IOException {
protected void trace(final String direction, final TFTPPacket packet) {
}
+ /**
+ * Sets the size of the buffers used to receive and send packets.
+ * This method can be used to increase the size of the buffer to support
`blksize`.
+ * For which valid values range between "8" and "65464" octets (RFC-2348).
+ * This only refers to the number of data octets, it does not include the
four octets of TFTP header
Review Comment:
A sentence should end in a period.
##########
src/main/java/org/apache/commons/net/tftp/TFTPRequestPacket.java:
##########
@@ -167,13 +210,22 @@ public final DatagramPacket newDatagram() {
fileLength = fileName.length();
modeLength = modeBytes[mode].length;
- data = new byte[fileLength + modeLength + 4];
+ int optionsLength = 0;
+ for (Map.Entry<String, String> entry : options.entrySet()) {
+ optionsLength += entry.getKey().length() + 1 +
entry.getValue().length() + 1;
+ }
+
Review Comment:
Remove extra vertical whitespace.
##########
src/main/java/org/apache/commons/net/tftp/TFTP.java:
##########
@@ -258,4 +266,32 @@ public final void send(final TFTPPacket packet) throws
IOException {
protected void trace(final String direction, final TFTPPacket packet) {
}
+ /**
+ * Sets the size of the buffers used to receive and send packets.
+ * This method can be used to increase the size of the buffer to support
`blksize`.
+ * For which valid values range between "8" and "65464" octets (RFC-2348).
+ * This only refers to the number of data octets, it does not include the
four octets of TFTP header
+ *
+ * @param packetSize The size of the data octets not including 4 octets
for the header.
+ * @since 3.12.0
+ */
+ public final void resetBuffersToSize(int packetSize) {
+ // the packet size should be between 8 - 65464 (inclusively) then we
add 4 for the header
+ this.packetSize = (Math.min(Math.max(packetSize, 8), 65464) + 4);
Review Comment:
Remove useless parentheses.
##########
src/main/java/org/apache/commons/net/tftp/TFTPOptionsAckPacket.java:
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.net.tftp;
+
+import java.net.DatagramPacket;
+import java.net.InetAddress;
+import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * A final class derived from TFTPPacket defining the TFTP OACK (Option
Acknowledgment) packet type.
+ * <p>
+ * Details regarding this packet type can be found in RFC 2347.
+ * </p>
+ *
+ * @since 3.12.0
+ */
+public final class TFTPOptionsAckPacket extends TFTPPacket {
+
+ private final Map<String, String> options;
+
+ /**
+ * Constructs an OACK packet informing which options are accepted.
+ *
+ * @param address The host to which the packet is going to be sent.
+ * @param port The port to which the packet is going to be sent.
+ * @param options The options accepted.
+ */
+ public TFTPOptionsAckPacket(InetAddress address, int port, Map<String,
String> options) {
+ super(OACK, address, port);
+ this.options = new HashMap<>(options);
+ }
+
+ /**
+ * Gets the options extensions being acknowledged.
+ *
+ * @return The options being acknowledged.
+ */
+ public Map<String, String> getOptions() {
+ return options;
+ }
+
+ /**
+ * Creates a UDP datagram containing all the accepted options data in the
proper format.
+ * <p>
+ * This is a method exposed to the programmer in case he
+ * wants to implement his own TFTP client instead of using the {@link
org.apache.commons.net.tftp.TFTPClient} class. Under normal circumstances, you
should
+ * not have a need to call this method.
+ * </p>
+ *
+ * @return A UDP datagram containing the TFTP OACK packet.
+ */
+ @Override
+ public DatagramPacket newDatagram() {
+ int optionsLength = 0;
+ for (Map.Entry<String, String> entry : options.entrySet()) {
+ optionsLength += entry.getKey().length() + 1 +
entry.getValue().length() + 1;
+ }
+
+ byte[] data = new byte[2 + optionsLength];
+ data[0] = 0;
+ data[1] = (byte) type;
+ writeOptionsData(data, 2);
+
+ return new DatagramPacket(data, data.length, getAddress(), getPort());
+ }
+
+ /**
+ * Creates a datagram with all the accepted options data in the proper
format.
+ * <p>
+ * This is a method only available within the package for implementing
efficient datagram transport by eliminating buffering. It takes a datagram as an
+ * argument, and a byte buffer in which to store the raw datagram data.
Inside the method, the data is set as the datagram's data and the datagram
returned.
+ * </p>
+ *
+ * @param datagram The datagram to create.
+ * @param data The buffer to store the packet and to use in the
datagram.
+ * @return The datagram argument.
+ */
+ @Override
+ DatagramPacket newDatagram(DatagramPacket datagram, byte[] data) {
+ int offset = 0;
+ data[offset++] = 0;
+ data[offset++] = (byte) type;
+
+ offset = writeOptionsData(data, offset);
+
+ datagram.setAddress(address);
+ datagram.setPort(port);
+ datagram.setData(data);
+ datagram.setLength(offset);
+
+ return datagram;
+ }
+
+ private int writeOptionsData(byte[] data, int offset) {
+ // all the options would get written to the data byte array in
following format
+ // +---~~---+---+---~~---+---+---~~---+---+---~~---+---+
+ // | opt1 | 0 | value1 | 0 | optN | 0 | valueN | 0 |
+ // +---~~---+---+---~~---+---+---~~---+---+---~~---+---+
+ for (Map.Entry<String, String> entry : options.entrySet()) {
+ byte[] key = entry.getKey().getBytes(StandardCharsets.US_ASCII);
Review Comment:
Use final where you can.
##########
src/main/java/org/apache/commons/net/tftp/TFTPOptionsAckPacket.java:
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.net.tftp;
+
+import java.net.DatagramPacket;
+import java.net.InetAddress;
+import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * A final class derived from TFTPPacket defining the TFTP OACK (Option
Acknowledgment) packet type.
+ * <p>
+ * Details regarding this packet type can be found in RFC 2347.
+ * </p>
+ *
+ * @since 3.12.0
+ */
+public final class TFTPOptionsAckPacket extends TFTPPacket {
+
+ private final Map<String, String> options;
+
+ /**
+ * Constructs an OACK packet informing which options are accepted.
+ *
+ * @param address The host to which the packet is going to be sent.
+ * @param port The port to which the packet is going to be sent.
+ * @param options The options accepted.
+ */
+ public TFTPOptionsAckPacket(InetAddress address, int port, Map<String,
String> options) {
+ super(OACK, address, port);
+ this.options = new HashMap<>(options);
+ }
+
+ /**
+ * Gets the options extensions being acknowledged.
+ *
+ * @return The options being acknowledged.
+ */
+ public Map<String, String> getOptions() {
+ return options;
+ }
+
+ /**
+ * Creates a UDP datagram containing all the accepted options data in the
proper format.
+ * <p>
+ * This is a method exposed to the programmer in case he
+ * wants to implement his own TFTP client instead of using the {@link
org.apache.commons.net.tftp.TFTPClient} class. Under normal circumstances, you
should
+ * not have a need to call this method.
+ * </p>
+ *
+ * @return A UDP datagram containing the TFTP OACK packet.
+ */
+ @Override
+ public DatagramPacket newDatagram() {
+ int optionsLength = 0;
+ for (Map.Entry<String, String> entry : options.entrySet()) {
+ optionsLength += entry.getKey().length() + 1 +
entry.getValue().length() + 1;
+ }
+
Review Comment:
Remove extra vertical whitespace.
##########
src/main/java/org/apache/commons/net/tftp/TFTP.java:
##########
@@ -112,6 +112,12 @@ public static final String getModeName(final int mode) {
/** A datagram used to minimize memory allocation in bufferedSend() */
private DatagramPacket sendDatagram;
+ /** Internal packet size, which is the data octet size plus 4 (for TFTP
header octets) */
+ private int packetSize = TFTPPacket.SEGMENT_SIZE + 4;
+
+ /** Internal state to track if the send/receive buffers are initialized */
+ private boolean buffersInitialized = false;
Review Comment:
Don't initialize an instance variable to its default value.
##########
src/main/java/org/apache/commons/net/tftp/TFTP.java:
##########
@@ -258,4 +266,32 @@ public final void send(final TFTPPacket packet) throws
IOException {
protected void trace(final String direction, final TFTPPacket packet) {
}
+ /**
+ * Sets the size of the buffers used to receive and send packets.
+ * This method can be used to increase the size of the buffer to support
`blksize`.
+ * For which valid values range between "8" and "65464" octets (RFC-2348).
+ * This only refers to the number of data octets, it does not include the
four octets of TFTP header
+ *
+ * @param packetSize The size of the data octets not including 4 octets
for the header.
+ * @since 3.12.0
+ */
+ public final void resetBuffersToSize(int packetSize) {
+ // the packet size should be between 8 - 65464 (inclusively) then we
add 4 for the header
+ this.packetSize = (Math.min(Math.max(packetSize, 8), 65464) + 4);
+ // if the buffers are already initialized reinitialize
+ if (buffersInitialized) {
+ endBufferedOps();
+ beginBufferedOps();
+ }
+ }
+
+ /**
+ * Gets the buffer size of the buffered used by {@link #bufferedSend
bufferedSend() } and {@link #bufferedReceive bufferedReceive() }.
Review Comment:
Simplify clutter: `{@link #bufferedSend bufferedSend() }` -> `{@link
#bufferedSend(TFTPPacket)}` and so on.
##########
src/main/java/org/apache/commons/net/tftp/TFTPOptionsAckPacket.java:
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.net.tftp;
+
+import java.net.DatagramPacket;
+import java.net.InetAddress;
+import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * A final class derived from TFTPPacket defining the TFTP OACK (Option
Acknowledgment) packet type.
+ * <p>
+ * Details regarding this packet type can be found in RFC 2347.
+ * </p>
+ *
+ * @since 3.12.0
+ */
+public final class TFTPOptionsAckPacket extends TFTPPacket {
+
+ private final Map<String, String> options;
+
+ /**
+ * Constructs an OACK packet informing which options are accepted.
+ *
+ * @param address The host to which the packet is going to be sent.
+ * @param port The port to which the packet is going to be sent.
+ * @param options The options accepted.
+ */
+ public TFTPOptionsAckPacket(InetAddress address, int port, Map<String,
String> options) {
+ super(OACK, address, port);
+ this.options = new HashMap<>(options);
+ }
+
+ /**
+ * Gets the options extensions being acknowledged.
+ *
+ * @return The options being acknowledged.
+ */
+ public Map<String, String> getOptions() {
+ return options;
+ }
+
+ /**
+ * Creates a UDP datagram containing all the accepted options data in the
proper format.
+ * <p>
+ * This is a method exposed to the programmer in case he
+ * wants to implement his own TFTP client instead of using the {@link
org.apache.commons.net.tftp.TFTPClient} class. Under normal circumstances, you
should
+ * not have a need to call this method.
+ * </p>
+ *
+ * @return A UDP datagram containing the TFTP OACK packet.
+ */
+ @Override
+ public DatagramPacket newDatagram() {
+ int optionsLength = 0;
+ for (Map.Entry<String, String> entry : options.entrySet()) {
+ optionsLength += entry.getKey().length() + 1 +
entry.getValue().length() + 1;
+ }
+
+ byte[] data = new byte[2 + optionsLength];
+ data[0] = 0;
+ data[1] = (byte) type;
+ writeOptionsData(data, 2);
+
+ return new DatagramPacket(data, data.length, getAddress(), getPort());
+ }
+
+ /**
+ * Creates a datagram with all the accepted options data in the proper
format.
+ * <p>
+ * This is a method only available within the package for implementing
efficient datagram transport by eliminating buffering. It takes a datagram as an
+ * argument, and a byte buffer in which to store the raw datagram data.
Inside the method, the data is set as the datagram's data and the datagram
returned.
+ * </p>
+ *
+ * @param datagram The datagram to create.
+ * @param data The buffer to store the packet and to use in the
datagram.
+ * @return The datagram argument.
+ */
+ @Override
+ DatagramPacket newDatagram(DatagramPacket datagram, byte[] data) {
+ int offset = 0;
+ data[offset++] = 0;
+ data[offset++] = (byte) type;
+
+ offset = writeOptionsData(data, offset);
+
+ datagram.setAddress(address);
+ datagram.setPort(port);
+ datagram.setData(data);
+ datagram.setLength(offset);
+
+ return datagram;
+ }
+
+ private int writeOptionsData(byte[] data, int offset) {
+ // all the options would get written to the data byte array in
following format
+ // +---~~---+---+---~~---+---+---~~---+---+---~~---+---+
+ // | opt1 | 0 | value1 | 0 | optN | 0 | valueN | 0 |
+ // +---~~---+---+---~~---+---+---~~---+---+---~~---+---+
+ for (Map.Entry<String, String> entry : options.entrySet()) {
+ byte[] key = entry.getKey().getBytes(StandardCharsets.US_ASCII);
+ System.arraycopy(key, 0, data, offset, key.length);
+ offset += key.length;
+ data[offset++] = 0;
+ byte[] value =
entry.getValue().getBytes(StandardCharsets.US_ASCII);
+ System.arraycopy(value, 0, data, offset, value.length);
+ offset += value.length;
+ data[offset++] = 0;
+ }
+
Review Comment:
Remove extra vertical whitespace.
##########
src/main/java/org/apache/commons/net/tftp/TFTPOptionsAckPacket.java:
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.net.tftp;
+
+import java.net.DatagramPacket;
+import java.net.InetAddress;
+import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
Review Comment:
This class is not tested. Remove it or create a unit test.
##########
src/main/java/org/apache/commons/net/tftp/TFTPRequestPacket.java:
##########
@@ -113,24 +119,50 @@ public abstract class TFTPRequestPacket extends
TFTPPacket {
}
final String modeString =
buffer.toString().toLowerCase(java.util.Locale.ENGLISH);
- length = modeStrings.length;
+ final int modeStringsLength = modeStrings.length;
int mode = 0;
- for (index = 0; index < length; index++) {
- if (modeString.equals(modeStrings[index])) {
- mode = index;
+ int modeIndex;
+ for (modeIndex = 0; modeIndex < modeStringsLength; modeIndex++) {
+ if (modeString.equals(modeStrings[modeIndex])) {
+ mode = modeIndex;
break;
}
}
this.mode = mode;
- if (index >= length) {
+ if (modeIndex >= modeStringsLength) {
throw new TFTPPacketException("Unrecognized TFTP transfer mode: "
+ modeString);
// May just want to default to binary mode instead of throwing
// exception.
// _mode = TFTP.OCTET_MODE;
}
+
+ ++index;
+ while (index < length) {
+ int start = index;
+ for (; data[index] != 0; ++index) {
+ if (index >= length) {
+ throw new TFTPPacketException("Invalid option format");
+ }
+ }
+
Review Comment:
Remove extra vertical whitespace.
##########
src/main/java/org/apache/commons/net/tftp/TFTP.java:
##########
@@ -258,4 +266,32 @@ public final void send(final TFTPPacket packet) throws
IOException {
protected void trace(final String direction, final TFTPPacket packet) {
}
+ /**
+ * Sets the size of the buffers used to receive and send packets.
+ * This method can be used to increase the size of the buffer to support
`blksize`.
+ * For which valid values range between "8" and "65464" octets (RFC-2348).
+ * This only refers to the number of data octets, it does not include the
four octets of TFTP header
+ *
+ * @param packetSize The size of the data octets not including 4 octets
for the header.
+ * @since 3.12.0
+ */
+ public final void resetBuffersToSize(int packetSize) {
Review Comment:
- This method is not tested or called, remove it or add a unit test for it.
- Use final where you can.
##########
src/main/java/org/apache/commons/net/tftp/TFTP.java:
##########
@@ -258,4 +266,32 @@ public final void send(final TFTPPacket packet) throws
IOException {
protected void trace(final String direction, final TFTPPacket packet) {
}
+ /**
+ * Sets the size of the buffers used to receive and send packets.
+ * This method can be used to increase the size of the buffer to support
`blksize`.
+ * For which valid values range between "8" and "65464" octets (RFC-2348).
+ * This only refers to the number of data octets, it does not include the
four octets of TFTP header
+ *
+ * @param packetSize The size of the data octets not including 4 octets
for the header.
+ * @since 3.12.0
+ */
+ public final void resetBuffersToSize(int packetSize) {
+ // the packet size should be between 8 - 65464 (inclusively) then we
add 4 for the header
+ this.packetSize = (Math.min(Math.max(packetSize, 8), 65464) + 4);
+ // if the buffers are already initialized reinitialize
+ if (buffersInitialized) {
+ endBufferedOps();
+ beginBufferedOps();
+ }
+ }
+
+ /**
+ * Gets the buffer size of the buffered used by {@link #bufferedSend
bufferedSend() } and {@link #bufferedReceive bufferedReceive() }.
+ *
+ * @return current buffer size
+ * @since 3.12.0
+ */
+ public int getBufferSize() {
Review Comment:
- This method is not tested or called, remove it or add a unit test for it.
- Rename to match the instance variable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
