-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Code quality fix - Security - Array is stored directly #1039
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,6 +38,7 @@ | |
| import org.asynchttpclient.netty.channel.pool.ChannelPool; | ||
| import org.asynchttpclient.proxy.ProxyServer; | ||
| import org.asynchttpclient.proxy.ProxyServerSelector; | ||
| import org.asynchttpclient.util.ArrayUtils; | ||
| import org.asynchttpclient.util.ProxyUtils; | ||
|
|
||
| /** | ||
|
|
@@ -806,12 +807,12 @@ public Builder setHandshakeTimeout(int handshakeTimeout) { | |
| } | ||
|
|
||
| public Builder setEnabledProtocols(String[] enabledProtocols) { | ||
| this.enabledProtocols = enabledProtocols; | ||
| this.enabledProtocols = ArrayUtils.copyOf(enabledProtocols); | ||
| return this; | ||
| } | ||
|
|
||
| public Builder setEnabledCipherSuites(String[] enabledCipherSuites) { | ||
| this.enabledCipherSuites = enabledCipherSuites; | ||
| this.enabledCipherSuites = ArrayUtils.copyOf(enabledCipherSuites); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not on hot path, why not. |
||
| return this; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,7 @@ | |
| import org.asynchttpclient.request.body.multipart.Part; | ||
| import org.asynchttpclient.resolver.NameResolver; | ||
| import org.asynchttpclient.uri.Uri; | ||
| import org.asynchttpclient.util.ArrayUtils; | ||
|
|
||
| public class DefaultRequest implements Request { | ||
|
|
||
|
|
@@ -95,7 +96,7 @@ public DefaultRequest(String method,// | |
| this.localAddress = localAddress; | ||
| this.headers = headers; | ||
| this.cookies = cookies; | ||
| this.byteData = byteData; | ||
| this.byteData = ArrayUtils.copyOf(byteData); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hot path, so no. |
||
| this.compositeByteData = compositeByteData; | ||
| this.stringData = stringData; | ||
| this.byteBufferData = byteBufferData; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,6 +40,7 @@ | |
| import org.asynchttpclient.resolver.JdkNameResolver; | ||
| import org.asynchttpclient.resolver.NameResolver; | ||
| import org.asynchttpclient.uri.Uri; | ||
| import org.asynchttpclient.util.ArrayUtils; | ||
| import org.asynchttpclient.util.UriEncoder; | ||
| import org.reactivestreams.Publisher; | ||
| import org.slf4j.Logger; | ||
|
|
@@ -274,7 +275,7 @@ private void resetBody() { | |
|
|
||
| public T setBody(byte[] data) { | ||
| resetBody(); | ||
| this.byteData = data; | ||
| this.byteData = ArrayUtils.copyOf(data); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hot path, no. |
||
| return asDerivedType(); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,8 @@ | |
| */ | ||
| package org.asynchttpclient.netty.request.body; | ||
|
|
||
| import org.asynchttpclient.util.ArrayUtils; | ||
|
|
||
| import io.netty.buffer.ByteBuf; | ||
| import io.netty.buffer.Unpooled; | ||
|
|
||
|
|
@@ -27,7 +29,7 @@ public NettyByteArrayBody(byte[] bytes) { | |
| } | ||
|
|
||
| public NettyByteArrayBody(byte[] bytes, String contentType) { | ||
| this.bytes = bytes; | ||
| this.bytes = ArrayUtils.copyOf(bytes); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hot path, no. |
||
| this.contentType = contentType; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,6 +40,7 @@ | |
| import javax.crypto.Cipher; | ||
| import javax.crypto.spec.SecretKeySpec; | ||
|
|
||
| import org.asynchttpclient.util.ArrayUtils; | ||
| import org.asynchttpclient.util.Base64; | ||
|
|
||
| /** | ||
|
|
@@ -247,12 +248,12 @@ public CipherGen(final String domain, final String user, final String password, | |
| this.target = target; | ||
| this.user = user; | ||
| this.password = password; | ||
| this.challenge = challenge; | ||
| this.targetInformation = targetInformation; | ||
| this.clientChallenge = clientChallenge; | ||
| this.clientChallenge2 = clientChallenge2; | ||
| this.secondaryKey = secondaryKey; | ||
| this.timestamp = timestamp; | ||
| this.challenge = ArrayUtils.copyOf(challenge); | ||
| this.targetInformation = ArrayUtils.copyOf(targetInformation); | ||
| this.clientChallenge = ArrayUtils.copyOf(clientChallenge); | ||
| this.clientChallenge2 = ArrayUtils.copyOf(clientChallenge2); | ||
| this.secondaryKey = ArrayUtils.copyOf(secondaryKey); | ||
| this.timestamp = ArrayUtils.copyOf(timestamp); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hot path, no. |
||
| } | ||
|
|
||
| public CipherGen(final String domain, final String user, final String password, final byte[] challenge, final String target, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ | |
| import java.io.IOException; | ||
|
|
||
| import org.asynchttpclient.request.body.Body; | ||
| import org.asynchttpclient.util.ArrayUtils; | ||
|
|
||
| /** | ||
| * A {@link BodyGenerator} backed by a byte array. | ||
|
|
@@ -26,7 +27,7 @@ public final class ByteArrayBodyGenerator implements BodyGenerator { | |
| private final byte[] bytes; | ||
|
|
||
| public ByteArrayBodyGenerator(byte[] bytes) { | ||
| this.bytes = bytes; | ||
| this.bytes = ArrayUtils.copyOf(bytes); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hot path, no. |
||
| } | ||
|
|
||
| protected final class ByteBody implements Body { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,8 @@ | |
|
|
||
| import java.nio.charset.Charset; | ||
|
|
||
| import org.asynchttpclient.util.ArrayUtils; | ||
|
|
||
| public class ByteArrayPart extends FileLikePart { | ||
|
|
||
| private final byte[] bytes; | ||
|
|
@@ -43,7 +45,7 @@ public ByteArrayPart(String name, byte[] bytes, String contentType, Charset char | |
| public ByteArrayPart(String name, byte[] bytes, String contentType, Charset charset, String fileName, String contentId, String transferEncoding) { | ||
| super(name, contentType, charset, contentId, transferEncoding); | ||
| assertNotNull(bytes, "bytes"); | ||
| this.bytes = bytes; | ||
| this.bytes = ArrayUtils.copyOf(bytes); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hot path, no. |
||
| setFileName(fileName); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,7 @@ | |
| import org.asynchttpclient.request.body.RandomAccessBody; | ||
| import org.asynchttpclient.request.body.multipart.part.MultipartPart; | ||
| import org.asynchttpclient.request.body.multipart.part.MultipartState; | ||
| import org.asynchttpclient.util.ArrayUtils; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
|
|
@@ -43,7 +44,7 @@ public class MultipartBody implements RandomAccessBody { | |
|
|
||
| public MultipartBody(List<MultipartPart<? extends Part>> parts, String contentType, byte[] boundary) { | ||
| assertNotNull(parts, "parts"); | ||
| this.boundary = boundary; | ||
| this.boundary = ArrayUtils.copyOf(boundary); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hot path, no. |
||
| this.contentType = contentType; | ||
| this.parts = parts; | ||
| this.contentLength = computeContentLength(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,6 +29,7 @@ | |
| import org.asynchttpclient.request.body.multipart.FileLikePart; | ||
| import org.asynchttpclient.request.body.multipart.part.PartVisitor.ByteBufVisitor; | ||
| import org.asynchttpclient.request.body.multipart.part.PartVisitor.CounterPartVisitor; | ||
| import org.asynchttpclient.util.ArrayUtils; | ||
|
|
||
| public abstract class MultipartPart<T extends FileLikePart> implements Closeable { | ||
|
|
||
|
|
@@ -101,7 +102,7 @@ public abstract class MultipartPart<T extends FileLikePart> implements Closeable | |
|
|
||
| public MultipartPart(T part, byte[] boundary) { | ||
| this.part = part; | ||
| this.boundary = boundary; | ||
| this.boundary = ArrayUtils.copyOf(boundary); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hot path, no. |
||
| preContentLength = computePreContentLength(); | ||
| postContentLength = computePostContentLength(); | ||
| state = MultipartState.PRE_CONTENT; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,165 @@ | ||
| package org.asynchttpclient.util; | ||
|
|
||
| import java.util.Arrays; | ||
|
|
||
| /** | ||
| * <p> | ||
| * Operations on arrays, primitive arrays (like {@code int[]}) and primitive | ||
| * wrapper arrays (like {@code Integer[]}). | ||
| * </p> | ||
| * <p/> | ||
| * <p> | ||
| * This class tries to handle {@code null} input gracefully. An exception will | ||
| * not be thrown for a {@code null} array input. | ||
| * </p> | ||
| */ | ||
| public final class ArrayUtils { | ||
|
|
||
| private ArrayUtils() { | ||
| } | ||
|
|
||
| /** | ||
| * <p> | ||
| * Return a copy of original array and handling {@code null}. | ||
| * </p> | ||
| * | ||
| * @param array | ||
| * the array to be copy, may be {@code null} | ||
| * @return a copy of the original array, {@code null} if {@code null} input | ||
| */ | ||
| public static boolean[] copyOf(final boolean[] array) { | ||
| if (array == null) { | ||
| return null; | ||
| } | ||
| return Arrays.copyOf(array, array.length); | ||
| } | ||
|
|
||
| /** | ||
| * <p> | ||
| * Return a copy of original array and handling {@code null}. | ||
| * </p> | ||
| * | ||
| * @param array | ||
| * the array to be copy, may be {@code null} | ||
| * @return a copy of the original array, {@code null} if {@code null} input | ||
| */ | ||
| public static byte[] copyOf(final byte[] array) { | ||
| if (array == null) { | ||
| return null; | ||
| } | ||
| return Arrays.copyOf(array, array.length); | ||
| } | ||
|
|
||
| /** | ||
| * <p> | ||
| * Return a copy of original array and handling {@code null}. | ||
| * </p> | ||
| * | ||
| * @param array | ||
| * the array to be copy, may be {@code null} | ||
| * @return a copy of the original array, {@code null} if {@code null} input | ||
| */ | ||
| public static short[] copyOf(final short[] array) { | ||
| if (array == null) { | ||
| return null; | ||
| } | ||
| return Arrays.copyOf(array, array.length); | ||
| } | ||
|
|
||
| /** | ||
| * <p> | ||
| * Return a copy of original array and handling {@code null}. | ||
| * </p> | ||
| * | ||
| * @param array | ||
| * the array to be copy, may be {@code null} | ||
| * @return a copy of the original array, {@code null} if {@code null} input | ||
| */ | ||
| public static int[] copyOf(final int[] array) { | ||
| if (array == null) { | ||
| return null; | ||
| } | ||
| return Arrays.copyOf(array, array.length); | ||
| } | ||
|
|
||
| /** | ||
| * <p> | ||
| * Return a copy of original array and handling {@code null}. | ||
| * </p> | ||
| * | ||
| * @param array | ||
| * the array to be copy, may be {@code null} | ||
| * @return a copy of the original array, {@code null} if {@code null} input | ||
| */ | ||
| public static long[] copyOf(final long[] array) { | ||
| if (array == null) { | ||
| return null; | ||
| } | ||
| return Arrays.copyOf(array, array.length); | ||
| } | ||
|
|
||
| /** | ||
| * <p> | ||
| * Return a copy of original array and handling {@code null}. | ||
| * </p> | ||
| * | ||
| * @param array | ||
| * the array to be copy, may be {@code null} | ||
| * @return a copy of the original array, {@code null} if {@code null} input | ||
| */ | ||
| public static float[] copyOf(final float[] array) { | ||
| if (array == null) { | ||
| return null; | ||
| } | ||
| return Arrays.copyOf(array, array.length); | ||
| } | ||
|
|
||
| /** | ||
| * <p> | ||
| * Return a copy of original array and handling {@code null}. | ||
| * </p> | ||
| * | ||
| * @param array | ||
| * the array to be copy, may be {@code null} | ||
| * @return a copy of the original array, {@code null} if {@code null} input | ||
| */ | ||
| public static double[] copyOf(final double[] array) { | ||
| if (array == null) { | ||
| return null; | ||
| } | ||
| return Arrays.copyOf(array, array.length); | ||
| } | ||
|
|
||
| /** | ||
| * <p> | ||
| * Return a copy of original array and handling {@code null}. | ||
| * </p> | ||
| * | ||
| * @param array | ||
| * the array to be copy, may be {@code null} | ||
| * @return a copy of the original array, {@code null} if {@code null} input | ||
| */ | ||
| public static char[] copyOf(final char[] array) { | ||
| if (array == null) { | ||
| return null; | ||
| } | ||
| return Arrays.copyOf(array, array.length); | ||
| } | ||
|
|
||
| /** | ||
| * <p> | ||
| * Return a copy of original array and handling {@code null}. | ||
| * </p> | ||
| * | ||
| * @param array | ||
| * the array to be copy, may be {@code null} | ||
| * @return a copy of the original array, {@code null} if {@code null} input | ||
| */ | ||
| public static <T> T[] copyOf(final T[] array) { | ||
| if (array == null) { | ||
| return null; | ||
| } | ||
| return Arrays.copyOf(array, array.length); | ||
| } | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| /* | ||
| * Copyright (c) 2013 Sonatype, Inc. All rights reserved. | ||
| * | ||
| * This program is licensed to you under the Apache License Version 2.0, | ||
| * and you may not use this file except in compliance with the Apache License Version 2.0. | ||
| * You may obtain a copy of the Apache License Version 2.0 at http://www.apache.org/licenses/LICENSE-2.0. | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the Apache License Version 2.0 is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the Apache License Version 2.0 for the specific language governing permissions and limitations there under. | ||
| */ | ||
| package org.asynchttpclient; | ||
|
|
||
| import org.testng.Assert; | ||
| import org.testng.annotations.Test; | ||
|
|
||
| public class DefaultRequestTest { | ||
|
|
||
| @Test | ||
| public void testByteData() { | ||
|
|
||
| byte[] actualByteData = "testData".getBytes(); | ||
|
|
||
| DefaultRequest request = new DefaultRequest(null, null, null, null, null, null, actualByteData, null, null, null, null, null, null, null, | ||
| null, 0, null, null, null, false, 0, 0, null, null, null); | ||
|
|
||
| /* | ||
| * Test references are different | ||
| */ | ||
| Assert.assertTrue(actualByteData != request.getByteData()); | ||
|
|
||
| /* | ||
| * Test array contents are same | ||
| */ | ||
| Assert.assertEquals(actualByteData, request.getByteData()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not on hot path, why not.