On Tue, Dec 20, 2011 at 9:52 AM, Andrea Aime
<[email protected]>wrote:

> On Tue, Dec 20, 2011 at 9:39 AM, Gabriel Roldan <[email protected]>wrote:
>
>> Attached a patch to the issue. Testing, review and comments welcomed.
>>
>> I tested it with geoserver and seems to work fine. Yet I'm not sure how
>> to write a unit test for that.
>>
>
> Yeah, me neither. I've seen people setup a temporary http server using
> Grizzlie to run tests
> (I guess the same could be done with an embedded Jetty),
> that might work, but it's quite the heavy handed way to make a build test.
>
> I'll setup a proxy using Burp (http://portswigger.net/burp/proxy.html),
> apply the patch, and manually
> check the requests actually go through it
>

Hey there,
so I've tried out the patch, it seems to be working fine, thanks for
putting it togheter on
a such short notice.

As said, I agree unit testing it might be a daunting task, even setting up
a temporary mock wms server on a real port one has to guess a free port and
so on... annoying, it may be seen as a online test where the fixture
declares a known open port or it could try several ports until it finds one
open... but I'm not so worried about it, and it's a lot of work.

There are two things that might still be considered regressions in there:
- are we sure the password has to be specified along with the username?
It's a bit of a stretch, but why don't we let it go and have the proxy
decide if the password is required or not?
- the http.nonProxyHosts is not supported. In simple setups it's not
required, and I agree it might not be
  common in usage, but consider a case in which you have a server that's
cascading two other servers,
  one that is local (a legacy in house WMS server that GeoServer is acting
as a front-end for) and
  another is remote, with the proxy you can access the remote but not the
local, and vice versa.

It might be a good idea to have some sort of configuration allowing the
usage of the java based http client instead so that also this case is
covered, or we can try to match the nonProxyHost list
(which would require some parsing and some regex usage, as the doc say "The
value can be a list of hosts, each seperated by a |, and in addition a
wildcard character (*) can be used for matching. For example:
-Dhttp.nonProxyHosts="*.foo.com|localhost".")

Cheers
Andrea


-- 
-------------------------------------------------------
Ing. Andrea Aime
GeoSolutions S.A.S.
Tech lead

Via Poggio alle Viti 1187
55054  Massarosa (LU)
Italy

phone: +39 0584 962313
fax:      +39 0584 962313
mob:    +39 339 8844549

http://www.geo-solutions.it
http://geo-solutions.blogspot.com/
http://www.youtube.com/user/GeoSolutionsIT
http://www.linkedin.com/in/andreaaime
http://twitter.com/geowolf

Please take note that GeoSolutions will be closed for Christmas holidays
from 27/12 to 30/12

-------------------------------------------------------
------------------------------------------------------------------------------
Write once. Port to many.
Get the SDK and tools to simplify cross-platform app development. Create 
new or port existing apps to sell to consumers worldwide. Explore the 
Intel AppUpSM program developer opportunity. appdeveloper.intel.com/join
http://p.sf.net/sfu/intel-appdev
_______________________________________________
Geotools-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Reply via email to