Correctly handle a digest authorization header when one of the hex field values ends the header with in an invalid character.
Expand the test cases to improve coverage including this issue.
git-svn-id: https://svn.apache.org/repos/asf/tomcat/tc8.0.x/trunk@1832548 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/java/org/apache/tomcat/util/http/parser/HttpParser.java b/java/org/apache/tomcat/util/http/parser/HttpParser.java
index 64497a5..9618a0c 100644
--- a/java/org/apache/tomcat/util/http/parser/HttpParser.java
+++ b/java/org/apache/tomcat/util/http/parser/HttpParser.java
@@ -337,38 +337,36 @@
}
- // Skip any LWS and return the next char
- static int skipLws(Reader input, boolean withReset) throws IOException {
+ // Skip any LWS and position to read the next character. The next character
+ // is returned as being able to 'peek()' it allows a small optimisation in
+ // some cases.
+ static int skipLws(Reader input) throws IOException {
- if (withReset) {
- input.mark(1);
- }
+ input.mark(1);
int c = input.read();
while (c == 32 || c == 9 || c == 10 || c == 13) {
- if (withReset) {
- input.mark(1);
- }
+ input.mark(1);
c = input.read();
}
- if (withReset) {
- input.reset();
- }
+ input.reset();
return c;
}
static SkipResult skipConstant(Reader input, String constant) throws IOException {
int len = constant.length();
- int c = skipLws(input, false);
+ skipLws(input);
+ input.mark(len);
+ int c = input.read();
for (int i = 0; i < len; i++) {
if (i == 0 && c == -1) {
return SkipResult.EOF;
}
if (c != constant.charAt(i)) {
- input.skip(-(i + 1));
+ input.reset();
return SkipResult.NOT_FOUND;
}
if (i != (len - 1)) {
@@ -386,14 +384,18 @@
static String readToken(Reader input) throws IOException {
StringBuilder result = new StringBuilder();
- int c = skipLws(input, false);
+ skipLws(input);
+ input.mark(1);
+ int c = input.read();
while (c != -1 && isToken(c)) {
result.append((char) c);
+ input.mark(1);
c = input.read();
}
- // Skip back so non-token character is available for next read
- input.skip(-1);
+ // Use mark(1)/reset() rather than skip(-1) since skip() is a NOP
+ // once the end of the String has been reached.
+ input.reset();
if (c != -1 && result.length() == 0) {
return null;
@@ -409,7 +411,8 @@
*/
static String readQuotedString(Reader input, boolean returnQuoted) throws IOException {
- int c = skipLws(input, false);
+ skipLws(input);
+ int c = input.read();
if (c != '"') {
return null;
@@ -445,8 +448,8 @@
static String readTokenOrQuotedString(Reader input, boolean returnQuoted)
throws IOException {
- // Go back so first non-LWS character is available to be read again
- int c = skipLws(input, true);
+ // Peek at next character to enable correct method to be called
+ int c = skipLws(input);
if (c == '"') {
return readQuotedString(input, returnQuoted);
@@ -472,7 +475,9 @@
StringBuilder result = new StringBuilder();
boolean quoted = false;
- int c = skipLws(input, false);
+ skipLws(input);
+ input.mark(1);
+ int c = input.read();
if (c == '"') {
quoted = true;
@@ -481,10 +486,12 @@
} else {
result.append((char) c);
}
+ input.mark(1);
c = input.read();
while (c != -1 && isToken(c)) {
result.append((char) c);
+ input.mark(1);
c = input.read();
}
@@ -493,8 +500,9 @@
return null;
}
} else {
- // Skip back so non-token character is available for next read
- input.skip(-1);
+ // Use mark(1)/reset() rather than skip(-1) since skip() is a NOP
+ // once the end of the String has been reached.
+ input.reset();
}
if (c != -1 && result.length() == 0) {
@@ -523,7 +531,9 @@
StringBuilder result = new StringBuilder();
boolean quoted = false;
- int c = skipLws(input, false);
+ skipLws(input);
+ input.mark(1);
+ int c = input.read();
if (c == '"') {
quoted = true;
@@ -535,6 +545,7 @@
}
result.append((char) c);
}
+ input.mark(1);
c = input.read();
while (c != -1 && isHex(c)) {
@@ -542,6 +553,7 @@
c -= ('A' - 'a');
}
result.append((char) c);
+ input.mark(1);
c = input.read();
}
@@ -550,8 +562,9 @@
return null;
}
} else {
- // Skip back so non-hex character is available for next read
- input.skip(-1);
+ // Use mark(1)/reset() rather than skip(-1) since skip() is a NOP
+ // once the end of the String has been reached.
+ input.reset();
}
if (c != -1 && result.length() == 0) {
@@ -562,7 +575,8 @@
}
static double readWeight(Reader input, char delimiter) throws IOException {
- int c = skipLws(input, false);
+ skipLws(input);
+ int c = input.read();
if (c == -1 || c == delimiter) {
// No q value just whitespace
return 1;
@@ -572,7 +586,8 @@
return 0;
}
// RFC 7231 does not allow whitespace here but be tolerant
- c = skipLws(input, false);
+ skipLws(input);
+ c = input.read();
if (c != '=') {
// Malformed. Use quality of zero so it is dropped.
skipUntil(input, c, delimiter);
@@ -580,7 +595,8 @@
}
// RFC 7231 does not allow whitespace here but be tolerant
- c = skipLws(input, false);
+ skipLws(input);
+ c = input.read();
// Should be no more than 3 decimal places
StringBuilder value = new StringBuilder(5);
diff --git a/test/org/apache/tomcat/util/http/parser/TestAuthorizationDigest.java b/test/org/apache/tomcat/util/http/parser/TestAuthorizationDigest.java
index 0e28907..2f14841 100644
--- a/test/org/apache/tomcat/util/http/parser/TestAuthorizationDigest.java
+++ b/test/org/apache/tomcat/util/http/parser/TestAuthorizationDigest.java
@@ -126,6 +126,17 @@
}
@Test
+ public void testEndWithLhexReverse() throws Exception {
+ String header = "Digest nc=10000000";
+
+ StringReader input = new StringReader(header);
+
+ Map<String,String> result = Authorization.parseAuthorizationDigest(input);
+
+ Assert.assertEquals("10000000", result.get("nc"));
+ }
+
+ @Test
public void testQuotedLhex() throws Exception {
String header = "Digest nc=\"09abcdef\"";
@@ -137,6 +148,39 @@
}
@Test
+ public void testQuotedLhexReverse() throws Exception {
+ String header = "Digest nc=\"fedcba90\"";
+
+ StringReader input = new StringReader(header);
+
+ Map<String,String> result = Authorization.parseAuthorizationDigest(input);
+
+ Assert.assertEquals("fedcba90", result.get("nc"));
+ }
+
+ @Test
+ public void testLhex() throws Exception {
+ String header = "Digest nc=09abcdef";
+
+ StringReader input = new StringReader(header);
+
+ Map<String,String> result = Authorization.parseAuthorizationDigest(input);
+
+ Assert.assertEquals("09abcdef", result.get("nc"));
+ }
+
+ @Test
+ public void testLhexReverse() throws Exception {
+ String header = "Digest nc=fedcba90";
+
+ StringReader input = new StringReader(header);
+
+ Map<String,String> result = Authorization.parseAuthorizationDigest(input);
+
+ Assert.assertEquals("fedcba90", result.get("nc"));
+ }
+
+ @Test
public void testQuotedLhexUppercase() throws Exception {
String header = "Digest nc=\"00ABCDEF\"";
@@ -148,6 +192,39 @@
}
@Test
+ public void testQuotedLhexUppercaseReverse() throws Exception {
+ String header = "Digest nc=\"FEDCBA00\"";
+
+ StringReader input = new StringReader(header);
+
+ Map<String,String> result = Authorization.parseAuthorizationDigest(input);
+
+ Assert.assertEquals("fedcba00", result.get("nc"));
+ }
+
+ @Test
+ public void testLhexUppercase() throws Exception {
+ String header = "Digest nc=00ABCDEF";
+
+ StringReader input = new StringReader(header);
+
+ Map<String,String> result = Authorization.parseAuthorizationDigest(input);
+
+ Assert.assertEquals("00abcdef", result.get("nc"));
+ }
+
+ @Test
+ public void testLhexUppercaseReverse() throws Exception {
+ String header = "Digest nc=FEDCBA00";
+
+ StringReader input = new StringReader(header);
+
+ Map<String,String> result = Authorization.parseAuthorizationDigest(input);
+
+ Assert.assertEquals("fedcba00", result.get("nc"));
+ }
+
+ @Test
public void testUnclosedQuotedLhex() throws Exception {
String header = "Digest nc=\"00000001";
@@ -159,6 +236,28 @@
}
@Test
+ public void testEmptyLhex() throws Exception {
+ String header = "Digest nc=";
+
+ StringReader input = new StringReader(header);
+
+ Map<String,String> result = Authorization.parseAuthorizationDigest(input);
+
+ Assert.assertNull(result);
+ }
+
+ @Test
+ public void testQuotedEmptyLhex() throws Exception {
+ String header = "Digest nc=\"\"";
+
+ StringReader input = new StringReader(header);
+
+ Map<String,String> result = Authorization.parseAuthorizationDigest(input);
+
+ Assert.assertNull(result);
+ }
+
+ @Test
public void testUnclosedQuotedString1() throws Exception {
String header = "Digest username=\"test";
@@ -209,7 +308,17 @@
}
@Test
- public void testNonTokenQop() throws Exception {
+ public void testEmptyQuotedTokenQop() throws Exception {
+ String header = "Digest qop=\"\"";
+
+ StringReader input = new StringReader(header);
+
+ Map<String,String> result = Authorization.parseAuthorizationDigest(input);
+ Assert.assertNull(result);
+ }
+
+ @Test
+ public void testNonTokenQop01() throws Exception {
String header = "Digest qop=au{th";
StringReader input = new StringReader(header);
@@ -219,6 +328,16 @@
}
@Test
+ public void testNonTokenQop02() throws Exception {
+ String header = "Digest qop=auth{";
+
+ StringReader input = new StringReader(header);
+
+ Map<String,String> result = Authorization.parseAuthorizationDigest(input);
+ Assert.assertNull(result);
+ }
+
+ @Test
public void testQuotedNonTokenQop() throws Exception {
String header = "Digest qop=\"au{th\"";
@@ -279,7 +398,7 @@
}
@Test
- public void testWrongCharacterInHex() throws Exception {
+ public void testWrongCharacterInHex01() throws Exception {
String header = "Digest nc=\u044f";
StringReader input = new StringReader(header);
@@ -289,6 +408,26 @@
}
@Test
+ public void testWrongCharacterInHex02() throws Exception {
+ String header = "Digest nc=aaa\u044f";
+
+ StringReader input = new StringReader(header);
+
+ Map<String,String> result = Authorization.parseAuthorizationDigest(input);
+ Assert.assertNull(result);
+ }
+
+ @Test
+ public void testWrongCharacterInHex03() throws Exception {
+ String header = "Digest nc=\u044faaa";
+
+ StringReader input = new StringReader(header);
+
+ Map<String,String> result = Authorization.parseAuthorizationDigest(input);
+ Assert.assertNull(result);
+ }
+
+ @Test
public void testWrongCharacterInQuotedHex() throws Exception {
String header = "Digest nc=\"\u044f\"";
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 8046dba..343e420 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -127,6 +127,10 @@
Correctly handle a digest authorization header when the user name
contains an escaped character. (markt)
</fix>
+ <fix>
+ Correctly handle a digest authorization header when one of the hex
+ field values ends the header with in an invalid character. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
