Make HttpRequest toXContent / parse method coherent (elastic/elasticsearch#4153)
This commit changes the toXContent() method so that it generates content that can be parsed again by the parse() method. Before this commit, the authentication of the HTTP request is rendered as: { "auth": {"username": "foo", "password": "bar" } } but the parsing method expects the authentication type to be a root object: { "auth": { "basic" : {"username": "foo", "password": "bar" } } } The toXContent method has been adapted to include the type of authentication in the generated content. Original commit: elastic/x-pack-elasticsearch@b740466109
This commit is contained in:
parent
efe6524253
commit
c9b3de6f23
|
@ -153,7 +153,9 @@ public class HttpRequest implements ToXContent {
|
|||
builder.field(Field.HEADERS.getPreferredName(), headers);
|
||||
}
|
||||
if (auth != null) {
|
||||
builder.field(Field.AUTH.getPreferredName(), auth, params);
|
||||
builder.startObject(Field.AUTH.getPreferredName())
|
||||
.field(auth.type(), auth, params)
|
||||
.endObject();
|
||||
}
|
||||
if (body != null) {
|
||||
builder.field(Field.BODY.getPreferredName(), body);
|
||||
|
@ -219,6 +221,9 @@ public class HttpRequest implements ToXContent {
|
|||
}
|
||||
sb.append("], ");
|
||||
}
|
||||
if (auth != null) {
|
||||
sb.append("auth=[").append(auth.type()).append("], ");
|
||||
}
|
||||
sb.append("connection_timeout=[").append(connectionTimeout).append("], ");
|
||||
sb.append("read_timeout=[").append(readTimeout).append("], ");
|
||||
if (proxy != null) {
|
||||
|
|
|
@ -5,23 +5,24 @@
|
|||
*/
|
||||
package org.elasticsearch.xpack.common.http;
|
||||
|
||||
import com.carrotsearch.randomizedtesting.annotations.Repeat;
|
||||
import org.elasticsearch.ElasticsearchParseException;
|
||||
import org.elasticsearch.common.bytes.BytesReference;
|
||||
import org.elasticsearch.common.unit.TimeValue;
|
||||
import org.elasticsearch.common.xcontent.ToXContent;
|
||||
import org.elasticsearch.common.xcontent.XContentBuilder;
|
||||
import org.elasticsearch.common.xcontent.XContentHelper;
|
||||
import org.elasticsearch.common.xcontent.XContentParser;
|
||||
import org.elasticsearch.common.xcontent.json.JsonXContent;
|
||||
import org.elasticsearch.test.ESTestCase;
|
||||
import org.elasticsearch.xpack.common.http.HttpRequest;
|
||||
import org.elasticsearch.xpack.common.http.Scheme;
|
||||
import org.elasticsearch.xpack.common.http.auth.HttpAuthRegistry;
|
||||
import org.elasticsearch.xpack.common.http.auth.basic.BasicAuth;
|
||||
import org.elasticsearch.xpack.common.http.auth.basic.BasicAuthFactory;
|
||||
|
||||
import java.util.HashMap;
|
||||
import java.util.Map;
|
||||
|
||||
import static java.util.Collections.singletonMap;
|
||||
import static org.elasticsearch.common.xcontent.XContentFactory.cborBuilder;
|
||||
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
|
||||
import static org.elasticsearch.common.xcontent.XContentFactory.smileBuilder;
|
||||
import static org.elasticsearch.common.xcontent.XContentFactory.yamlBuilder;
|
||||
import static org.hamcrest.Matchers.containsString;
|
||||
import static org.hamcrest.Matchers.is;
|
||||
import static org.mockito.Mockito.mock;
|
||||
|
@ -116,7 +117,25 @@ public class HttpRequestTests extends ESTestCase {
|
|||
builder.proxy(new HttpProxy(randomAsciiOfLength(10), randomIntBetween(1024, 65000)));
|
||||
}
|
||||
|
||||
builder.build().toXContent(jsonBuilder(), ToXContent.EMPTY_PARAMS);
|
||||
final HttpRequest httpRequest = builder.build();
|
||||
assertNotNull(httpRequest);
|
||||
|
||||
BytesReference bytes = null;
|
||||
try (XContentBuilder xContentBuilder = randomFrom(jsonBuilder(), smileBuilder(), yamlBuilder(), cborBuilder())) {
|
||||
httpRequest.toXContent(xContentBuilder, ToXContent.EMPTY_PARAMS);
|
||||
bytes = xContentBuilder.bytes();
|
||||
}
|
||||
|
||||
HttpAuthRegistry registry = new HttpAuthRegistry(singletonMap(BasicAuth.TYPE, new BasicAuthFactory(null)));
|
||||
HttpRequest.Parser httpRequestParser = new HttpRequest.Parser(registry);
|
||||
|
||||
try (XContentParser parser = XContentHelper.createParser(bytes)) {
|
||||
assertNull(parser.currentToken());
|
||||
parser.nextToken();
|
||||
|
||||
HttpRequest parsedRequest = httpRequestParser.parse(parser);
|
||||
assertEquals(httpRequest, parsedRequest);
|
||||
}
|
||||
}
|
||||
|
||||
private void assertThatManualBuilderEqualsParsingFromUrl(String url, HttpRequest.Builder builder) throws Exception {
|
||||
|
|
|
@ -18,11 +18,13 @@ import org.elasticsearch.xpack.common.http.HttpRequest;
|
|||
import org.elasticsearch.xpack.common.http.HttpResponse;
|
||||
import org.elasticsearch.xpack.common.http.auth.HttpAuthRegistry;
|
||||
import org.elasticsearch.xpack.common.http.auth.basic.BasicAuth;
|
||||
import org.elasticsearch.xpack.common.http.auth.basic.BasicAuthFactory;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.HashMap;
|
||||
import java.util.Map;
|
||||
|
||||
import static java.util.Collections.singletonMap;
|
||||
import static org.elasticsearch.common.xcontent.XContentFactory.cborBuilder;
|
||||
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
|
||||
import static org.elasticsearch.common.xcontent.XContentFactory.smileBuilder;
|
||||
|
@ -33,11 +35,9 @@ import static org.elasticsearch.xpack.notification.jira.JiraIssue.resolveFailure
|
|||
import static org.hamcrest.CoreMatchers.equalTo;
|
||||
import static org.hamcrest.Matchers.hasEntry;
|
||||
import static org.hamcrest.Matchers.is;
|
||||
import static org.mockito.Mockito.mock;
|
||||
|
||||
public class JiraIssueTests extends ESTestCase {
|
||||
|
||||
@AwaitsFix(bugUrl = "https://github.com/elastic/x-plugins/pull/4153")
|
||||
public void testToXContent() throws Exception {
|
||||
final JiraIssue issue = randomJiraIssue();
|
||||
|
||||
|
@ -68,7 +68,8 @@ public class JiraIssueTests extends ESTestCase {
|
|||
} else if ("result".equals(currentFieldName)) {
|
||||
parsedResult = parser.map();
|
||||
} else if ("request".equals(currentFieldName)) {
|
||||
HttpRequest.Parser httpRequestParser = new HttpRequest.Parser(mock(HttpAuthRegistry.class));
|
||||
HttpAuthRegistry registry = new HttpAuthRegistry(singletonMap(BasicAuth.TYPE, new BasicAuthFactory(null)));
|
||||
HttpRequest.Parser httpRequestParser = new HttpRequest.Parser(registry);
|
||||
parsedRequest = httpRequestParser.parse(parser);
|
||||
} else if ("response".equals(currentFieldName)) {
|
||||
parsedResponse = HttpResponse.parse(parser);
|
||||
|
|
|
@ -208,12 +208,12 @@ public class HttpSecretsIntegrationTests extends AbstractWatcherIntegrationTestC
|
|||
assertThat(value, instanceOf(Number.class));
|
||||
assertThat(((Number) value).intValue(), is(200));
|
||||
|
||||
value = contentSource.getValue("result.actions.0.webhook.request.auth.username");
|
||||
value = contentSource.getValue("result.actions.0.webhook.request.auth.basic.username");
|
||||
assertThat(value, notNullValue());
|
||||
assertThat(value, instanceOf(String.class));
|
||||
assertThat(value, is(USERNAME)); // the auth username exists
|
||||
|
||||
value = contentSource.getValue("result.actions.0.webhook.request.auth.password");
|
||||
value = contentSource.getValue("result.actions.0.webhook.request.auth.basic.password");
|
||||
assertThat(value, nullValue()); // but the auth password was filtered out
|
||||
|
||||
RecordedRequest request = webServer.takeRequest();
|
||||
|
|
Loading…
Reference in New Issue