hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject 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 15:26:37 GMT
Again, looks generally OK.

Some comments below.

On 6 August 2014 10:29,  <olegk@apache.org> wrote:
> Author: olegk
> Date: Wed Aug  6 09:29:39 2014
> New Revision: 1616136
>
> URL: http://svn.apache.org/r1616136
> Log:
> Custom distingushed name parser
>
> Added:
>     httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/conn/ssl/DistinguishedNameParser.java
  (with props)
>     httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/conn/ssl/TestDistinguishedNameParser.java
  (with props)
>
> Added: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/conn/ssl/DistinguishedNameParser.java
> URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/conn/ssl/DistinguishedNameParser.java?rev=1616136&view=auto
> ==============================================================================
> --- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/conn/ssl/DistinguishedNameParser.java
(added)
> +++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/conn/ssl/DistinguishedNameParser.java
Wed Aug  6 09:29:39 2014
> @@ -0,0 +1,132 @@
> +/*
> + * ====================================================================
> + * Licensed to the Apache Software Foundation (ASF) under one
> + * or more contributor license agreements.  See the NOTICE file
> + * distributed with this work for additional information
> + * regarding copyright ownership.  The ASF licenses this file
> + * to you under the Apache License, Version 2.0 (the
> + * "License"); you may not use this file except in compliance
> + * with the License.  You may obtain a copy of the License at
> + *
> + *   http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing,
> + * software distributed under the License is distributed on an
> + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
> + * KIND, either express or implied.  See the License for the
> + * specific language governing permissions and limitations
> + * under the License.
> + * ====================================================================
> + *
> + * This software consists of voluntary contributions made by many
> + * individuals on behalf of the Apache Software Foundation.  For more
> + * information on the Apache Software Foundation, please see
> + * <http://www.apache.org/>.
> + *
> + */
> +
> +package org.apache.http.conn.ssl;
> +
> +import java.util.ArrayList;
> +import java.util.BitSet;
> +import java.util.List;
> +
> +import org.apache.http.NameValuePair;
> +import org.apache.http.annotation.Immutable;
> +import org.apache.http.message.BasicNameValuePair;
> +import org.apache.http.message.ParserCursor;
> +import org.apache.http.util.CharArrayBuffer;
> +
> +@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.

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

