You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2018/05/30 13:40:17 UTC
svn commit: r1832545 - in /tomcat/trunk:
java/org/apache/tomcat/util/http/parser/HttpParser.java
test/org/apache/tomcat/util/http/parser/TestAuthorizationDigest.java
webapps/docs/changelog.xml
Author: markt
Date: Wed May 30 13:40:16 2018
New Revision: 1832545
URL: http://svn.apache.org/viewvc?rev=1832545&view=rev
Log:
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.
Modified:
tomcat/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
tomcat/trunk/test/org/apache/tomcat/util/http/parser/TestAuthorizationDigest.java
tomcat/trunk/webapps/docs/changelog.xml
Modified: tomcat/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java?rev=1832545&r1=1832544&r2=1832545&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java Wed May 30 13:40:16 2018
@@ -317,38 +317,36 @@ public class HttpParser {
}
- // 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)) {
@@ -366,14 +364,18 @@ public class HttpParser {
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;
@@ -389,7 +391,8 @@ public class HttpParser {
*/
static String readQuotedString(Reader input, boolean returnQuoted) throws IOException {
- int c = skipLws(input, false);
+ skipLws(input);
+ int c = input.read();
if (c != '"') {
return null;
@@ -425,8 +428,8 @@ public class HttpParser {
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);
@@ -452,7 +455,9 @@ public class HttpParser {
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;
@@ -461,10 +466,12 @@ public class HttpParser {
} 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();
}
@@ -473,8 +480,9 @@ public class HttpParser {
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) {
@@ -503,7 +511,9 @@ public class HttpParser {
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;
@@ -515,6 +525,7 @@ public class HttpParser {
}
result.append((char) c);
}
+ input.mark(1);
c = input.read();
while (c != -1 && isHex(c)) {
@@ -522,6 +533,7 @@ public class HttpParser {
c -= ('A' - 'a');
}
result.append((char) c);
+ input.mark(1);
c = input.read();
}
@@ -530,8 +542,9 @@ public class HttpParser {
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) {
@@ -542,7 +555,8 @@ public class HttpParser {
}
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;
@@ -552,7 +566,8 @@ public class HttpParser {
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);
@@ -560,7 +575,8 @@ public class HttpParser {
}
// 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);
Modified: tomcat/trunk/test/org/apache/tomcat/util/http/parser/TestAuthorizationDigest.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/util/http/parser/TestAuthorizationDigest.java?rev=1832545&r1=1832544&r2=1832545&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/tomcat/util/http/parser/TestAuthorizationDigest.java (original)
+++ tomcat/trunk/test/org/apache/tomcat/util/http/parser/TestAuthorizationDigest.java Wed May 30 13:40:16 2018
@@ -126,6 +126,17 @@ public class TestAuthorizationDigest {
}
@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 @@ public class TestAuthorizationDigest {
}
@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 @@ public class TestAuthorizationDigest {
}
@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 @@ public class TestAuthorizationDigest {
}
@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";
@@ -229,7 +328,17 @@ public class TestAuthorizationDigest {
}
@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);
@@ -239,6 +348,16 @@ public class TestAuthorizationDigest {
}
@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\"";
@@ -299,11 +418,31 @@ public class TestAuthorizationDigest {
}
@Test
- public void testWrongCharacterInHex() throws Exception {
+ public void testWrongCharacterInHex01() throws Exception {
String header = "Digest nc=\u044f";
StringReader input = new StringReader(header);
+ Map<String,String> result = Authorization.parseAuthorizationDigest(input);
+ Assert.assertNull(result);
+ }
+
+ @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);
}
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1832545&r1=1832544&r2=1832545&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Wed May 30 13:40:16 2018
@@ -165,6 +165,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">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org