Skip to content
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

HTTPCLIENT-2070: Auth cache to no longer rely on Java serialization for auth state caching #544

Merged
merged 1 commit into from
Jan 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* ====================================================================
* 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.
* ====================================================================
*
* This software consists of voluntary contributions made by many
* individuals on behalf of the Apache Software Foundation. For more
* information on the Apache Software Foundation, please see
* <http://www.apache.org/>.
*
*/
package org.apache.hc.client5.http.impl;

import org.apache.hc.core5.annotation.Internal;

/**
* @since 5.4
*/
@Internal
public interface StateHolder<T> {

T store();

void restore(T state);

}
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,6 @@
*/
package org.apache.hc.client5.http.impl.auth;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.io.Serializable;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
Expand All @@ -41,6 +35,7 @@
import org.apache.hc.client5.http.auth.AuthCache;
import org.apache.hc.client5.http.auth.AuthScheme;
import org.apache.hc.client5.http.impl.DefaultSchemePortResolver;
import org.apache.hc.client5.http.impl.StateHolder;
import org.apache.hc.core5.annotation.Contract;
import org.apache.hc.core5.annotation.ThreadingBehavior;
import org.apache.hc.core5.http.HttpHost;
Expand Down Expand Up @@ -123,7 +118,19 @@ public String toString() {
}
}

private final Map<Key, byte[]> map;
static class AuthData {

final Class<? extends AuthScheme> clazz;
final Object state;

public AuthData(final Class<? extends AuthScheme> clazz, final Object state) {
this.clazz = clazz;
this.state = state;
}

}

private final Map<Key, AuthData> map;
private final SchemePortResolver schemePortResolver;

/**
Expand All @@ -145,6 +152,10 @@ private Key key(final String scheme, final NamedEndpoint authority, final String
return new Key(scheme, authority.getHostName(), schemePortResolver.resolve(scheme, authority), pathPrefix);
}

private AuthData data(final AuthScheme authScheme) {
return new AuthData(authScheme.getClass(), ((StateHolder) authScheme).store());
}

@Override
public void put(final HttpHost host, final AuthScheme authScheme) {
put(host, null, authScheme);
Expand All @@ -166,42 +177,28 @@ public void put(final HttpHost host, final String pathPrefix, final AuthScheme a
if (authScheme == null) {
return;
}
if (authScheme instanceof Serializable) {
try {
final ByteArrayOutputStream buf = new ByteArrayOutputStream();
try (final ObjectOutputStream out = new ObjectOutputStream(buf)) {
out.writeObject(authScheme);
}
this.map.put(key(host.getSchemeName(), host, pathPrefix), buf.toByteArray());
} catch (final IOException ex) {
if (LOG.isWarnEnabled()) {
LOG.warn("Unexpected I/O error while serializing auth scheme", ex);
}
}
if (authScheme instanceof StateHolder) {
this.map.put(key(host.getSchemeName(), host, pathPrefix), data(authScheme));
} else {
if (LOG.isDebugEnabled()) {
LOG.debug("Auth scheme {} is not serializable", authScheme.getClass());
LOG.debug("Auth scheme {} cannot be cached", authScheme.getClass());
}
}
}

@Override
@SuppressWarnings("unchecked")
public AuthScheme get(final HttpHost host, final String pathPrefix) {
Args.notNull(host, "HTTP host");
final byte[] bytes = this.map.get(key(host.getSchemeName(), host, pathPrefix));
if (bytes != null) {
final AuthData authData = this.map.get(key(host.getSchemeName(), host, pathPrefix));
if (authData != null) {
try {
final ByteArrayInputStream buf = new ByteArrayInputStream(bytes);
try (final ObjectInputStream in = new ObjectInputStream(buf)) {
return (AuthScheme) in.readObject();
}
} catch (final IOException ex) {
if (LOG.isWarnEnabled()) {
LOG.warn("Unexpected I/O error while de-serializing auth scheme", ex);
}
} catch (final ClassNotFoundException ex) {
final AuthScheme authScheme = authData.clazz.newInstance();
((StateHolder<Object>) authScheme).restore(authData.state);
return authScheme;
} catch (final IllegalAccessException | InstantiationException ex) {
if (LOG.isWarnEnabled()) {
LOG.warn("Unexpected error while de-serializing auth scheme", ex);
LOG.warn("Unexpected error while reading auth scheme state", ex);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,9 @@
*/
package org.apache.hc.client5.http.impl.auth;