> +
> +    private final TokenParser tokenParser;
> +
> +    DistinguishedNameParser() {
> +        this.tokenParser = new InternalTokenParser();
> +    }
> +
> +    String parseToken(final CharArrayBuffer buf, final ParserCursor cursor, final BitSet
delimiters) {
> +        return tokenParser.parseToken(buf, cursor, delimiters);
> +    }
> +
> +    String parseValue(final CharArrayBuffer buf, final ParserCursor cursor, final BitSet
delimiters) {
> +        return tokenParser.parseValue(buf, cursor, delimiters);
> +    }
> +
> +    NameValuePair parseParameter(final CharArrayBuffer buf, final ParserCursor cursor)
{
> +        final String name = parseToken(buf, cursor, EQUAL_OR_COMMA_OR_PLUS);
> +        if (cursor.atEnd()) {
> +            return new BasicNameValuePair(name, null);
> +        }
> +        final int delim = buf.charAt(cursor.getPos());
> +        cursor.updatePos(cursor.getPos() + 1);
> +        if (delim == ',') {
> +            return new BasicNameValuePair(name, null);
> +        }
> +        final String value = parseValue(buf, cursor, COMMA_OR_PLUS);
> +        if (!cursor.atEnd()) {
> +            cursor.updatePos(cursor.getPos() + 1);
> +        }
> +        return new BasicNameValuePair(name, value);
> +    }
> +
> +    public List<NameValuePair> parse(final CharArrayBuffer buf, final ParserCursor
cursor) {
> +        final List<NameValuePair> params = new ArrayList<NameValuePair>();
> +        tokenParser.skipWhiteSpace(buf, cursor);
> +        while (!cursor.atEnd()) {
> +            final NameValuePair param = parseParameter(buf, cursor);
> +            params.add(param);
> +        }
> +        return params;
> +    }
> +
> +    public List<NameValuePair> parse(final String s) {
> +        if (s == null) {
> +            return null;
> +        }
> +        final CharArrayBuffer buffer = new CharArrayBuffer(s.length());
> +        buffer.append(s);
> +        final ParserCursor cursor = new ParserCursor(0, s.length());
> +        return parse(buffer, cursor);
> +    }
> +
> +    static class InternalTokenParser extends TokenParser {
> +

Do we really need this?
Perhaps escape processing should be merged into the super-class.

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

> +        @Override
> +        public void copyUnquotedContent(
> +                final CharArrayBuffer buf,
> +                final ParserCursor cursor,
> +                final BitSet delimiters,
> +                final StringBuilder dst) {
> +            int pos = cursor.getPos();
> +            final int indexFrom = cursor.getPos();
> +            final int indexTo = cursor.getUpperBound();
> +            boolean escaped = false;
> +            for (int i = indexFrom; i < indexTo; i++, pos++) {

A bit confusing to increment pos in the for loop here whereas the
superclass increments it in the loop body.
Assuming the methods are not merged, should use the same approach for both.

> +                final char current = buf.charAt(i);
> +                if (escaped) {
> +                    dst.append(current);
> +                    escaped = false;
> +                } else {
> +                    if ((delimiters != null && delimiters.get(current))
> +                            || TokenParser.isWhitespace(current) || current == '\"')
{

Needs constant

> +                        break;
> +                    } else if (current == '\\') {

Needs constant

> +                        escaped = true;
> +                    } else {
> +                        dst.append(current);
> +                    }
> +                }
> +            }
> +            cursor.updatePos(pos);
> +        }
> +    }
> +
> +}
> +
>
> Propchange: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/conn/ssl/DistinguishedNameParser.java
> ------------------------------------------------------------------------------
>     svn:eol-style = native
>
> Propchange: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/conn/ssl/DistinguishedNameParser.java
> ------------------------------------------------------------------------------
>     svn:keywords = Date Revision
>
> Propchange: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/conn/ssl/DistinguishedNameParser.java
> ------------------------------------------------------------------------------
>     svn:mime-type = text/plain
>
> Added: httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/conn/ssl/TestDistinguishedNameParser.java
> URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/conn/ssl/TestDistinguishedNameParser.java?rev=1616136&view=auto
> ==============================================================================
> --- httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/conn/ssl/TestDistinguishedNameParser.java
(added)
> +++ httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/conn/ssl/TestDistinguishedNameParser.java
Wed Aug  6 09:29:39 2014
> @@ -0,0 +1,234 @@
> +/*
> + * ====================================================================
> + * Licensed to the Apache Software Foundation (ASF) under one
> + * or more contributor license agreements.  See the NOTICE file
> + * distributed with this work for additional information
> + * regarding copyright ownership.  The ASF licenses this file
> + * to you under the Apache License, Version 2.0 (the
> + * "License"); you may not use this file except in compliance
> + * with the License.  You may obtain a copy of the License at
> + *
> + *   http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing,
> + * software distributed under the License is distributed on an
> + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
> + * KIND, either express or implied.  See the License for the
> + * specific language governing permissions and limitations
> + * under the License.
> + * ====================================================================
> + *
> + * This software consists of voluntary contributions made by many
> + * individuals on behalf of the Apache Software Foundation.  For more
> + * information on the Apache Software Foundation, please see
> + * <http://www.apache.org/>.
> + *
> + */
> +
> +package org.apache.http.conn.ssl;
> +
> +import java.util.Arrays;
> +
> +import org.apache.http.NameValuePair;
> +import org.apache.http.message.BasicNameValuePair;
> +import org.apache.http.message.ParserCursor;
> +import org.apache.http.util.CharArrayBuffer;
> +import org.junit.Assert;
> +import org.junit.Before;
> +import org.junit.Test;
> +
> +public class TestDistinguishedNameParser {
> +
> +    private DistinguishedNameParser parser;
> +
> +    @Before
> +    public void setUp() throws Exception {
> +        parser = new DistinguishedNameParser();
> +    }
> +
> +    private static CharArrayBuffer createBuffer(final String value) {
> +        if (value == null) {
> +            return null;
> +        }
> +        final CharArrayBuffer buffer = new CharArrayBuffer(value.length());
> +        buffer.append(value);
> +        return buffer;
> +    }
> +
> +    @Test
> +    public void testTokenParsingMixedValuesAndQuotedValues() throws Exception {
> +        final String s = "  stuff and    \" some more \"   \"stuff  ;";
> +        final CharArrayBuffer raw = createBuffer(s);
> +        final ParserCursor cursor = new ParserCursor(0, s.length());
> +        final String result = parser.parseValue(raw, cursor, TokenParser.INIT_BITSET(';'));
> +        Assert.assertEquals("stuff and  some more  stuff  ;", result);
> +    }
> +
> +    @Test
> +    public void testTokenParsingMixedValuesAndQuotedValues2() throws Exception {
> +        final String s = "stuff\"more\"stuff;";
> +        final CharArrayBuffer raw = createBuffer(s);
> +        final ParserCursor cursor = new ParserCursor(0, s.length());
> +        final String result = parser.parseValue(raw, cursor, TokenParser.INIT_BITSET(';'));
> +        Assert.assertEquals("stuffmorestuff", result);
> +    }
> +
> +    @Test
> +    public void testTokenParsingEscapedQuotes() throws Exception {
> +        final String s = "stuff\"\\\"more\\\"\"stuff;";
> +        final CharArrayBuffer raw = createBuffer(s);
> +        final ParserCursor cursor = new ParserCursor(0, s.length());
> +        final String result = parser.parseValue(raw, cursor, TokenParser.INIT_BITSET(';'));
> +        Assert.assertEquals("stuff\"more\"stuff", result);
> +    }
> +
> +    @Test
> +    public void testTokenParsingEscapedDelimiter() throws Exception {
> +        final String s = "stuff\"\\\"more\\\";\"stuff;";
> +        final CharArrayBuffer raw = createBuffer(s);
> +        final ParserCursor cursor = new ParserCursor(0, s.length());
> +        final String result = parser.parseValue(raw, cursor, TokenParser.INIT_BITSET(';'));
> +        Assert.assertEquals("stuff\"more\";stuff", result);
> +    }
> +
> +    @Test
> +    public void testTokenParsingEscapedSlash() throws Exception {
> +        final String s = "stuff\"\\\"more\\\";\\\\\"stuff;";
> +        final CharArrayBuffer raw = createBuffer(s);
> +        final ParserCursor cursor = new ParserCursor(0, s.length());
> +        final String result = parser.parseValue(raw, cursor, TokenParser.INIT_BITSET(';'));
> +        Assert.assertEquals("stuff\"more\";\\stuff", result);
> +    }
> +
> +    @Test
> +    public void testTokenParsingSlashOutsideQuotes() throws Exception {
> +        final String s = "stuff\\; more stuff;";
> +        final CharArrayBuffer raw = createBuffer(s);
> +        final ParserCursor cursor = new ParserCursor(0, s.length());
> +        final String result = parser.parseValue(raw, cursor, TokenParser.INIT_BITSET(';'));
> +        Assert.assertEquals("stuff; more stuff", result);
> +    }
> +
> +    @Test
> +    public void testBasicParameterParsing() throws Exception {
> +        final String s = "cn=blah,";
> +        final CharArrayBuffer raw = createBuffer(s);
> +        final ParserCursor cursor = new ParserCursor(0, s.length());
> +        final NameValuePair result = parser.parseParameter(raw, cursor);
> +        Assert.assertNotNull("cn", result.getName());
> +        Assert.assertEquals("blah", result.getValue());
> +    }
> +
> +    @Test
> +    public void testParameterParsingBlanks() throws Exception {
> +        final String s = "  cn  =     blah    ,stuff";
> +        final CharArrayBuffer raw = createBuffer(s);
> +        final ParserCursor cursor = new ParserCursor(0, s.length());
> +        final NameValuePair result = parser.parseParameter(raw, cursor);
> +        Assert.assertNotNull("cn", result.getName());
> +        Assert.assertEquals("blah", result.getValue());
> +        Assert.assertEquals('s', raw.charAt(cursor.getPos()));
> +    }
> +
> +    @Test
> +    public void testParameterParsingEmptyValue() throws Exception {
> +        final String s = "  cn  =    ,stuff ";
> +        final CharArrayBuffer raw = createBuffer(s);
> +        final ParserCursor cursor = new ParserCursor(0, s.length());
> +        final NameValuePair result = parser.parseParameter(raw, cursor);
> +        Assert.assertNotNull("cn", result.getName());
> +        Assert.assertEquals("", result.getValue());
> +        Assert.assertEquals('s', raw.charAt(cursor.getPos()));
> +    }
> +
> +    @Test
> +    public void testParameterParsingNullValue() throws Exception {
> +        final String s = "  cn     ";
> +        final CharArrayBuffer raw = createBuffer(s);
> +        final ParserCursor cursor = new ParserCursor(0, s.length());
> +        final NameValuePair result = parser.parseParameter(raw, cursor);
> +        Assert.assertNotNull("cn", result.getName());
> +        Assert.assertEquals(null, result.getValue());
> +        Assert.assertTrue(cursor.atEnd());
> +    }
> +
> +    @Test
> +    public void testParameterParsingQuotedValue() throws Exception {
> +        final String s = "cn = \"blah, blah\"  ,stuff";
> +        final CharArrayBuffer raw = createBuffer(s);
> +        final ParserCursor cursor = new ParserCursor(0, s.length());
> +        final NameValuePair result = parser.parseParameter(raw, cursor);
> +        Assert.assertNotNull("cn", result.getName());
> +        Assert.assertEquals("blah, blah", result.getValue());
> +        Assert.assertEquals('s', raw.charAt(cursor.getPos()));
> +    }
> +
> +    @Test
> +    public void testParameterParsingQuotedValueWithEscapedQuotes() throws Exception
{
> +        final String s = "cn = \"blah, blah, \\\"yada, yada\\\"\"  ,stuff";
> +        final CharArrayBuffer raw = createBuffer(s);
> +        final ParserCursor cursor = new ParserCursor(0, s.length());
> +        final NameValuePair result = parser.parseParameter(raw, cursor);
> +        Assert.assertNotNull("cn", result.getName());
> +        Assert.assertEquals("blah, blah, \"yada, yada\"", result.getValue());
> +        Assert.assertEquals('s', raw.charAt(cursor.getPos()));
> +    }
> +
> +    @Test
> +    public void testParameterParsingValueWithEscapedDelimiter() throws Exception {
> +        final String s = "cn = blah\\, blah\\, blah  ,stuff";
> +        final CharArrayBuffer raw = createBuffer(s);
> +        final ParserCursor cursor = new ParserCursor(0, s.length());
> +        final NameValuePair result = parser.parseParameter(raw, cursor);
> +        Assert.assertNotNull("cn", result.getName());
> +        Assert.assertEquals("blah, blah, blah", result.getValue());
> +        Assert.assertEquals('s', raw.charAt(cursor.getPos()));
> +    }
> +
> +    @Test
> +    public void testDNParsing() throws Exception {
> +        Assert.assertEquals(Arrays.asList(
> +                        new BasicNameValuePair("cn", "blah"),
> +                        new BasicNameValuePair("cn", "yada"),
> +                        new BasicNameValuePair("cn", "booh")),
> +                parser.parse("cn=blah, cn=yada, cn=booh"));
> +        Assert.assertEquals(Arrays.asList(
> +                        new BasicNameValuePair("c", "pampa"),
> +                        new BasicNameValuePair("cn", "blah"),
> +                        new BasicNameValuePair("ou", "blah"),
> +                        new BasicNameValuePair("o", "blah")),
> +                parser.parse("c = pampa ,  cn  =    blah    , ou = blah , o = blah"));
> +        Assert.assertEquals(Arrays.asList(
> +                        new BasicNameValuePair("cn", "blah"),
> +                        new BasicNameValuePair("ou", "blah"),
> +                        new BasicNameValuePair("o", "blah")),
> +                parser.parse("cn=\"blah\", ou=blah, o=blah"));
> +        Assert.assertEquals(Arrays.asList(
> +                        new BasicNameValuePair("cn", "blah  blah"),
> +                        new BasicNameValuePair("ou", "blah"),
> +                        new BasicNameValuePair("o", "blah")),
> +                parser.parse("cn=\"blah  blah\", ou=blah, o=blah"));
> +        Assert.assertEquals(Arrays.asList(
> +                        new BasicNameValuePair("cn", "blah, blah"),
> +                        new BasicNameValuePair("ou", "blah"),
> +                        new BasicNameValuePair("o", "blah")),
> +                parser.parse("cn=\"blah, blah\", ou=blah, o=blah"));
> +        Assert.assertEquals(Arrays.asList(
> +                        new BasicNameValuePair("cn", "blah, blah"),
> +                        new BasicNameValuePair("ou", "blah"),
> +                        new BasicNameValuePair("o", "blah")),
> +                parser.parse("cn=blah\\, blah, ou=blah, o=blah"));
> +        Assert.assertEquals(Arrays.asList(
> +                        new BasicNameValuePair("c", "cn=uuh"),
> +                        new BasicNameValuePair("cn", "blah"),
> +                        new BasicNameValuePair("ou", "blah"),
> +                        new BasicNameValuePair("o", "blah")),
> +                parser.parse("c = cn=uuh, cn=blah, ou=blah, o=blah"));
> +        Assert.assertEquals(Arrays.asList(
> +                        new BasicNameValuePair("cn", ""),
> +                        new BasicNameValuePair("ou", "blah"),
> +                        new BasicNameValuePair("o", "blah")),
> +                parser.parse("cn=   , ou=blah, o=blah"));
> +    }
> +
> +}
>
> Propchange: httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/conn/ssl/TestDistinguishedNameParser.java
> ------------------------------------------------------------------------------
>     svn:eol-style = native
>
> Propchange: httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/conn/ssl/TestDistinguishedNameParser.java
> ------------------------------------------------------------------------------
>     svn:keywords = Date Revision
>
> Propchange: httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/conn/ssl/TestDistinguishedNameParser.java
> ------------------------------------------------------------------------------
>     svn:mime-type = text/plain
>
>

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


Mime
View raw message