Skip to content
This repository has been archived by the owner on Apr 16, 2022. It is now read-only.

Commit

Permalink
Let the request decide if cookies should be ignored or not (#805)
Browse files Browse the repository at this point in the history 
We changed the cookie spec to IGNORE_COOKIES to work around an issue with multiple auth methods sent to Central servers. Doing so broke auth with Aggregate servers.

Since the cookie spec is set at an http client level in the executor we can't create it each time we need to send a request because that would prevent requests to the same host to reuse connections. Having two executors (one with cookies, one without) would leak too much of the internal workings.

Due to this, now we let the Request object tell the http port when it needs to use cookies or not. Instead of ignoring cookies, we clear the cookie store before sending the request, which is not the same concept but achieves the same effect.
  • Loading branch information
@ggalmazor
ggalmazor authored Sep 10, 2019
1 parent 31b9a31 commit 7bb6185
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 35 deletions.
34 changes: 22 additions & 12 deletions src/org/opendatakit/briefcase/reused/http/CommonsHttp.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

package org.opendatakit.briefcase.reused.http;

import static org.apache.http.client.config.CookieSpecs.IGNORE_COOKIES;
import static org.apache.http.client.config.CookieSpecs.STANDARD;
import static org.apache.http.client.config.RequestConfig.custom;
import static org.opendatakit.briefcase.reused.http.RequestMethod.POST;

Expand All @@ -33,45 +33,52 @@
import org.apache.http.entity.ContentType;
import org.apache.http.entity.mime.MultipartEntityBuilder;
import org.apache.http.entity.mime.content.InputStreamBody;
import org.apache.http.impl.client.BasicCookieStore;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClientBuilder;
import org.opendatakit.briefcase.reused.BriefcaseException;
import org.opendatakit.briefcase.reused.http.response.Response;

public class CommonsHttp implements Http {
private Executor executor;
private final int maxConnections;
private final BasicCookieStore cookieStore;

private CommonsHttp(Executor executor, int maxConnections) {
private CommonsHttp(Executor executor, int maxConnections, BasicCookieStore cookieStore) {
this.executor = executor;
this.maxConnections = maxConnections;
this.cookieStore = cookieStore;
}

public static Http of(int maxConnections, HttpHost httpProxy) {
if (!Http.isValidHttpConnections(maxConnections))
throw new BriefcaseException("Invalid maximum simultaneous HTTP connections " + maxConnections + ". Try a value between " + MIN_HTTP_CONNECTIONS + " and " + MAX_HTTP_CONNECTIONS);
return new CommonsHttp(Executor.newInstance(
getBaseBuilder(maxConnections).setProxy(httpProxy).build()
), maxConnections);
BasicCookieStore cookieStore = new BasicCookieStore();
return new CommonsHttp(Executor.newInstance(getBaseBuilder(maxConnections, cookieStore).setProxy(httpProxy).build()), maxConnections, cookieStore);
}

public static Http of(int maxConnections) {
if (!Http.isValidHttpConnections(maxConnections))
throw new BriefcaseException("Invalid maximum simultaneous HTTP connections " + maxConnections + ". Try a value between " + MIN_HTTP_CONNECTIONS + " and " + MAX_HTTP_CONNECTIONS);
return new CommonsHttp(Executor.newInstance(
getBaseBuilder(maxConnections).build()
), maxConnections);
BasicCookieStore cookieStore = new BasicCookieStore();
HttpClientBuilder baseBuilder = getBaseBuilder(maxConnections, cookieStore);

CloseableHttpClient build = baseBuilder.build();

return new CommonsHttp(Executor.newInstance(build), maxConnections, cookieStore);
}

private static HttpClientBuilder getBaseBuilder(int maxConnections) {
private static HttpClientBuilder getBaseBuilder(int maxConnections, BasicCookieStore cookieStore) {
return HttpClientBuilder
.create()
.setDefaultCookieStore(cookieStore)
.setMaxConnPerRoute(maxConnections)
.setMaxConnTotal(maxConnections)
.setDefaultRequestConfig(custom()
.setConnectionRequestTimeout(0)
.setSocketTimeout(0)
.setConnectTimeout(0)
.setCookieSpec(IGNORE_COOKIES)
.setCookieSpec(STANDARD)
.build());
}

Expand All @@ -89,15 +96,18 @@ public <T> Response<T> execute(Request<T> request) {

@Override
public void setProxy(HttpHost proxy) {
executor = Executor.newInstance(getBaseBuilder(maxConnections).setProxy(proxy).build());
executor = Executor.newInstance(getBaseBuilder(maxConnections, new BasicCookieStore()).setProxy(proxy).build());
}

@Override
public void unsetProxy() {
executor = Executor.newInstance(getBaseBuilder(maxConnections).build());
executor = Executor.newInstance(getBaseBuilder(maxConnections, new BasicCookieStore()).build());
}

private <T> Response<T> uncheckedExecute(Request<T> request, Executor executor) {
if (request.ignoreCookies())
cookieStore.clear();

// Get an Apache Commons HTTPClient request and set some reasonable timeouts
org.apache.http.client.fluent.Request commonsRequest = getCommonsRequest(request);

Expand Down
10 changes: 8 additions & 2 deletions src/org/opendatakit/briefcase/reused/http/Request.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,17 @@ public class Request<T> {
final Map<String, String> headers;
private final Optional<InputStream> body;
final List<MultipartMessage> multipartMessages;
private final boolean ignoreCookies;

Request(RequestMethod method, URL url, Optional<Credentials> credentials, Function<InputStream, T> responseMapper, Map<String, String> headers, Optional<InputStream> body, List<MultipartMessage> multipartMessages) {
Request(RequestMethod method, URL url, Optional<Credentials> credentials, Function<InputStream, T> responseMapper, Map<String, String> headers, Optional<InputStream> body, List<MultipartMessage> multipartMessages, boolean ignoreCookies) {
this.method = method;
this.url = url;
this.credentials = credentials;
this.responseMapper = responseMapper;
this.headers = headers;
this.body = body;
this.multipartMessages = multipartMessages;
this.ignoreCookies = ignoreCookies;
}

public T map(InputStream responseBody) {
Expand Down Expand Up @@ -91,7 +93,7 @@ URI asUri() {
* Returns a RequestBuilder that would produce this instance when built.
*/
public RequestBuilder<T> builder() {
return new RequestBuilder<>(method, url, responseMapper, credentials, headers, body, multipartMessages);
return new RequestBuilder<>(method, url, responseMapper, credentials, headers, body, multipartMessages, ignoreCookies);
}

@Override
Expand Down Expand Up @@ -121,4 +123,8 @@ boolean isMultipart() {
public InputStream getBody() {
return body.orElseThrow(BriefcaseException::new);
}

public boolean ignoreCookies() {
return ignoreCookies;
}
}
48 changes: 27 additions & 21 deletions src/org/opendatakit/briefcase/reused/http/RequestBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,31 +71,33 @@ public class RequestBuilder<T> {
private final Map<String, String> headers;
private final Optional<InputStream> body;
private final List<MultipartMessage> multipartMessages;
private final boolean ignoreCookies;

RequestBuilder(RequestMethod method, URL baseUrl, Function<InputStream, T> responseMapper, Optional<Credentials> credentials, Map<String, String> headers, Optional<InputStream> body, List<MultipartMessage> multipartMessages) {
RequestBuilder(RequestMethod method, URL baseUrl, Function<InputStream, T> responseMapper, Optional<Credentials> credentials, Map<String, String> headers, Optional<InputStream> body, List<MultipartMessage> multipartMessages, boolean ignoreCookies) {
this.method = method;
this.baseUrl = baseUrl;
this.credentials = credentials;
this.responseMapper = responseMapper;
this.headers = headers;
this.body = body;
this.multipartMessages = multipartMessages;
this.ignoreCookies = ignoreCookies;
}

public static RequestBuilder<InputStream> get(String baseUrl) {
return new RequestBuilder<>(GET, url(stripTrailingSlash(baseUrl)), Function.identity(), empty(), new HashMap<>(), empty(), emptyList());
return new RequestBuilder<>(GET, url(stripTrailingSlash(baseUrl)), Function.identity(), empty(), new HashMap<>(), empty(), emptyList(), false);
}

public static RequestBuilder<InputStream> get(URL baseUrl) {
return new RequestBuilder<>(GET, url(stripTrailingSlash(baseUrl)), Function.identity(), empty(), new HashMap<>(), empty(), emptyList());
return new RequestBuilder<>(GET, url(stripTrailingSlash(baseUrl)), Function.identity(), empty(), new HashMap<>(), empty(), emptyList(), false);
}

public static RequestBuilder<InputStream> post(URL baseUrl) {
return new RequestBuilder<>(POST, url(stripTrailingSlash(baseUrl)), Function.identity(), empty(), new HashMap<>(), empty(), emptyList());
return new RequestBuilder<>(POST, url(stripTrailingSlash(baseUrl)), Function.identity(), empty(), new HashMap<>(), empty(), emptyList(), false);
}

public static RequestBuilder<InputStream> head(URL baseUrl) {
return new RequestBuilder<>(HEAD, url(stripTrailingSlash(baseUrl)), Function.identity(), empty(), new HashMap<>(), empty(), emptyList());
return new RequestBuilder<>(HEAD, url(stripTrailingSlash(baseUrl)), Function.identity(), empty(), new HashMap<>(), empty(), emptyList(), false);
}

private static String readString(InputStream in) {
Expand Down Expand Up @@ -176,54 +178,54 @@ public static boolean isUri(String candidate) {
}

public Request<T> build() {
return new Request<>(method, baseUrl, credentials, responseMapper, headers, body, multipartMessages);
return new Request<>(method, baseUrl, credentials, responseMapper, headers, body, multipartMessages, ignoreCookies);
}

public RequestBuilder<String> asText() {
return new RequestBuilder<>(method, baseUrl, RequestBuilder::readString, credentials, headers, body, multipartMessages);
return new RequestBuilder<>(method, baseUrl, RequestBuilder::readString, credentials, headers, body, multipartMessages, ignoreCookies);
}

public RequestBuilder<XmlElement> asXmlElement() {
return new RequestBuilder<>(method, baseUrl, RequestBuilder::readXmlElement, credentials, headers, body, multipartMessages);
return new RequestBuilder<>(method, baseUrl, RequestBuilder::readXmlElement, credentials, headers, body, multipartMessages, ignoreCookies);
}

public RequestBuilder<Map<String, Object>> asJsonMap() {
return new RequestBuilder<>(method, baseUrl, RequestBuilder::readJsonMap, credentials, headers, body, multipartMessages);
return new RequestBuilder<>(method, baseUrl, RequestBuilder::readJsonMap, credentials, headers, body, multipartMessages, ignoreCookies);
}

public RequestBuilder<List<Map>> asJsonList() {
return new RequestBuilder<>(method, baseUrl, RequestBuilder.readJsonList(Map.class), credentials, headers, body, multipartMessages);
return new RequestBuilder<>(method, baseUrl, RequestBuilder.readJsonList(Map.class), credentials, headers, body, multipartMessages, ignoreCookies);
}

public <U> RequestBuilder<List<U>> asJsonList(Class<U> mappingClass) {
return new RequestBuilder<>(method, baseUrl, RequestBuilder.readJsonList(mappingClass), credentials, headers, body, multipartMessages);
return new RequestBuilder<>(method, baseUrl, RequestBuilder.readJsonList(mappingClass), credentials, headers, body, multipartMessages, ignoreCookies);
}

public RequestBuilder<Void> downloadTo(Path target) {
return new RequestBuilder<>(method, baseUrl, in -> {
copy(in, target, REPLACE_EXISTING);
return null;
}, credentials, headers, body, multipartMessages);
}, credentials, headers, body, multipartMessages, ignoreCookies);
}

public <U> RequestBuilder<U> withResponseMapper(Function<T, U> mapper) {
return new RequestBuilder<>(method, baseUrl, responseMapper.andThen(mapper), credentials, headers, body, multipartMessages);
return new RequestBuilder<>(method, baseUrl, responseMapper.andThen(mapper), credentials, headers, body, multipartMessages, ignoreCookies);
}

public RequestBuilder<T> withCredentials(Optional<Credentials> credentials) {
return new RequestBuilder<>(method, baseUrl, responseMapper, credentials, headers, body, multipartMessages);
return new RequestBuilder<>(method, baseUrl, responseMapper, credentials, headers, body, multipartMessages, ignoreCookies);
}

public RequestBuilder<T> withCredentials(Credentials credentials) {
return new RequestBuilder<>(method, baseUrl, responseMapper, Optional.of(credentials), headers, body, multipartMessages);
return new RequestBuilder<>(method, baseUrl, responseMapper, Optional.of(credentials), headers, body, multipartMessages, ignoreCookies);
}

public RequestBuilder<T> withPath(String path) {
int startOffset = path.startsWith("/") ? 1 : 0;
int endOffset = path.endsWith("/") ? 1 : 0;
String cleanPath = path.substring(startOffset, path.length() - endOffset);
URL newBaseUrl = url(baseUrl + "/" + cleanPath);
return new RequestBuilder<>(method, newBaseUrl, responseMapper, credentials, headers, body, multipartMessages);
return new RequestBuilder<>(method, newBaseUrl, responseMapper, credentials, headers, body, multipartMessages, ignoreCookies);
}

@SafeVarargs
Expand All @@ -232,27 +234,31 @@ public final RequestBuilder<T> withQuery(Pair<String, String>... keyValues) {
throw new BriefcaseException("Can't apply withQuery() twice");
String queryString = Stream.of(keyValues).map(p -> p.getLeft() + "=" + urlEncode(p.getRight())).collect(joining("&"));
URL newBaseUrl = url(baseUrl.toString() + "?" + queryString);
return new RequestBuilder<>(method, newBaseUrl, responseMapper, credentials, headers, body, multipartMessages);
return new RequestBuilder<>(method, newBaseUrl, responseMapper, credentials, headers, body, multipartMessages, ignoreCookies);
}

public RequestBuilder<T> withBody(String bodyContents) {
Optional<InputStream> newBody = Optional.of(new ByteArrayInputStream(bodyContents.getBytes(UTF_8)));
return new RequestBuilder<>(method, baseUrl, responseMapper, credentials, headers, newBody, multipartMessages);
return new RequestBuilder<>(method, baseUrl, responseMapper, credentials, headers, newBody, multipartMessages, ignoreCookies);
}

public RequestBuilder<T> withBody(InputStream body) {
return new RequestBuilder<>(method, baseUrl, responseMapper, credentials, headers, Optional.of(body), multipartMessages);
return new RequestBuilder<>(method, baseUrl, responseMapper, credentials, headers, Optional.of(body), multipartMessages, ignoreCookies);
}

public RequestBuilder<T> withHeader(String name, String value) {
Map<String, String> newHeaders = new HashMap<>(headers);
newHeaders.put(name, value);
return new RequestBuilder<>(method, baseUrl, responseMapper, credentials, newHeaders, body, multipartMessages);
return new RequestBuilder<>(method, baseUrl, responseMapper, credentials, newHeaders, body, multipartMessages, ignoreCookies);
}

public RequestBuilder<T> withMultipartMessage(String name, String contentType, String attachmentName, InputStream messageBody) {
List<MultipartMessage> newMultipartMessages = new ArrayList<>(multipartMessages);
newMultipartMessages.add(new MultipartMessage(name, contentType, attachmentName, messageBody));
return new RequestBuilder<>(method, baseUrl, responseMapper, credentials, headers, body, newMultipartMessages);
return new RequestBuilder<>(method, baseUrl, responseMapper, credentials, headers, body, newMultipartMessages, ignoreCookies);
}

public RequestBuilder<T> withIgnoreCookies() {
return new RequestBuilder<>(method, baseUrl, responseMapper, credentials, headers, body, multipartMessages, true);
}
}
Loading

0 comments on commit 7bb6185

Please sign in to comment.