Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package com.azure.core.http.jdk.httpclient.implementation;

import com.azure.core.http.HttpHeaders;
import com.azure.core.implementation.util.HttpHeadersAccessHelper;
import com.azure.core.util.CoreUtils;

import java.nio.ByteBuffer;
Expand All @@ -25,13 +26,14 @@ public final class JdkHttpUtils {
* @param headers the JDK Http headers
* @return the azure-core Http headers
*/
@SuppressWarnings("deprecation")
public static HttpHeaders fromJdkHttpHeaders(java.net.http.HttpHeaders headers) {
final HttpHeaders httpHeaders = new HttpHeaders((int) (headers.map().size() / 0.75F));

for (Map.Entry<String, List<String>> kvp : headers.map().entrySet()) {
if (!CoreUtils.isNullOrEmpty(kvp.getValue())) {
httpHeaders.set(kvp.getKey(), kvp.getValue());
// JDK HttpClient parses headers to lower case, use the access helper to bypass lowercasing the header
// name (or in this case, just checking that the header name is lowercased).
HttpHeadersAccessHelper.setInternal(httpHeaders, kvp.getKey(), kvp.getKey(), kvp.getValue());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,13 @@
import com.azure.core.http.HttpHeaders;
import com.azure.core.http.HttpRequest;
import com.azure.core.http.HttpResponse;
import com.azure.core.implementation.util.HttpHeadersAccessHelper;
import io.netty.util.AsciiString;
import reactor.netty.http.client.HttpClientResponse;

import java.util.Iterator;
import java.util.Map;
import java.util.Objects;

/**
* Base response class for Reactor Netty with implementations for response metadata.
Expand Down Expand Up @@ -40,8 +43,19 @@ public abstract class NettyAsyncHttpResponseBase extends HttpResponse {
while (nettyHeadersIterator.hasNext()) {
Map.Entry<CharSequence, CharSequence> next = nettyHeadersIterator.next();
// Value may be null and that needs to be guarded but key should never be null.
CharSequence value = next.getValue();
this.headers.add(next.getKey().toString(), (value == null) ? null : value.toString());
String value = Objects.toString(next.getValue(), null);
CharSequence key = next.getKey();

// Check for the header name being a Netty AsciiString as it has optimizations around lowercasing.
if (key instanceof AsciiString) {
// Hook into optimizations exposed through shared implementation to speed up the conversion.
AsciiString asciiString = (AsciiString) key;
HttpHeadersAccessHelper.addInternal(headers, asciiString.toLowerCase().toString(),
asciiString.toString(), value);
} else {
// If it isn't an AsciiString, then fallback to the shared, albeit, slower path.
this.headers.add(key.toString(), value);
}
}
} else {
this.headers = new NettyToAzureCoreHttpHeadersWrapper(nettyHeaders);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,9 @@ public final class Utility {
* @return A newly allocated {@link ByteBuffer} containing the copied bytes.
*/
public static ByteBuffer deepCopyBuffer(ByteBuf byteBuf) {
ByteBuffer buffer = ByteBuffer.allocate(byteBuf.readableBytes());
byteBuf.readBytes(buffer);
buffer.rewind();
return buffer;
byte[] bytes = new byte[byteBuf.readableBytes()];
byteBuf.getBytes(byteBuf.readerIndex(), bytes);
return ByteBuffer.wrap(bytes);
}

/**
Expand Down
6 changes: 6 additions & 0 deletions sdk/core/azure-core/spotbugs-exclude.xml
Original file line number Diff line number Diff line change
Expand Up @@ -441,4 +441,10 @@
<Class name="com.azure.core.implementation.http.rest.SwaggerMethodParser" />
<Method name="serialize" />
</Match>

<Match>
<Bug pattern="EI_EXPOSE_STATIC_REP2" />
<Class name="com.azure.core.implementation.util.HttpHeadersAccessHelper" />
<Method name="setAccessor" />
</Match>
</FindBugsFilter>
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,22 @@ public class HttpHeaders implements Iterable<HttpHeader> {
private final Map<String, HttpHeader> headers;

static {
HttpHeadersAccessHelper.setAccessor(headers -> headers.headers);
HttpHeadersAccessHelper.setAccessor(new HttpHeadersAccessHelper.HttpHeadersAccessor() {
@Override
public Map<String, HttpHeader> getRawHeaderMap(HttpHeaders headers) {
return headers.headers;
}

@Override
public void addInternal(HttpHeaders headers, String formattedName, String name, String value) {
headers.addInternal(formattedName, name, value);
}

@Override
public void setInternal(HttpHeaders headers, String formattedName, String name, List<String> values) {
headers.setInternal(formattedName, name, values);
}
});
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,6 @@ public Mono<HttpResponse> send(HttpRequest request, Context contextData) {

@Override
public Object invoke(Object proxy, final Method method, Object[] args) {
RestProxyUtils.validateResumeOperationIsNotPresent(method);

// Note: request options need to be evaluated here, as it is a public class with package private methods.
// Evaluating here allows the package private methods to be invoked here for downstream use.
final SwaggerMethodParser methodParser = getMethodParser(method);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,6 @@ Mono<HttpResponse> send(HttpRequest request, Context contextData) {
@SuppressWarnings({ "try", "unused" })
public Object invoke(Object proxy, Method method, RequestOptions options, EnumSet<ErrorOptions> errorOptions,
Consumer<HttpRequest> requestCallback, SwaggerMethodParser methodParser, HttpRequest request, Context context) {
RestProxyUtils.validateResumeOperationIsNotPresent(method);

context = startTracingSpan(methodParser, context);

// If there is 'RequestOptions' apply its request callback operations before validating the body.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,6 @@ public RestProxyBase(HttpPipeline httpPipeline, SerializerAdapter serializer,
*/
public final Object invoke(Object proxy, Method method, RequestOptions options, EnumSet<ErrorOptions> errorOptions,
Consumer<HttpRequest> requestCallback, SwaggerMethodParser methodParser, boolean isAsync, Object[] args) {
RestProxyUtils.validateResumeOperationIsNotPresent(method);

try {
HttpRequest request = createHttpRequest(methodParser, serializer, isAsync, args);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,8 @@ public SwaggerMethodParser(Method swaggerMethod) {
this.responseEagerlyRead = isResponseEagerlyRead(unwrappedReturnType);
this.ignoreResponseBody = isResponseBodyIgnored(unwrappedReturnType);
this.spanName = interfaceParser.getServiceName() + "." + swaggerMethod.getName();

RestProxyUtils.validateResumeOperationIsNotPresent(swaggerMethod);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import com.azure.core.http.HttpHeader;
import com.azure.core.http.HttpHeaders;

import java.util.List;
import java.util.Locale;
import java.util.Map;

/**
Expand All @@ -24,6 +26,34 @@ public interface HttpHeadersAccessor {
* @return The raw header map.
*/
Map<String, HttpHeader> getRawHeaderMap(HttpHeaders headers);

/**
* Adds a header value to the backing map in {@link HttpHeaders}.
* <p>
* This bypasses using {@link HttpHeaders#add(String, String)} which uses {@link String#toLowerCase(Locale)},
* which may be slower than options available by implementing HTTP stacks (such as Netty which has an ASCII
* string class which has optimizations around lowercasing due to ASCII constraints).
*
* @param headers The {@link HttpHeaders} to add the header to.
* @param formattedName The lower-cased header name.
* @param name The original header name.
* @param value The header value.
*/
void addInternal(HttpHeaders headers, String formattedName, String name, String value);

/**
* Sets a header value to the backing map in {@link HttpHeaders}.
* <p>
* This bypasses using {@link HttpHeaders#set(String, List)} which uses {@link String#toLowerCase(Locale)},
* which may be slower than options available by implementing HTTP stacks (such as JDK HttpClient where all
* response header names are already lowercased).
*
* @param headers The {@link HttpHeaders} to set the header to.
* @param formattedName The lower-cased header name.
* @param name The original header name.
* @param values The header values.
*/
void setInternal(HttpHeaders headers, String formattedName, String name, List<String> values);
}

/**
Expand All @@ -36,6 +66,38 @@ public static Map<String, HttpHeader> getRawHeaderMap(HttpHeaders headers) {
return accessor.getRawHeaderMap(headers);
}

/**
* Adds a header value to the backing map in {@link HttpHeaders}.
* <p>
* This bypasses using {@link HttpHeaders#add(String, String)} which uses {@link String#toLowerCase(Locale)},
* which may be slower than options available by implementing HTTP stacks (such as Netty which has an ASCII
* string class which has optimizations around lowercasing due to ASCII constraints).
*
* @param headers The {@link HttpHeaders} to add the header to.
* @param formattedName The lower-cased header name.
* @param name The original header name.
* @param value The header value.
*/
public static void addInternal(HttpHeaders headers, String formattedName, String name, String value) {
accessor.addInternal(headers, formattedName, name, value);
}

/**
* Sets a header value to the backing map in {@link HttpHeaders}.
* <p>
* This bypasses using {@link HttpHeaders#set(String, List)} which uses {@link String#toLowerCase(Locale)},
* which may be slower than options available by implementing HTTP stacks (such as JDK HttpClient where all
* response header names are already lowercased).
*
* @param headers The {@link HttpHeaders} to set the header to.
* @param formattedName The lower-cased header name.
* @param name The original header name.
* @param values The header values.
*/
public static void setInternal(HttpHeaders headers, String formattedName, String name, List<String> values) {
accessor.setInternal(headers, formattedName, name, values);
}

/**
* Sets the {@link HttpHeadersAccessor}.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@
import com.azure.core.util.logging.ClientLogger;
import com.azure.core.util.logging.LogLevel;

import java.util.Map;
import java.util.TreeMap;

/**
* Supported serialization encoding formats.
*/
Expand All @@ -32,20 +29,6 @@ public enum SerializerEncoding {
TEXT;

private static final ClientLogger LOGGER = new ClientLogger(SerializerEncoding.class);
private static final Map<String, SerializerEncoding> SUPPORTED_MIME_TYPES;

static {
// Encodings and suffixes from: https://tools.ietf.org/html/rfc6838
SUPPORTED_MIME_TYPES = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
SUPPORTED_MIME_TYPES.put("text/xml", XML);
SUPPORTED_MIME_TYPES.put("application/xml", XML);
SUPPORTED_MIME_TYPES.put("application/json", JSON);
SUPPORTED_MIME_TYPES.put("text/css", TEXT);
SUPPORTED_MIME_TYPES.put("text/csv", TEXT);
SUPPORTED_MIME_TYPES.put("text/html", TEXT);
SUPPORTED_MIME_TYPES.put("text/javascript", TEXT);
SUPPORTED_MIME_TYPES.put("text/plain", TEXT);
}

/**
* Determines the serializer encoding to use based on the Content-Type header.
Expand All @@ -63,7 +46,7 @@ public static SerializerEncoding fromHeaders(HttpHeaders headers) {

int contentTypeEnd = mimeContentType.indexOf(';');
String contentType = (contentTypeEnd == -1) ? mimeContentType : mimeContentType.substring(0, contentTypeEnd);
final SerializerEncoding encoding = SUPPORTED_MIME_TYPES.get(contentType);
SerializerEncoding encoding = checkForKnownEncoding(contentType);
if (encoding != null) {
return encoding;
}
Expand Down Expand Up @@ -97,4 +80,43 @@ public static SerializerEncoding fromHeaders(HttpHeaders headers) {

return JSON;
}

/*
* There is a limited set of serialization encodings that are known ahead of time. Instead of using a TreeMap with
* a case-insensitive comparator, use an optimized search specifically for the known encodings.
*/
private static SerializerEncoding checkForKnownEncoding(String contentType) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have a sufficient test coverage for it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

int length = contentType.length();

// Check the length of the content type first as it is a quick check.
if (length != 8 && length != 9 && length != 10 && length != 15 && length != 16) {
return null;
}

if ("text/".regionMatches(true, 0, contentType, 0, 5)) {
if (length == 8) {
if ("xml".regionMatches(true, 0, contentType, 5, 3)) {
return XML;
} else if ("csv".regionMatches(true, 0, contentType, 5, 3)) {
return TEXT;
} else if ("css".regionMatches(true, 0, contentType, 5, 3)) {
return TEXT;
}
} else if (length == 9 && "html".regionMatches(true, 0, contentType, 5, 4)) {
return TEXT;
} else if (length == 10 && "plain".regionMatches(true, 0, contentType, 5, 5)) {
return TEXT;
} else if (length == 15 && "javascript".regionMatches(true, 0, contentType, 5, 10)) {
return TEXT;
}
} else if ("application/".regionMatches(true, 0, contentType, 0, 12)) {
if (length == 16 && "json".regionMatches(true, 0, contentType, 12, 4)) {
return JSON;
} else if (length == 15 && "xml".regionMatches(true, 0, contentType, 12, 3)) {
return XML;
}
}

return null;
}
}