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 <zbed...@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 <rmannibu...@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 <zbed...@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 <rmannibu...@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" <zbed...@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 >>> >>> >
signature.asc
Description: Message signed with OpenPGP