import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.io.Serializable;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.nio.charset.UnsupportedCharsetException;
import java.security.Principal;
import java.util.HashMap;
import java.util.List;
Expand All @@ -49,9 +45,11 @@
import org.apache.hc.client5.http.auth.MalformedChallengeException;
import org.apache.hc.client5.http.auth.StandardAuthScheme;
import org.apache.hc.client5.http.auth.UsernamePasswordCredentials;
import org.apache.hc.client5.http.impl.StateHolder;
import org.apache.hc.client5.http.protocol.HttpClientContext;
import org.apache.hc.client5.http.utils.Base64;
import org.apache.hc.client5.http.utils.ByteArrayBuilder;
import org.apache.hc.core5.annotation.Internal;
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.HttpRequest;
import org.apache.hc.core5.http.NameValuePair;
Expand All @@ -66,14 +64,13 @@
* @since 4.0
*/
@AuthStateCacheable
public class BasicScheme implements AuthScheme, Serializable {
public class BasicScheme implements AuthScheme, StateHolder<BasicScheme.State>, Serializable {

private static final long serialVersionUID = -1931571557597830536L;

private static final Logger LOG = LoggerFactory.getLogger(BasicScheme.class);

private final Map<String, String> paramMap;
private transient Charset defaultCharset;
private transient ByteArrayBuilder buffer;
private transient Base64 base64codec;
private boolean complete;
Expand All @@ -89,7 +86,6 @@ public class BasicScheme implements AuthScheme, Serializable {
@Deprecated
public BasicScheme(final Charset charset) {
this.paramMap = new HashMap<>();
this.defaultCharset = StandardCharsets.UTF_8; // Always use UTF-8
this.complete = false;
}

Expand All @@ -100,7 +96,6 @@ public BasicScheme(final Charset charset) {
*/
public BasicScheme() {
this.paramMap = new HashMap<>();
this.defaultCharset = StandardCharsets.UTF_8;
this.complete = false;
}

Expand All @@ -109,6 +104,7 @@ public void initPreemptive(final Credentials credentials) {
Args.check(credentials instanceof UsernamePasswordCredentials,
"Unsupported credential type: " + credentials.getClass());
this.credentials = (UsernamePasswordCredentials) credentials;
this.complete = true;
} else {
this.credentials = null;
}
Expand Down Expand Up @@ -221,7 +217,7 @@ public String generateAuthResponse(
} else {
this.buffer.reset();
}
final Charset charset = AuthSchemeSupport.parseCharset(paramMap.get("charset"), defaultCharset);
final Charset charset = AuthSchemeSupport.parseCharset(paramMap.get("charset"), StandardCharsets.UTF_8);
this.buffer.charset(charset);
this.buffer.append(this.credentials.getUserName()).append(":").append(this.credentials.getUserPassword());
if (this.base64codec == null) {
Expand All @@ -232,27 +228,47 @@ public String generateAuthResponse(
return StandardAuthScheme.BASIC + " " + new String(encodedCreds, 0, encodedCreds.length, StandardCharsets.US_ASCII);
}

private void writeObject(final ObjectOutputStream out) throws IOException {
out.defaultWriteObject();
out.writeUTF(this.defaultCharset.name());
}

@SuppressWarnings("unchecked")
private void readObject(final ObjectInputStream in) throws IOException, ClassNotFoundException {
in.defaultReadObject();
try {
this.defaultCharset = Charset.forName(in.readUTF());
} catch (final UnsupportedCharsetException ex) {
this.defaultCharset = StandardCharsets.UTF_8;
@Override
public State store() {
if (complete) {
return new State(new HashMap<>(paramMap), credentials);
} else {
return null;
}
}

private void readObjectNoData() {
@Override
public void restore(final State state) {
if (state != null) {
paramMap.clear();
paramMap.putAll(state.params);
credentials = state.credentials;
complete = true;
}
}

@Override
public String toString() {
return getName() + this.paramMap;
}

@Internal
public static class State {

final Map<String, String> params;
Copy link
Member

Choose a reason for hiding this comment

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

I assume this variables are not private due to its internal scope.

Copy link
Member Author

Choose a reason for hiding this comment

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

@arturobernalg Correct.

final UsernamePasswordCredentials credentials;

State(final Map<String, String> params, final UsernamePasswordCredentials credentials) {
this.params = params;
this.credentials = credentials;
}

@Override
public String toString() {
return "State{" +
"params=" + params +
'}';
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@
import org.apache.hc.client5.http.auth.CredentialsProvider;
import org.apache.hc.client5.http.auth.MalformedChallengeException;
import org.apache.hc.client5.http.auth.StandardAuthScheme;
import org.apache.hc.client5.http.impl.StateHolder;
import org.apache.hc.client5.http.protocol.HttpClientContext;
import org.apache.hc.core5.annotation.Internal;
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.HttpRequest;
import org.apache.hc.core5.http.NameValuePair;
Expand All @@ -59,7 +61,7 @@
* @since 5.3
*/
@AuthStateCacheable
public class BearerScheme implements AuthScheme, Serializable {
public class BearerScheme implements AuthScheme, StateHolder<BearerScheme.State>, Serializable {

private static final Logger LOG = LoggerFactory.getLogger(BearerScheme.class);

Expand Down Expand Up @@ -161,9 +163,49 @@ public String generateAuthResponse(
return StandardAuthScheme.BEARER + " " + bearerToken.getToken();
}

@Override
public State store() {
if (complete) {
return new State(new HashMap<>(paramMap), bearerToken);
} else {
return null;
}
}

@Override
public void restore(final State state) {
if (state != null) {
paramMap.clear();
paramMap.putAll(state.params);
bearerToken = state.bearerToken;
complete = true;
}
}

@Override
public String toString() {
return getName() + this.paramMap;
}

@Internal
public static class State {

final Map<String, String> params;
Copy link
Member

Choose a reason for hiding this comment

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

I assume this variables are not private due to its internal scope.

final BearerToken bearerToken;

State(final Map<String, String> params, final BearerToken bearerToken) {
this.params = params;
this.bearerToken = bearerToken;
}

@Override
public String toString() {
return "State{" +
"params=" + params +
", bearerToken=" + bearerToken +
'}';
}

}

}
Loading
Loading