hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Oleg Kalnichevski <ol...@apache.org>
Subject Re: Fwd: svn commit: r1616136 - in /httpcomponents/httpclient/trunk/httpclient/src: main/java/org/apache/http/conn/ssl/DistinguishedNameParser.java test/java/org/apache/http/conn/ssl/TestDistinguishedNameParser.java
Date Thu, 07 Aug 2014 18:27:01 GMT
On 07/08/14 17:26 , sebb wrote:
> Again, looks generally OK.
>
> Some comments below.
>
> On 6 August 2014 10:29,  <olegk@apache.org> wrote:
>> +@Immutable
>
> Are we sure it's immutable?
>
>> +final class DistinguishedNameParser {
>> +
>> +    public final static DistinguishedNameParser INSTANCE = new DistinguishedNameParser();
>> +
>> +    private static final BitSet EQUAL_OR_COMMA_OR_PLUS      = TokenParser.INIT_BITSET('=',
',', '+');
>> +    private static final BitSet COMMA_OR_PLUS               = TokenParser.INIT_BITSET(',',
'+');
>
> BitSet is not immutable, so using a shared static field relies on the
> code never calling any of the mutation methods after creation.
>

True, but these static variables are private and their state does not 
mutate post construction.

> Perhaps consider using a simple delegation wrapper to provide only read access?
> Otherwise we need to document the non-mutability assumption.
>

We could but given those static variables are private I see no real need 
to do so.

>> +
>> +    static class InternalTokenParser extends TokenParser {
>> +
>
> Do we really need this?
> Perhaps escape processing should be merged into the super-class.
>

Yes, we do. HTTP spec states that escaped chars are valid only when 
enclosed in double quotes. It is quite different from RFC 4514 and 
earlier RFCs superseded by it.


> But if it is kept, it needs to have Javadoc.
>

It is a package private class. Does this make any sense?

Oleg


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


Mime
View raw message