michael-o commented on a change in pull request #230:
URL: 
https://github.com/apache/httpcomponents-client/pull/230#discussion_r447597392



##########
File path: 
httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/AuthChallengeParser.java
##########
@@ -86,52 +91,114 @@ NameValuePair parseTokenOrParameter(final CharSequence 
buffer, final ParserCurso
      */
     public List<AuthChallenge> parse(
             final ChallengeType challengeType, final CharSequence buffer, 
final ParserCursor cursor) throws ParseException {
-
-        final List<AuthChallenge> list = new ArrayList<>();
-        String schemeName = null;
-        final List<NameValuePair> params = new ArrayList<>();
-        while (!cursor.atEnd()) {
-            final NameValuePair tokenOrParameter = 
parseTokenOrParameter(buffer, cursor);
-            if (tokenOrParameter.getValue() == null && !cursor.atEnd() && 
buffer.charAt(cursor.getPos()) != COMMA_CHAR) {
-                if (schemeName != null) {
-                    if (params.isEmpty()) {
-                        throw new ParseException("Malformed auth challenge");
-                    }
-                    list.add(createAuthChallenge(challengeType, schemeName, 
params));
+        tokenParser.skipWhiteSpace(buffer, cursor);
+        if (cursor.atEnd()) {
+            throw new ParseException("Malformed auth challenge");
+        }
+        final List<ChallengeInt> internalChallenges = new ArrayList<>();
+        final String schemeName = tokenParser.parseToken(buffer, cursor, 
SPACE);
+        if (TextUtils.isBlank(schemeName)) {
+            throw new ParseException("Malformed auth challenge");
+        }
+        ChallengeInt current = new ChallengeInt(schemeName);
+        while (current != null) {
+            internalChallenges.add(current);
+            current = parseChallenge(buffer, cursor, current);
+        }
+        final List<AuthChallenge> challenges = new 
ArrayList<>(internalChallenges.size());
+        for (final ChallengeInt internal : internalChallenges) {
+            final List<NameValuePair> params = internal.params;
+            String token68 = null;
+            if (params.size() == 1) {
+                final NameValuePair param = params.get(0);
+                if (param.getValue() == null) {
+                    token68 = param.getName();
                     params.clear();
                 }
-                schemeName = tokenOrParameter.getName();
-            } else {
-                params.add(tokenOrParameter);
-                if (!cursor.atEnd() && buffer.charAt(cursor.getPos()) != 
COMMA_CHAR) {
-                    schemeName = null;
-                }
-            }
-            if (!cursor.atEnd() && buffer.charAt(cursor.getPos()) == 
COMMA_CHAR) {
-                cursor.updatePos(cursor.getPos() + 1);
             }
+            challenges.add(
+                    new AuthChallenge(challengeType, internal.schemeName, 
token68, !params.isEmpty() ? params : null));
         }
-        list.add(createAuthChallenge(challengeType, schemeName, params));
-        return list;
+        return challenges;
     }
 
-    private static AuthChallenge createAuthChallenge(final ChallengeType 
challengeType, final String schemeName, final List<NameValuePair> params) 
throws ParseException {
-        if (schemeName != null) {
-            if (params.size() == 1) {
-                final NameValuePair nvp = params.get(0);
-                if (nvp.getValue() == null) {
-                    return new AuthChallenge(challengeType, schemeName, 
nvp.getName(), null);
+    ChallengeInt parseChallenge(
+            final CharSequence buffer,
+            final ParserCursor cursor,
+            final ChallengeInt currentChallenge) throws ParseException {
+        for (;;) {
+            tokenParser.skipWhiteSpace(buffer, cursor);
+            if (cursor.atEnd()) {
+                return null;
+            }
+            final String token = parseToken(buffer, cursor);
+            if (TextUtils.isBlank(token)) {
+                throw new ParseException("Malformed auth challenge");
+            }
+            tokenParser.skipWhiteSpace(buffer, cursor);
+
+            // it gets really messy here
+            if (cursor.atEnd()) {
+                // at the end of the header
+                currentChallenge.params.add(new BasicNameValuePair(token, 
null));
+            } else {
+                char ch = buffer.charAt(cursor.getPos());
+                if (ch == EQUAL_CHAR) {
+                    cursor.updatePos(cursor.getPos() + 1);
+                    final String value = tokenParser.parseValue(buffer, 
cursor, DELIMITER);
+                    tokenParser.skipWhiteSpace(buffer, cursor);
+                    if (!cursor.atEnd()) {
+                        ch = buffer.charAt(cursor.getPos());
+                        if (ch == COMMA_CHAR) {
+                            cursor.updatePos(cursor.getPos() + 1);
+                        }
+                    }
+                    currentChallenge.params.add(new BasicNameValuePair(token, 
value));
+                } else if (ch == COMMA_CHAR) {
+                    cursor.updatePos(cursor.getPos() + 1);
+                    currentChallenge.params.add(new BasicNameValuePair(token, 
null));
+                } else {
+                    // the token represents new challenge
+                    if (currentChallenge.params.isEmpty()) {
+                        throw new ParseException("Malformed auth challenge");
+                    }
+                    return new ChallengeInt(token);
                 }
             }
-            return new AuthChallenge(challengeType, schemeName, null, 
params.size() > 0 ? params : null);
         }
-        if (params.size() == 1) {
-            final NameValuePair nvp = params.get(0);
-            if (nvp.getValue() == null) {
-                return new AuthChallenge(challengeType, nvp.getName(), null, 
null);
+    }
+
+    String parseToken(final CharSequence buf, final ParserCursor cursor) {
+        final StringBuilder dst = new StringBuilder();
+        while (!cursor.atEnd()) {
+            int pos = cursor.getPos();
+            char current = buf.charAt(pos);
+            if (TERMINATORS.get(current)) {
+                // Here it gets really ugly
+                if (current == EQUAL_CHAR) {
+                    // it can be a start of a parameter value or token86 
padding

Review comment:
       digit twister => 86 => 68

##########
File path: 
httpclient5/src/test/java/org/apache/hc/client5/http/impl/auth/TestAuthChallengeParser.java
##########
@@ -321,12 +323,26 @@ public void testParseNTLMAuthChallenge() throws Exception 
{
         Assert.assertEquals(null, challenge1.getValue());
     }
 
-    private static void assertNameValuePair (
-            final NameValuePair expected,
-            final NameValuePair result) {
-        Assert.assertNotNull(result);
-        Assert.assertEquals(expected.getName(), result.getName());
-        Assert.assertEquals(expected.getValue(), result.getValue());
+    @Test
+    public void testParseParameterAndToken68AuthChallengeMix() throws 
Exception {
+        final CharArrayBuffer buffer = new CharArrayBuffer(64);
+        buffer.append("scheme1 aaaa  , scheme2 aaaa==,  scheme3 aaaa=aaaa, 
scheme4 aaaa=");
+        final ParserCursor cursor = new ParserCursor(0, buffer.length());
+        final List<AuthChallenge> challenges = 
parser.parse(ChallengeType.TARGET, buffer, cursor);
+        Assert.assertNotNull(challenges);
+        Assert.assertEquals(4, challenges.size());
+        final AuthChallenge challenge1 = challenges.get(0);
+        Assert.assertThat(challenge1.getSchemeName(), 
CoreMatchers.equalTo("scheme1"));
+        Assert.assertThat(challenge1.getValue(), CoreMatchers.equalTo("aaaa"));
+        final AuthChallenge challenge2 = challenges.get(1);
+        Assert.assertThat(challenge2.getSchemeName(), 
CoreMatchers.equalTo("scheme2"));
+        Assert.assertThat(challenge2.getValue(), 
CoreMatchers.equalTo("aaaa=="));
+        final AuthChallenge challenge3 = challenges.get(2);
+        Assert.assertThat(challenge3.getSchemeName(), 
CoreMatchers.equalTo("scheme3"));
+        Assert.assertThat(challenge3.getValue(), CoreMatchers.nullValue());

Review comment:
       I don't understand this. Param is there no param map with the key 
`aaaa`?  I this supported to match token68 only?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to