You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@tomee.apache.org by Zachary Bedell <zb...@nycourts.gov> on 2017/10/02 17:15:29 UTC
Re: Incorrect URL decoding in
MulticastConnectionFactory$URI::parseParameters
Sorry for the delay in submitting this. I've not had much luck building & passing the unit test suite for the full TomEE build. Currently failing with "javax.ws.rs.NotSupportedException: HTTP 415 Unsupported Media Type" in OpenEJB :: Examples :: REST XML JSON. Is there any documentation in terms of what's expected for the build environment for TomEE? Any Docker or other devops-ish canned configurations? I'm trying on a fresh Ubuntu 17.04 VM with JDK 8u144 and the master branch cloned.
In any case, I got far enough for the "OpenEJB :: Server :: Client" tests to run and fail with my added unit test. Included patch fixes the problem and passes the new unit test. No additional failures with my patch beyond the Unsupported Media Type which failed for me with a fresh clone before changing anything.
Sent via PR #104 on Github.
Best regards,
Zac Bedell
> On Sep 20, 2017, at 15:19, Romain Manni-Bucau <rm...@gmail.com> wrote:
>
> then
> https://github.com/apache/tomee/blob/master/server/openejb-client/src/test/java/org/apache/openejb/client/HttpConnectionTest.java#L171
> was supposed to cover it but happy to get it enhanced ;)
>
>
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> <https://blog-rmannibucau.rhcloud.com> | Old Blog
> <http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
> LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
> <https://javaeefactory-rmannibucau.rhcloud.com>
>
> 2017-09-20 21:17 GMT+02:00 Zachary Bedell <zb...@nycourts.gov>:
>
>> I'll give it a shot. Running into some trouble getting stock tests to
>> pass on Mac OS and separate problems behind a corporate proxy. Spinning a
>> Linux VM on the open WiFi to try.
>>
>> FWIW, we're not using multicast for this. I just looks like
>> HttpConnectionFactory calls the URI utility methods physically located in
>> the static URI class on MulticastConnectionFactory. Otherwise this is
>> https with sticky failover.
>>
>> -Zac
>>
>>> On Sep 20, 2017, at 12:50, Romain Manni-Bucau <rm...@gmail.com>
>> wrote:
>>>
>>> Hi Zachary
>>>
>>>
>>> We have some coverage for it in
>>> https://github.com/apache/tomee/blob/master/server/
>> openejb-client/src/test/java/org/apache/openejb/client/
>> HttpConnectionTest.java
>>>
>>> Multicast not being used that often with http (more likely with ejbd) we
>>> didnt test it, do you want to try to add a test and fix?
>>>
>>> Le 20 sept. 2017 18:28, "Zachary Bedell" <zb...@nycourts.gov> a écrit
>> :
>>>
>>> Greetings all,
>>>
>>> Pretty sure I found a bug in the way
>>> org.apache.openejb.client.MulticastConnectionFactory
>>> decodes URL parameters. The final result of the issue is that if you use
>>> HTTP basic authentication when calling ServerServlet and
>>> openejb.ejbd.authenticate-with-request=true, you can't login with a
>>> password that contains an ampersand character.
>>>
>>> The flow looks something like:
>>>
>>> 1) Create a new IntitialContext with PROVIDER_URL set to
>>> failover:sticky+random:https://myserver/ejb/invoke?basic.
>>> username=xyz&basic.password=pass%26word
>>> a) /ejb/invoke is where I have org.apache.openejb.server.
>> httpd.ServerServlet
>>> mapped
>>> b) web.xml on that mapping requires BASIC auth.
>>> c) key part of URL is the literal password "pass&word" URL encoded with
>>> ampersand -> %26
>>>
>>> 2) TomEE internals eventually end up at HttpConnectionFactory$
>> HttpConnection's
>>> constructor where line 76 calls:
>>> params = MulticastConnectionFactory.URIs.parseParamters(uri);
>>> By this time, various unwrapping has paired the URL down to:
>>> https://myserver/ejb/invoke?basic.username=xyz&basic.
>>> password=pass%26word
>>>
>>> 3) MulticastConnectionFactory...parseParameters, IE line 136:
>>> return uri.getQuery() == null ? new HashMap<String, String>(0) :
>>> parseQuery(stripPrefix(uri.getQuery(), "?"));
>>>
>>> That calls URI.getQuery() which decodes the URI's query string, then
>> passes
>>> that into parseQuery() which splits up the query parameters delimited by
>>> ampersands. The call to URI.getQuery() is the problem. For the above
>> URI,
>>> the result is:
>>> basic.username=xyz&basic.password=pass&word
>>> The ampersand in the query parameter basic.password is decoded and
>>> then indistinguishable from a query parameter separator. When passed
>> into
>>> parseQuery, the resulting value for basic.password is just "pass".
>>>
>>> Since MulticastConnectionFactory$URIs::parseQuery already calls
>>> URLDecoder.decode() on both name and value pairs, the call in
>>> parseParameters should be to URI::getRawQuery() instead of getQuery(). I
>>> think there's also a possible double decoding issue here which could
>>> corrupt certain values by decoding the value a second time.
>>>
>>> For the time being, I think I can work around this by passing the
>>> authorization query parameter with the user:pass already base64 encoded.
>>> Pretty sure this should be a complete & safe fix:
>>>
>>>
>>> diff --git a/server/openejb-client/src/main/java/org/apache/openejb/
>> client/
>>> MulticastConnectionFactory.java b/server/openejb-client/src/
>>> main/java/org/apache/openejb/client/MulticastConnectionFactory.java
>>> index 22f2f86a6a..eedb54840e 100644
>>> --- a/server/openejb-client/src/main/java/org/apache/openejb/client/
>>> MulticastConnectionFactory.java
>>> +++ b/server/openejb-client/src/main/java/org/apache/openejb/client/
>>> MulticastConnectionFactory.java
>>> @@ -133,7 +133,7 @@ public class MulticastConnectionFactory implements
>>> ConnectionFactory {
>>> }
>>>
>>> public static Map<String, String> parseParamters(final URI uri)
>>> throws URISyntaxException {
>>> - return uri.getQuery() == null ? new HashMap<String,
>> String>(0)
>>> : parseQuery(stripPrefix(uri.getQuery(), "?"));
>>> + return uri.getQuery() == null ? new HashMap<String,
>> String>(0)
>>> : parseQuery(stripPrefix(uri.getRawQuery(), "?"));
>>> }
>>>
>>> public static String stripPrefix(final String value, final String
>>> prefix) {
>>>
>>>
>>>
>>> Best regards,
>>> Zac Bedell
>>
>>
Re: Incorrect URL decoding in MulticastConnectionFactory$URI::parseParameters
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Le 2 oct. 2017 20:54, "Zachary Bedell" <zb...@nycourts.gov> a écrit :
Not sure what you mean by misses a test? The code I modified is in the
class MulticastConnectionFactory.java, but the current tests that exercise
that code are in HttpConnectionTest.java. Seems like a bit of a weird
cross-over, but a pre-existing condition. MulticastConnectionFactory
contains a static subclass URIs with methods that are called by the HTTP
related connection code.
I could move the test somewhere else, but there was already a test in
HttpConnectionTest (httpBasicSpecificConfig) that tested the exact code I
had to modify. The existing test didn't include an ampersand in the test
data. I added a new test that does include an ampersand which fails
without the modification to MulticastConnectionFactory. Both the existing
& new tests pass with the change to MulticastConnectionFactory.
It does seem a little odd that the Http connection code ends up in a class
with "multicast" in the name, but that's a much deeper change than I feel
comfortable attempting at this point.
Yes, this is fine for now. What i had in mind was other possible multicast
usages like ejbd protocol which can be impacted by this change. Not sure we
already cover with a unit test but should be here to avoid a regression at
least.
-Zac
> On Oct 2, 2017, at 13:15, Zachary Bedell <zb...@nycourts.gov> wrote:
>
> Sorry for the delay in submitting this. I've not had much luck building
& passing the unit test suite for the full TomEE build. Currently failing
with "javax.ws.rs.NotSupportedException: HTTP 415 Unsupported Media Type"
in OpenEJB :: Examples :: REST XML JSON. Is there any documentation in
terms of what's expected for the build environment for TomEE? Any Docker
or other devops-ish canned configurations? I'm trying on a fresh Ubuntu
17.04 VM with JDK 8u144 and the master branch cloned.
>
> In any case, I got far enough for the "OpenEJB :: Server :: Client" tests
to run and fail with my added unit test. Included patch fixes the problem
and passes the new unit test. No additional failures with my patch beyond
the Unsupported Media Type which failed for me with a fresh clone before
changing anything.
>
> Sent via PR #104 on Github.
>
> Best regards,
> Zac Bedell
>
>> On Sep 20, 2017, at 15:19, Romain Manni-Bucau <rm...@gmail.com>
wrote:
>>
>> then
>> https://github.com/apache/tomee/blob/master/server/
openejb-client/src/test/java/org/apache/openejb/client/
HttpConnectionTest.java#L171
>> was supposed to cover it but happy to get it enhanced ;)
>>
>>
>> Romain Manni-Bucau
>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>> <https://blog-rmannibucau.rhcloud.com> | Old Blog
>> <http://rmannibucau.wordpress.com> | Github <https://github.com/
rmannibucau> |
>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
>> <https://javaeefactory-rmannibucau.rhcloud.com>
>>
>> 2017-09-20 21:17 GMT+02:00 Zachary Bedell <zb...@nycourts.gov>:
>>
>>> I'll give it a shot. Running into some trouble getting stock tests to
>>> pass on Mac OS and separate problems behind a corporate proxy.
Spinning a
>>> Linux VM on the open WiFi to try.
>>>
>>> FWIW, we're not using multicast for this. I just looks like
>>> HttpConnectionFactory calls the URI utility methods physically located
in
>>> the static URI class on MulticastConnectionFactory. Otherwise this is
>>> https with sticky failover.
>>>
>>> -Zac
>>>
>>>> On Sep 20, 2017, at 12:50, Romain Manni-Bucau <rm...@gmail.com>
>>> wrote:
>>>>
>>>> Hi Zachary
>>>>
>>>>
>>>> We have some coverage for it in
>>>> https://github.com/apache/tomee/blob/master/server/
>>> openejb-client/src/test/java/org/apache/openejb/client/
>>> HttpConnectionTest.java
>>>>
>>>> Multicast not being used that often with http (more likely with ejbd)
we
>>>> didnt test it, do you want to try to add a test and fix?
>>>>
>>>> Le 20 sept. 2017 18:28, "Zachary Bedell" <zb...@nycourts.gov> a écrit
>>> :
>>>>
>>>> Greetings all,
>>>>
>>>> Pretty sure I found a bug in the way
>>>> org.apache.openejb.client.MulticastConnectionFactory
>>>> decodes URL parameters. The final result of the issue is that if you
use
>>>> HTTP basic authentication when calling ServerServlet and
>>>> openejb.ejbd.authenticate-with-request=true, you can't login with a
>>>> password that contains an ampersand character.
>>>>
>>>> The flow looks something like:
>>>>
>>>> 1) Create a new IntitialContext with PROVIDER_URL set to
>>>> failover:sticky+random:https://myserver/ejb/invoke?basic.
>>>> username=xyz&basic.password=pass%26word
>>>> a) /ejb/invoke is where I have org.apache.openejb.server.
>>> httpd.ServerServlet
>>>> mapped
>>>> b) web.xml on that mapping requires BASIC auth.
>>>> c) key part of URL is the literal password "pass&word" URL encoded with
>>>> ampersand -> %26
>>>>
>>>> 2) TomEE internals eventually end up at HttpConnectionFactory$
>>> HttpConnection's
>>>> constructor where line 76 calls:
>>>> params = MulticastConnectionFactory.URIs.parseParamters(uri);
>>>> By this time, various unwrapping has paired the URL down to:
>>>> https://myserver/ejb/invoke?basic.username=xyz&basic.
>>>> password=pass%26word
>>>>
>>>> 3) MulticastConnectionFactory...parseParameters, IE line 136:
>>>> return uri.getQuery() == null ? new HashMap<String, String>(0) :
>>>> parseQuery(stripPrefix(uri.getQuery(), "?"));
>>>>
>>>> That calls URI.getQuery() which decodes the URI's query string, then
>>> passes
>>>> that into parseQuery() which splits up the query parameters delimited
by
>>>> ampersands. The call to URI.getQuery() is the problem. For the above
>>> URI,
>>>> the result is:
>>>> basic.username=xyz&basic.password=pass&word
>>>> The ampersand in the query parameter basic.password is decoded and
>>>> then indistinguishable from a query parameter separator. When passed
>>> into
>>>> parseQuery, the resulting value for basic.password is just "pass".
>>>>
>>>> Since MulticastConnectionFactory$URIs::parseQuery already calls
>>>> URLDecoder.decode() on both name and value pairs, the call in
>>>> parseParameters should be to URI::getRawQuery() instead of
getQuery(). I
>>>> think there's also a possible double decoding issue here which could
>>>> corrupt certain values by decoding the value a second time.
>>>>
>>>> For the time being, I think I can work around this by passing the
>>>> authorization query parameter with the user:pass already base64
encoded.
>>>> Pretty sure this should be a complete & safe fix:
>>>>
>>>>
>>>> diff --git a/server/openejb-client/src/main/java/org/apache/openejb/
>>> client/
>>>> MulticastConnectionFactory.java b/server/openejb-client/src/
>>>> main/java/org/apache/openejb/client/MulticastConnectionFactory.java
>>>> index 22f2f86a6a..eedb54840e 100644
>>>> --- a/server/openejb-client/src/main/java/org/apache/openejb/client/
>>>> MulticastConnectionFactory.java
>>>> +++ b/server/openejb-client/src/main/java/org/apache/openejb/client/
>>>> MulticastConnectionFactory.java
>>>> @@ -133,7 +133,7 @@ public class MulticastConnectionFactory implements
>>>> ConnectionFactory {
>>>> }
>>>>
>>>> public static Map<String, String> parseParamters(final URI uri)
>>>> throws URISyntaxException {
>>>> - return uri.getQuery() == null ? new HashMap<String,
>>> String>(0)
>>>> : parseQuery(stripPrefix(uri.getQuery(), "?"));
>>>> + return uri.getQuery() == null ? new HashMap<String,
>>> String>(0)
>>>> : parseQuery(stripPrefix(uri.getRawQuery(), "?"));
>>>> }
>>>>
>>>> public static String stripPrefix(final String value, final String
>>>> prefix) {
>>>>
>>>>
>>>>
>>>> Best regards,
>>>> Zac Bedell
>>>
>>>
>
Re: Incorrect URL decoding in
MulticastConnectionFactory$URI::parseParameters
Posted by Zachary Bedell <zb...@nycourts.gov>.
Not sure what you mean by misses a test? The code I modified is in the class MulticastConnectionFactory.java, but the current tests that exercise that code are in HttpConnectionTest.java. Seems like a bit of a weird cross-over, but a pre-existing condition. MulticastConnectionFactory contains a static subclass URIs with methods that are called by the HTTP related connection code.
I could move the test somewhere else, but there was already a test in HttpConnectionTest (httpBasicSpecificConfig) that tested the exact code I had to modify. The existing test didn't include an ampersand in the test data. I added a new test that does include an ampersand which fails without the modification to MulticastConnectionFactory. Both the existing & new tests pass with the change to MulticastConnectionFactory.
It does seem a little odd that the Http connection code ends up in a class with "multicast" in the name, but that's a much deeper change than I feel comfortable attempting at this point.
-Zac
> On Oct 2, 2017, at 13:15, Zachary Bedell <zb...@nycourts.gov> wrote:
>
> Sorry for the delay in submitting this. I've not had much luck building & passing the unit test suite for the full TomEE build. Currently failing with "javax.ws.rs.NotSupportedException: HTTP 415 Unsupported Media Type" in OpenEJB :: Examples :: REST XML JSON. Is there any documentation in terms of what's expected for the build environment for TomEE? Any Docker or other devops-ish canned configurations? I'm trying on a fresh Ubuntu 17.04 VM with JDK 8u144 and the master branch cloned.
>
> In any case, I got far enough for the "OpenEJB :: Server :: Client" tests to run and fail with my added unit test. Included patch fixes the problem and passes the new unit test. No additional failures with my patch beyond the Unsupported Media Type which failed for me with a fresh clone before changing anything.
>
> Sent via PR #104 on Github.
>
> Best regards,
> Zac Bedell
>
>> On Sep 20, 2017, at 15:19, Romain Manni-Bucau <rm...@gmail.com> wrote:
>>
>> then
>> https://github.com/apache/tomee/blob/master/server/openejb-client/src/test/java/org/apache/openejb/client/HttpConnectionTest.java#L171
>> was supposed to cover it but happy to get it enhanced ;)
>>
>>
>> Romain Manni-Bucau
>> @rmannibucau <https://twitter.com/rmannibucau> | Blog
>> <https://blog-rmannibucau.rhcloud.com> | Old Blog
>> <http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
>> <https://javaeefactory-rmannibucau.rhcloud.com>
>>
>> 2017-09-20 21:17 GMT+02:00 Zachary Bedell <zb...@nycourts.gov>:
>>
>>> I'll give it a shot. Running into some trouble getting stock tests to
>>> pass on Mac OS and separate problems behind a corporate proxy. Spinning a
>>> Linux VM on the open WiFi to try.
>>>
>>> FWIW, we're not using multicast for this. I just looks like
>>> HttpConnectionFactory calls the URI utility methods physically located in
>>> the static URI class on MulticastConnectionFactory. Otherwise this is
>>> https with sticky failover.
>>>
>>> -Zac
>>>
>>>> On Sep 20, 2017, at 12:50, Romain Manni-Bucau <rm...@gmail.com>
>>> wrote:
>>>>
>>>> Hi Zachary
>>>>
>>>>
>>>> We have some coverage for it in
>>>> https://github.com/apache/tomee/blob/master/server/
>>> openejb-client/src/test/java/org/apache/openejb/client/
>>> HttpConnectionTest.java
>>>>
>>>> Multicast not being used that often with http (more likely with ejbd) we
>>>> didnt test it, do you want to try to add a test and fix?
>>>>
>>>> Le 20 sept. 2017 18:28, "Zachary Bedell" <zb...@nycourts.gov> a écrit
>>> :
>>>>
>>>> Greetings all,
>>>>
>>>> Pretty sure I found a bug in the way
>>>> org.apache.openejb.client.MulticastConnectionFactory
>>>> decodes URL parameters. The final result of the issue is that if you use
>>>> HTTP basic authentication when calling ServerServlet and
>>>> openejb.ejbd.authenticate-with-request=true, you can't login with a
>>>> password that contains an ampersand character.
>>>>
>>>> The flow looks something like:
>>>>
>>>> 1) Create a new IntitialContext with PROVIDER_URL set to
>>>> failover:sticky+random:https://myserver/ejb/invoke?basic.
>>>> username=xyz&basic.password=pass%26word
>>>> a) /ejb/invoke is where I have org.apache.openejb.server.
>>> httpd.ServerServlet
>>>> mapped
>>>> b) web.xml on that mapping requires BASIC auth.
>>>> c) key part of URL is the literal password "pass&word" URL encoded with
>>>> ampersand -> %26
>>>>
>>>> 2) TomEE internals eventually end up at HttpConnectionFactory$
>>> HttpConnection's
>>>> constructor where line 76 calls:
>>>> params = MulticastConnectionFactory.URIs.parseParamters(uri);
>>>> By this time, various unwrapping has paired the URL down to:
>>>> https://myserver/ejb/invoke?basic.username=xyz&basic.
>>>> password=pass%26word
>>>>
>>>> 3) MulticastConnectionFactory...parseParameters, IE line 136:
>>>> return uri.getQuery() == null ? new HashMap<String, String>(0) :
>>>> parseQuery(stripPrefix(uri.getQuery(), "?"));
>>>>
>>>> That calls URI.getQuery() which decodes the URI's query string, then
>>> passes
>>>> that into parseQuery() which splits up the query parameters delimited by
>>>> ampersands. The call to URI.getQuery() is the problem. For the above
>>> URI,
>>>> the result is:
>>>> basic.username=xyz&basic.password=pass&word
>>>> The ampersand in the query parameter basic.password is decoded and
>>>> then indistinguishable from a query parameter separator. When passed
>>> into
>>>> parseQuery, the resulting value for basic.password is just "pass".
>>>>
>>>> Since MulticastConnectionFactory$URIs::parseQuery already calls
>>>> URLDecoder.decode() on both name and value pairs, the call in
>>>> parseParameters should be to URI::getRawQuery() instead of getQuery(). I
>>>> think there's also a possible double decoding issue here which could
>>>> corrupt certain values by decoding the value a second time.
>>>>
>>>> For the time being, I think I can work around this by passing the
>>>> authorization query parameter with the user:pass already base64 encoded.
>>>> Pretty sure this should be a complete & safe fix:
>>>>
>>>>
>>>> diff --git a/server/openejb-client/src/main/java/org/apache/openejb/
>>> client/
>>>> MulticastConnectionFactory.java b/server/openejb-client/src/
>>>> main/java/org/apache/openejb/client/MulticastConnectionFactory.java
>>>> index 22f2f86a6a..eedb54840e 100644
>>>> --- a/server/openejb-client/src/main/java/org/apache/openejb/client/
>>>> MulticastConnectionFactory.java
>>>> +++ b/server/openejb-client/src/main/java/org/apache/openejb/client/
>>>> MulticastConnectionFactory.java
>>>> @@ -133,7 +133,7 @@ public class MulticastConnectionFactory implements
>>>> ConnectionFactory {
>>>> }
>>>>
>>>> public static Map<String, String> parseParamters(final URI uri)
>>>> throws URISyntaxException {
>>>> - return uri.getQuery() == null ? new HashMap<String,
>>> String>(0)
>>>> : parseQuery(stripPrefix(uri.getQuery(), "?"));
>>>> + return uri.getQuery() == null ? new HashMap<String,
>>> String>(0)
>>>> : parseQuery(stripPrefix(uri.getRawQuery(), "?"));
>>>> }
>>>>
>>>> public static String stripPrefix(final String value, final String
>>>> prefix) {
>>>>
>>>>
>>>>
>>>> Best regards,
>>>> Zac Bedell
>>>
>>>
>
Re: Incorrect URL decoding in MulticastConnectionFactory$URI::parseParameters
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Hi Zachary,
environment is just a java one, nothing special (except on Mac where the
network config can need some tuning), the "NotSupportedException: HTTP 415
Unsupported Media Type" is weird, can be related last changes on master
with dependencies maybe, our CI is available at
https://ci.apache.org/builders/tomee-trunk-ubuntu
The PR misses a test for the code change (multicast) AFAIK otherwise it
looks a good start.
Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> | Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau>
2017-10-02 19:15 GMT+02:00 Zachary Bedell <zb...@nycourts.gov>:
> Sorry for the delay in submitting this. I've not had much luck building &
> passing the unit test suite for the full TomEE build. Currently failing
> with "javax.ws.rs.NotSupportedException: HTTP 415 Unsupported Media Type"
> in OpenEJB :: Examples :: REST XML JSON. Is there any documentation in
> terms of what's expected for the build environment for TomEE? Any Docker
> or other devops-ish canned configurations? I'm trying on a fresh Ubuntu
> 17.04 VM with JDK 8u144 and the master branch cloned.
>
> In any case, I got far enough for the "OpenEJB :: Server :: Client" tests
> to run and fail with my added unit test. Included patch fixes the problem
> and passes the new unit test. No additional failures with my patch beyond
> the Unsupported Media Type which failed for me with a fresh clone before
> changing anything.
>
> Sent via PR #104 on Github.
>
> Best regards,
> Zac Bedell
>
> > On Sep 20, 2017, at 15:19, Romain Manni-Bucau <rm...@gmail.com>
> wrote:
> >
> > then
> > https://github.com/apache/tomee/blob/master/server/
> openejb-client/src/test/java/org/apache/openejb/client/
> HttpConnectionTest.java#L171
> > was supposed to cover it but happy to get it enhanced ;)
> >
> >
> > Romain Manni-Bucau
> > @rmannibucau <https://twitter.com/rmannibucau> | Blog
> > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> > <http://rmannibucau.wordpress.com> | Github <https://github.com/
> rmannibucau> |
> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
> > <https://javaeefactory-rmannibucau.rhcloud.com>
> >
> > 2017-09-20 21:17 GMT+02:00 Zachary Bedell <zb...@nycourts.gov>:
> >
> >> I'll give it a shot. Running into some trouble getting stock tests to
> >> pass on Mac OS and separate problems behind a corporate proxy.
> Spinning a
> >> Linux VM on the open WiFi to try.
> >>
> >> FWIW, we're not using multicast for this. I just looks like
> >> HttpConnectionFactory calls the URI utility methods physically located
> in
> >> the static URI class on MulticastConnectionFactory. Otherwise this is
> >> https with sticky failover.
> >>
> >> -Zac
> >>
> >>> On Sep 20, 2017, at 12:50, Romain Manni-Bucau <rm...@gmail.com>
> >> wrote:
> >>>
> >>> Hi Zachary
> >>>
> >>>
> >>> We have some coverage for it in
> >>> https://github.com/apache/tomee/blob/master/server/
> >> openejb-client/src/test/java/org/apache/openejb/client/
> >> HttpConnectionTest.java
> >>>
> >>> Multicast not being used that often with http (more likely with ejbd)
> we
> >>> didnt test it, do you want to try to add a test and fix?
> >>>
> >>> Le 20 sept. 2017 18:28, "Zachary Bedell" <zb...@nycourts.gov> a
> écrit
> >> :
> >>>
> >>> Greetings all,
> >>>
> >>> Pretty sure I found a bug in the way
> >>> org.apache.openejb.client.MulticastConnectionFactory
> >>> decodes URL parameters. The final result of the issue is that if you
> use
> >>> HTTP basic authentication when calling ServerServlet and
> >>> openejb.ejbd.authenticate-with-request=true, you can't login with a
> >>> password that contains an ampersand character.
> >>>
> >>> The flow looks something like:
> >>>
> >>> 1) Create a new IntitialContext with PROVIDER_URL set to
> >>> failover:sticky+random:https://myserver/ejb/invoke?basic.
> >>> username=xyz&basic.password=pass%26word
> >>> a) /ejb/invoke is where I have org.apache.openejb.server.
> >> httpd.ServerServlet
> >>> mapped
> >>> b) web.xml on that mapping requires BASIC auth.
> >>> c) key part of URL is the literal password "pass&word" URL encoded with
> >>> ampersand -> %26
> >>>
> >>> 2) TomEE internals eventually end up at HttpConnectionFactory$
> >> HttpConnection's
> >>> constructor where line 76 calls:
> >>> params = MulticastConnectionFactory.URIs.parseParamters(uri);
> >>> By this time, various unwrapping has paired the URL down to:
> >>> https://myserver/ejb/invoke?basic.username=xyz&basic.
> >>> password=pass%26word
> >>>
> >>> 3) MulticastConnectionFactory...parseParameters, IE line 136:
> >>> return uri.getQuery() == null ? new HashMap<String, String>(0) :
> >>> parseQuery(stripPrefix(uri.getQuery(), "?"));
> >>>
> >>> That calls URI.getQuery() which decodes the URI's query string, then
> >> passes
> >>> that into parseQuery() which splits up the query parameters delimited
> by
> >>> ampersands. The call to URI.getQuery() is the problem. For the above
> >> URI,
> >>> the result is:
> >>> basic.username=xyz&basic.password=pass&word
> >>> The ampersand in the query parameter basic.password is decoded
> and
> >>> then indistinguishable from a query parameter separator. When passed
> >> into
> >>> parseQuery, the resulting value for basic.password is just "pass".
> >>>
> >>> Since MulticastConnectionFactory$URIs::parseQuery already calls
> >>> URLDecoder.decode() on both name and value pairs, the call in
> >>> parseParameters should be to URI::getRawQuery() instead of
> getQuery(). I
> >>> think there's also a possible double decoding issue here which could
> >>> corrupt certain values by decoding the value a second time.
> >>>
> >>> For the time being, I think I can work around this by passing the
> >>> authorization query parameter with the user:pass already base64
> encoded.
> >>> Pretty sure this should be a complete & safe fix:
> >>>
> >>>
> >>> diff --git a/server/openejb-client/src/main/java/org/apache/openejb/
> >> client/
> >>> MulticastConnectionFactory.java b/server/openejb-client/src/
> >>> main/java/org/apache/openejb/client/MulticastConnectionFactory.java
> >>> index 22f2f86a6a..eedb54840e 100644
> >>> --- a/server/openejb-client/src/main/java/org/apache/openejb/client/
> >>> MulticastConnectionFactory.java
> >>> +++ b/server/openejb-client/src/main/java/org/apache/openejb/client/
> >>> MulticastConnectionFactory.java
> >>> @@ -133,7 +133,7 @@ public class MulticastConnectionFactory implements
> >>> ConnectionFactory {
> >>> }
> >>>
> >>> public static Map<String, String> parseParamters(final URI uri)
> >>> throws URISyntaxException {
> >>> - return uri.getQuery() == null ? new HashMap<String,
> >> String>(0)
> >>> : parseQuery(stripPrefix(uri.getQuery(), "?"));
> >>> + return uri.getQuery() == null ? new HashMap<String,
> >> String>(0)
> >>> : parseQuery(stripPrefix(uri.getRawQuery(), "?"));
> >>> }
> >>>
> >>> public static String stripPrefix(final String value, final
> String
> >>> prefix) {
> >>>
> >>>
> >>>
> >>> Best regards,
> >>> Zac Bedell
> >>
> >>
>
>