On 15/05/13 11:04, Alan Bateman wrote:
On 14/05/2013 12:54, Michael McMahon wrote:
I have updated the webrev for this at:
http://cr.openjdk.java.net/~michaelm/8010464/webrev.2/
I took a pass over the updated webrev and it mostly looks good to me.
In HttpURLConnection then I wonder if it would be better if
getInputStream, getOutputStream and followRedirect set the connecting
flag rather than URLtoSocketPermission. The side effect on the
protocol handler state is a bit surprising in this method.
Alternatively then maybe the method needs a better name.
Yes, that did looks a little incongruous. So, I think moving it to
getInputStream, getOutputStream
and connect() would make sense.
On MessageHeader then getHeaderNamesInList could use
java.util.StringJoiner to avoid rolling your own.
I can see the benefit of using StringJoiner (and a lambda) if I am
starting off from a Collection
and this is something that only struck me when looking at this. I was
surprised
to see that Arrays.asList() doesn't have a variant that limits the number of
elements coming from the array. So, I can't use it here. Maybe this is
something
we could look at again later, with the other changes we're contemplating?
Are you planning to add @bug to the tests?
Yes, will do that.
I wonder if about dependency on SimpleSSLContext in the test as I
don't think that test was originally intended to be a "test library".
SimpleSSLContext isn't a test. All it does is provide the glue to build
an SSLContext.
I've used it as a kind of library before.
I'll post one more webrev, and push it soon afterwards as I'd like to
make the
code freeze today.
Thanks
Michael
