You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by neykov <gi...@git.apache.org> on 2014/09/23 14:34:14 UTC
[GitHub] incubator-brooklyn pull request: Support credentials parameters in...
GitHub user neykov opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/185
Support credentials parameters in URLs - http://user:pass@host
Use HttpClient for HTTP URLs in ResourceUtils.getResourceFromUrl
HttpUrlConnection doesn't support authenticating using credentials in the url (http://user:pass@host) and redirecting between protocols.
Switch to HttpClient which supports multiple authentication protocols and manually get the credentials from the URL, passing them to HttpClient.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/neykov/incubator-brooklyn authorized-resource
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/185.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #185
----
commit c98f5b63bf7acc28d053f0dd4883513776095220
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Date: 2014-09-23T12:32:34Z
Use HttpClient for HTTP URLs in ResourceUtils.getResourceFromUrl
HttpUrlConnection doesn't support authenticating using credentials in the url (http://user:pass@host) and redirecting between protocols.
Switch to HttpClient which supports multiple authentication protocols and manually get the credentials from the URL, passing them to HttpClient.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Credentials parameters in URLs - ...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/185#discussion_r17906035
--- Diff: core/src/test/java/brooklyn/util/ResourceUtilsHttpTest.java ---
@@ -0,0 +1,173 @@
+package brooklyn.util;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.InetSocketAddress;
+
+import org.apache.http.Header;
+import org.apache.http.HttpException;
+import org.apache.http.HttpRequest;
+import org.apache.http.HttpResponse;
+import org.apache.http.HttpStatus;
+import org.apache.http.entity.StringEntity;
+import org.apache.http.localserver.LocalTestServer;
+import org.apache.http.localserver.RequestBasicAuth;
+import org.apache.http.localserver.ResponseBasicUnauthorized;
+import org.apache.http.message.BasicHeader;
+import org.apache.http.protocol.BasicHttpProcessor;
+import org.apache.http.protocol.HttpContext;
+import org.apache.http.protocol.HttpRequestHandler;
+import org.apache.http.protocol.ResponseConnControl;
+import org.apache.http.protocol.ResponseContent;
+import org.apache.http.protocol.ResponseServer;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import brooklyn.util.stream.Streams;
+
+public class ResourceUtilsHttpTest {
+ private ResourceUtils utils;
+ private LocalTestServer server;
+ private String baseUrl;
+
+ @BeforeClass(alwaysRun=true)
+ public void setUp() throws Exception {
+ utils = ResourceUtils.create(this, "mycontext");
+
+ BasicHttpProcessor httpProcessor = new BasicHttpProcessor();
+ httpProcessor.addInterceptor(new ResponseServer());
+ httpProcessor.addInterceptor(new ResponseContent());
+ httpProcessor.addInterceptor(new ResponseConnControl());
+ httpProcessor.addInterceptor(new RequestBasicAuth());
+ httpProcessor.addInterceptor(new ResponseBasicUnauthorized());
+
+ server = new LocalTestServer(httpProcessor, null);
+ server.register("/simple", new SimpleResponseHandler("OK"));
+ server.register("/empty", new SimpleResponseHandler(HttpStatus.SC_NO_CONTENT, ""));
+ server.register("/missing", new SimpleResponseHandler(HttpStatus.SC_NOT_FOUND, "Missing"));
+ server.register("/redirect", new SimpleResponseHandler(HttpStatus.SC_MOVED_TEMPORARILY, "Redirect", new BasicHeader("Location", "/simple")));
+ server.register("/cycle", new SimpleResponseHandler(HttpStatus.SC_MOVED_TEMPORARILY, "Redirect", new BasicHeader("Location", "/cycle")));
+ server.register("/secure", new SimpleResponseHandler(HttpStatus.SC_MOVED_TEMPORARILY, "Redirect", new BasicHeader("Location", "https://0.0.0.0/")));
+ server.register("/auth", new AuthHandler("test", "test", "OK"));
--- End diff --
also worth testing that `baseUrl+"/simple?no@auth:needed` works? the current auth regex strategy should mistakenly match this, if i'm not mistaken ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Credentials parameters in URLs - ...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/185#discussion_r17911954
--- Diff: core/src/main/java/brooklyn/util/ResourceUtils.java ---
@@ -62,6 +72,7 @@
private static final Logger log = LoggerFactory.getLogger(ResourceUtils.class);
private static final List<Function<Object,BrooklynClassLoadingContext>> classLoaderProviders = Lists.newCopyOnWriteArrayList();
+ private static final Pattern AUTH_URL = Pattern.compile("http(?:s?)://([^:]+):([^@]+)@.*");
--- End diff --
> what does (?:s?) do?
Optional 's'
(?: is non-capturing group with s? inside.
Re-worked the code so it doesn't use the regex at all.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Credentials parameters in URLs - ...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/185#discussion_r17905675
--- Diff: core/src/main/java/brooklyn/util/ResourceUtils.java ---
@@ -388,6 +403,43 @@ public void close() throws IOException {
}
}
+ //For HTTP(S) targets use HttpClient so
+ //we can do authentication
+ private InputStream getResourceViaHttp(String url) throws IOException {
+ HttpClientBuilder builder = HttpTool.httpClientBuilder()
+ .laxRedirect(true)
+ .uri(url);
+ Credentials credentials = getUrlCredentials(url);
+ if (credentials != null) {
+ builder.credentials(credentials);
+ }
+ HttpClient client = builder.build();
+ HttpResponse result = client.execute(new HttpGet(url));
+ int statusCode = result.getStatusLine().getStatusCode();
+ if (HttpTool.isStatusCodeHealthy(statusCode)) {
+ HttpEntity entity = result.getEntity();
+ if (entity != null) {
+ return entity.getContent();
+ } else {
+ return new ByteArrayInputStream(new byte[0]);
+ }
+ } else {
+ EntityUtils.consume(result.getEntity());
+ throw new IllegalStateException("Invalid response invoking " + url + ": response code " + statusCode);
+ }
+ }
+
+ private Credentials getUrlCredentials(String url) {
+ Matcher m = AUTH_URL.matcher(url);
+ if (m.matches() && m.groupCount() == 3) {
+ String username = m.group(1);
--- End diff --
do `username` and `password` need to be url unescaped?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Credentials parameters in URLs - ...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/185#issuecomment-56557266
very nice, i did not know about `get{,Raw}UserInfo()`; good find!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Credentials parameters in URLs - ...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/185#issuecomment-56514849
looks great -- some very pedantic comments; if this is past the point of marginal return just say so (no objections to merging as is)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Credentials parameters in URLs - ...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/185#discussion_r17905943
--- Diff: core/src/test/java/brooklyn/util/ResourceUtilsHttpTest.java ---
@@ -0,0 +1,173 @@
+package brooklyn.util;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.InetSocketAddress;
+
+import org.apache.http.Header;
+import org.apache.http.HttpException;
+import org.apache.http.HttpRequest;
+import org.apache.http.HttpResponse;
+import org.apache.http.HttpStatus;
+import org.apache.http.entity.StringEntity;
+import org.apache.http.localserver.LocalTestServer;
+import org.apache.http.localserver.RequestBasicAuth;
+import org.apache.http.localserver.ResponseBasicUnauthorized;
+import org.apache.http.message.BasicHeader;
+import org.apache.http.protocol.BasicHttpProcessor;
+import org.apache.http.protocol.HttpContext;
+import org.apache.http.protocol.HttpRequestHandler;
+import org.apache.http.protocol.ResponseConnControl;
+import org.apache.http.protocol.ResponseContent;
+import org.apache.http.protocol.ResponseServer;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import brooklyn.util.stream.Streams;
+
+public class ResourceUtilsHttpTest {
+ private ResourceUtils utils;
+ private LocalTestServer server;
+ private String baseUrl;
+
+ @BeforeClass(alwaysRun=true)
+ public void setUp() throws Exception {
+ utils = ResourceUtils.create(this, "mycontext");
+
+ BasicHttpProcessor httpProcessor = new BasicHttpProcessor();
+ httpProcessor.addInterceptor(new ResponseServer());
+ httpProcessor.addInterceptor(new ResponseContent());
+ httpProcessor.addInterceptor(new ResponseConnControl());
+ httpProcessor.addInterceptor(new RequestBasicAuth());
+ httpProcessor.addInterceptor(new ResponseBasicUnauthorized());
+
+ server = new LocalTestServer(httpProcessor, null);
+ server.register("/simple", new SimpleResponseHandler("OK"));
+ server.register("/empty", new SimpleResponseHandler(HttpStatus.SC_NO_CONTENT, ""));
+ server.register("/missing", new SimpleResponseHandler(HttpStatus.SC_NOT_FOUND, "Missing"));
+ server.register("/redirect", new SimpleResponseHandler(HttpStatus.SC_MOVED_TEMPORARILY, "Redirect", new BasicHeader("Location", "/simple")));
+ server.register("/cycle", new SimpleResponseHandler(HttpStatus.SC_MOVED_TEMPORARILY, "Redirect", new BasicHeader("Location", "/cycle")));
+ server.register("/secure", new SimpleResponseHandler(HttpStatus.SC_MOVED_TEMPORARILY, "Redirect", new BasicHeader("Location", "https://0.0.0.0/")));
+ server.register("/auth", new AuthHandler("test", "test", "OK"));
+ server.start();
+
+ InetSocketAddress addr = server.getServiceAddress();
+ baseUrl = "http://" + addr.getHostName() + ":" + addr.getPort();
+ }
+
+ @AfterClass(alwaysRun=true)
+ public void tearDown() throws Exception {
+ server.stop();
+ }
+
+ @Test
+ public void testGet() throws Exception {
+ InputStream stream = utils.getResourceFromUrl(baseUrl + "/simple");
+ assertEquals(Streams.readFullyString(stream), "OK");
+ }
+
+ @Test
+ public void testGetEmpty() throws Exception {
+ InputStream stream = utils.getResourceFromUrl(baseUrl + "/empty");
+ assertEquals(Streams.readFullyString(stream), "");
+ }
+
+ @Test
+ public void testGetProtected() throws Exception {
--- End diff --
test that *not* supplying credentials fails?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Credentials parameters in URLs - ...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/185#discussion_r17905828
--- Diff: core/src/main/java/brooklyn/util/ResourceUtils.java ---
@@ -62,6 +72,7 @@
private static final Logger log = LoggerFactory.getLogger(ResourceUtils.class);
private static final List<Function<Object,BrooklynClassLoadingContext>> classLoaderProviders = Lists.newCopyOnWriteArrayList();
+ private static final Pattern AUTH_URL = Pattern.compile("http(?:s?)://([^:]+):([^@]+)@.*");
--- End diff --
what does `(?:s?)` do?
do we need to exclude `/` before the `@`?
my first guess (untested) would be something like `http(s?)://([^/:]+):([^/@]+)@.*`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Credentials parameters in URLs - ...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/185#discussion_r17905932
--- Diff: core/src/test/java/brooklyn/util/ResourceUtilsHttpTest.java ---
@@ -0,0 +1,173 @@
+package brooklyn.util;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.InetSocketAddress;
+
+import org.apache.http.Header;
+import org.apache.http.HttpException;
+import org.apache.http.HttpRequest;
+import org.apache.http.HttpResponse;
+import org.apache.http.HttpStatus;
+import org.apache.http.entity.StringEntity;
+import org.apache.http.localserver.LocalTestServer;
+import org.apache.http.localserver.RequestBasicAuth;
+import org.apache.http.localserver.ResponseBasicUnauthorized;
+import org.apache.http.message.BasicHeader;
+import org.apache.http.protocol.BasicHttpProcessor;
+import org.apache.http.protocol.HttpContext;
+import org.apache.http.protocol.HttpRequestHandler;
+import org.apache.http.protocol.ResponseConnControl;
+import org.apache.http.protocol.ResponseContent;
+import org.apache.http.protocol.ResponseServer;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import brooklyn.util.stream.Streams;
+
+public class ResourceUtilsHttpTest {
+ private ResourceUtils utils;
+ private LocalTestServer server;
+ private String baseUrl;
+
+ @BeforeClass(alwaysRun=true)
+ public void setUp() throws Exception {
+ utils = ResourceUtils.create(this, "mycontext");
+
+ BasicHttpProcessor httpProcessor = new BasicHttpProcessor();
+ httpProcessor.addInterceptor(new ResponseServer());
+ httpProcessor.addInterceptor(new ResponseContent());
+ httpProcessor.addInterceptor(new ResponseConnControl());
+ httpProcessor.addInterceptor(new RequestBasicAuth());
+ httpProcessor.addInterceptor(new ResponseBasicUnauthorized());
+
+ server = new LocalTestServer(httpProcessor, null);
+ server.register("/simple", new SimpleResponseHandler("OK"));
+ server.register("/empty", new SimpleResponseHandler(HttpStatus.SC_NO_CONTENT, ""));
+ server.register("/missing", new SimpleResponseHandler(HttpStatus.SC_NOT_FOUND, "Missing"));
+ server.register("/redirect", new SimpleResponseHandler(HttpStatus.SC_MOVED_TEMPORARILY, "Redirect", new BasicHeader("Location", "/simple")));
+ server.register("/cycle", new SimpleResponseHandler(HttpStatus.SC_MOVED_TEMPORARILY, "Redirect", new BasicHeader("Location", "/cycle")));
+ server.register("/secure", new SimpleResponseHandler(HttpStatus.SC_MOVED_TEMPORARILY, "Redirect", new BasicHeader("Location", "https://0.0.0.0/")));
+ server.register("/auth", new AuthHandler("test", "test", "OK"));
--- End diff --
maybe an `server.register("/auth_naughty_chars", new AuthHandler("m@ry:/", "p@assword:/", "OK"));` ... and corresponding test (url-encoding the username and password to test decoding) ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Credentials parameters in URLs - ...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/185#issuecomment-56528806
Great comments, amended changes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Credentials parameters in URLs - ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/185
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---