Skip to content

Commit

Permalink
Do not reuse HTTP client instance (bug fix for #37) (#55)
Browse files Browse the repository at this point in the history
* Do not reuse HTTP client instance (bug fix for #37)

* Update README and pom.xml

* Update README and pom.xml
  • Loading branch information
dzieciou authored Nov 2, 2020
1 parent e66f6b2 commit 1476e85
Show file tree
Hide file tree
Showing 9 changed files with 283 additions and 18 deletions.
29 changes: 18 additions & 11 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -332,8 +332,12 @@ given()

## Releases

2.0.1:
* Bug fix: Do not reuse HTTP client instance across multiple/parallel requests (fix for [#37](https://github.com/dzieciou/curl-logger/issues/37))

2.0.0:
* Fix to invalid escaping of characters longer than 8 bits (#47, thanks to Srepfler Srdan for reporting it)
* Fix to invalid escaping of characters longer than 8 bits ([#47](https://github.com/dzieciou/curl-logger/issues/47),
thanks to Srepfler Srdan for reporting it)
* Tested with latest REST-assured (4.3.1)
* Support for custom log levels (thanks to Jérémie Bresson for pull request)
* Support for custom curl handlers
Expand All @@ -343,26 +347,26 @@ given()
1.0.5:
* Upgrade to REST-assured 4.0.0.
* Update test scope dependencies.
* Fix character escaping for both POSIX and Windows platforms (many thanks to Chirag008 for rausung
the issue: https://github.com/dzieciou/curl-logger/issues/25)
* Bug fix: fix character escaping for both POSIX and Windows platforms (many thanks to Chirag008 for raising
the issue: [#25](https://github.com/dzieciou/curl-logger/issues/25))

1.0.4:
* Bug fix: HTTPS protocol was not always recognized correctly
(https://github.com/dzieciou/curl-logger/issues/17). Many thanks to pafitchett-ks for troubleshooting.
([#17](https://github.com/dzieciou/curl-logger/issues/17)). Many thanks to pafitchett-ks for troubleshooting.
* Support slf4j 1.8.0-beta2.
* Support rest-assured 3.2.0.

1.0.3:
* Bug fix: Invalid basic authentication headers are failing curl generation
(https://github.com/dzieciou/curl-logger/issues/15)
([#15](https://github.com/dzieciou/curl-logger/issues/15))

1.0.2:
* Bug fix: CurlLogger was failing when multiple Cookie headers are present in HTTP Request. Now it
only prints warning (https://github.com/dzieciou/curl-logger/issues/13)
only prints warning ([#37](https://github.com/dzieciou/curl-logger/issues/13))

1.0.1:
* Bug fix: `CurlLoggingRestAssuredConfigBuilder` was not updating `RestAssuredConfig` properly
(https://github.com/dzieciou/curl-logger/issues/4):
([#4](https://github.com/dzieciou/curl-logger/issues/4)):

1.0.0:

Expand All @@ -375,16 +379,19 @@ only prints warning (https://github.com/dzieciou/curl-logger/issues/13)

* Added possibility to print shorter versions of curl parameters, e.g., -v instead of --verbose
* Added possibility to modify a curl command before printing it, inspired by the suggestion from
Alexey Dushen (blacky0x0): https://github.com/dzieciou/curl-logger/issues/2.
Alexey Dushen (blacky0x0): [#2](https://github.com/dzieciou/curl-logger/issues/2).

0.6:
* Fixed bug: For each cookie a separate `-b cookie=content` parameter was generated (https://github.com/dzieciou/curl-logger/issues/4)
* Fixed bug: For each cookie a separate `-b cookie=content` parameter was generated
([#6](https://github.com/dzieciou/curl-logger/issues/4))
* Upgraded to REST-assured 3.0.2
* Simplified curl-logger configuration with `CurlLoggingRestAssuredConfigBuilder`, based on suggestion from Tao Zhang (https://github.com/dzieciou/curl-logger/issues/4)
* Simplified curl-logger configuration with `CurlLoggingRestAssuredConfigBuilder`, based on suggestion from Tao Zhang
([#4](https://github.com/dzieciou/curl-logger/issues/4))

0.5:

* Upgraded to REST-assured 3.0.1 that contains important fix impacting curl-logger: Cookie attributes are no longer sent in request in accordance with RFC6265.
* Upgraded to REST-assured 3.0.1 that contains important fix impacting curl-logger: Cookie attributes are no longer sent
in request in accordance with RFC6265.
* Fixed bug: cookie values can have = sign inside so we need to get around them somehow
* Cookie strings are now escaped
* `CurlLoggingInterceptor`'s constructor is now protected to make extending it possible
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.github.dzieciou.testing</groupId>
<artifactId>curl-logger</artifactId>
<version>2.0.0</version>
<version>2.0.1-SNAPSHOT</version>
<packaging>jar</packaging>
<url>https://github.com/dzieciou/curl-logger</url>
<name>com.github.dzieciou.testing:curl-logger</name>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ private String escape(char c) {
case '\'': return "\\'";
case '\t': return "\\t";
case '\r': return "\\r";
// '@' character has a special meaning in --data-binary (loadin a file)
// '@' character has a special meaning in --data-binary (loading a file)
// So we need to escape it
case '@': return escapeAsHex(c);
default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ public static RestAssuredConfig updateConfig(RestAssuredConfig config, Options o
handlers);
return config
.httpClient(config.getHttpClientConfig()
.reuseHttpClientInstance()
.dontReuseHttpClientInstance()
.httpClientFactory(new MyHttpClientFactory(originalFactory, interceptor)));
}

Expand Down
107 changes: 107 additions & 0 deletions src/test/java/com/github/dzieciou/testing/curl/Bug.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
package com.github.dzieciou.testing.curl;

import io.restassured.config.HttpClientConfig;
import io.restassured.config.RestAssuredConfig;
import org.apache.http.client.HttpClient;
import org.apache.http.impl.client.AbstractHttpClient;
import org.apache.http.impl.client.DefaultHttpClient;
import org.mockserver.client.MockServerClient;
import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.function.Consumer;

import static com.github.dzieciou.testing.curl.CommandExecutor.runCommand;
import static io.restassured.RestAssured.config;
import static io.restassured.RestAssured.given;
import static io.restassured.config.HttpClientConfig.httpClientConfig;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.mockito.Mockito.mock;
import static org.mockserver.integration.ClientAndServer.startClientAndServer;
import static org.mockserver.model.HttpRequest.request;
import static org.mockserver.model.HttpResponse.response;

public class Bug {

private MockServerClient mockServer;

String matchingBody = "secret=P@ssword";

@BeforeMethod
public void setup() {
mockServer = startClientAndServer(9999);
mockServer.when(request().withBody(matchingBody)).respond(response());
}

@Test
public void buggy() throws IOException {
Options options = Options.builder().dontEscapeNonAscii().build();
final List<String> curls = new ArrayList<>();
CurlHandler handler = new CurlHandler() {
@Override
public void handle(String curl, Options options) {
curls.add(curl);
}
};
List<CurlHandler> handlers = Arrays.asList(handler, new CurlLogger());
RestAssuredConfig restAssuredConfig = getRestAssuredConfig(
new CurlGeneratingInterceptor(options, handlers));


//@formatter:off
given()
.baseUri("http://localhost")
.port(9999)
.config(restAssuredConfig)
.body(matchingBody)
.when()
.post()
.then()
.statusCode(200);

//@formatter:on

String curl = curls.get(0);
System.out.println(curl);
String output = runCommand(curl).stdErr;
assertThat(output, containsString("HTTP/1.1 200 OK"));

}

@AfterMethod
public void after() {
mockServer.stop();
}

private static RestAssuredConfig getRestAssuredConfig(
CurlGeneratingInterceptor curlGeneratingInterceptor) {
return config()
.httpClient(httpClientConfig()
.reuseHttpClientInstance()
.httpClientFactory(new MyHttpClientFactory(curlGeneratingInterceptor)));
}

private static class MyHttpClientFactory implements HttpClientConfig.HttpClientFactory {

private final CurlGeneratingInterceptor curlGeneratingInterceptor;

public MyHttpClientFactory(CurlGeneratingInterceptor curlGeneratingInterceptor) {
this.curlGeneratingInterceptor = curlGeneratingInterceptor;
}

@Override
public HttpClient createHttpClient() {
@SuppressWarnings("deprecation") AbstractHttpClient client = new DefaultHttpClient();
client.addRequestInterceptor(curlGeneratingInterceptor);
return client;
}
}


}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package com.github.dzieciou.testing.curl;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.util.ArrayList;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class CommandExecutor {

public static CommandOutput runCommand(String command) throws IOException {
String[] commands = tokenize(command).toArray(new String[0]);
Runtime rt = Runtime.getRuntime();
Process proc = rt.exec(commands);
return new CommandOutput(
readAll(proc.getInputStream()), readAll(proc.getErrorStream())
);
}

private static String readAll(InputStream in) throws IOException {
BufferedReader reader = new BufferedReader(new InputStreamReader(in));
StringBuffer sb = new StringBuffer();
String s;
while ((s = reader.readLine()) != null) {
sb.append(s).append("\n");
}
return sb.toString();
}

private static List<String> tokenize(String cmd) {
List<String> tokens = new ArrayList<String>();
Matcher m = Pattern.compile("([^']\\S*|'.+?')\\s*").matcher(cmd);
while (m.find())
tokens.add(m.group(1).replace("'", ""));
return tokens;
}

public static class CommandOutput {
final String stdIn;
final String stdErr;

public CommandOutput(String stdIn, String stdErr) {
this.stdIn = stdIn;
this.stdErr = stdErr;
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.github.dzieciou.testing.curl;


import io.restassured.specification.RequestSpecification;
import org.apache.http.HttpException;
import org.apache.http.HttpRequest;
import org.apache.http.HttpRequestInterceptor;
Expand All @@ -21,6 +22,7 @@
import io.restassured.config.HttpClientConfig;
import io.restassured.config.RestAssuredConfig;

import static io.restassured.RestAssured.config;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.not;
Expand Down Expand Up @@ -101,6 +103,24 @@ public void shouldSentRequestWhenUsingConfigurationFactory() {
.statusCode(200);
}

@Test
public void shouldSentSameRequestTwice() {
// Verifying fix for https://github.com/dzieciou/curl-logger/issues/37

//@formatter:off
RequestSpecification request = RestAssured.given()
.baseUri(MOCK_BASE_URI)
.port(MOCK_PORT)
.config(CurlRestAssuredConfigFactory.createConfig())
.body("anything")
.when();

request.post("/");

request.post("/");
//@formatter:on
}

@AfterClass
public void closeMock() {
mockServer.stop();
Expand Down
32 changes: 32 additions & 0 deletions src/test/java/com/github/dzieciou/testing/curl/Try.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package com.github.dzieciou.testing.curl;

public class Try {


public static void main(String[] args) {
for(int i=0;i<1024;i++) {
char c = (char)i;
System.out.println(i + "," + escapeAsHex(c) + "," + escapeAsHexFix(c));
assert escapeAsHex(c).equals(escapeAsHexFix(c));
}
}

private static String escapeAsHex(char c) {
int code = (int) c;
String codeAsHex = Integer.toHexString(code);
if (code < 256) {
// Add leading zero when needed to not care about the next character.
return code < 16 ? "\\x0" + codeAsHex : "\\x" + codeAsHex;
}
return String.format("\\u%04x", (int) c);
}

private static String escapeAsHexFix(char c) {
int code = c;
if (code < 256) {
return String.format("\\x%02x", (int)c);
}
return String.format("\\u%04x", (int) c);
}

}
Loading

0 comments on commit 1476e85

Please sign in to comment.