Skip to content

Commit

Permalink
=htc akka#1120 Allow '=' in query param values in relaxed mode (akka#…
Browse files Browse the repository at this point in the history
  • Loading branch information
gmethvin authored and tomrf1 committed Aug 5, 2017
1 parent c84b267 commit f77f140
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,7 @@ ProblemFilters.exclude[DirectMissingMethodProblem]("akka.http.impl.settings.Clie

# Added new setting to @DoNotInherit class
ProblemFilters.exclude[ReversedMissingMethodProblem]("akka.http.scaladsl.settings.ClientConnectionSettings.localAddress")

# akka.http.impl.model.parser.CharacterClasses is private[http].
ProblemFilters.exclude[DirectMissingMethodProblem]("akka.http.impl.model.parser.CharacterClasses.relaxed-query-char")
ProblemFilters.exclude[DirectMissingMethodProblem]("akka.http.impl.model.parser.CharacterClasses.strict-query-char")
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,14 @@ private[http] object CharacterClasses {
val `pchar-base-nc` = unreserved ++ `sub-delims` ++ '@'
val `pchar-base` = `pchar-base-nc` ++ ':' // pchar without percent
val `query-fragment-char` = `pchar-base` ++ "/?"
val `strict-query-char` = `query-fragment-char` -- "&=;"
val `strict-query-char-np` = `strict-query-char` -- '+'
val `strict-query-key-char` = `query-fragment-char` -- "&=;"
val `strict-query-value-char` = `query-fragment-char` -- "&=;"
val `strict-query-char-np` = `strict-query-value-char` -- '+'

val `relaxed-fragment-char` = VCHAR -- '%'
val `relaxed-path-segment-char` = VCHAR -- "%/?#"
val `relaxed-query-char` = VCHAR -- "%&=#"
val `relaxed-query-key-char` = VCHAR -- "%&=#"
val `relaxed-query-value-char` = VCHAR -- "%&#"
val `raw-query-char` = VCHAR -- '#'
val `scheme-char` = ALPHA ++ DIGIT ++ '+' ++ '-' ++ '.'

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,13 @@ private[http] final class UriParser(val input: ParserInput,
case Uri.ParsingMode.Strict `pchar-base`
case _ `relaxed-path-segment-char`
}
private[this] val `query-char` = uriParsingMode match {
case Uri.ParsingMode.Strict `strict-query-char`
case Uri.ParsingMode.Relaxed `relaxed-query-char`
private[this] val `query-key-char` = uriParsingMode match {
case Uri.ParsingMode.Strict `strict-query-key-char`
case Uri.ParsingMode.Relaxed `relaxed-query-key-char`
}
private[this] val `query-value-char` = uriParsingMode match {
case Uri.ParsingMode.Strict `strict-query-value-char`
case Uri.ParsingMode.Relaxed `relaxed-query-value-char`
}
private[this] val `fragment-char` = uriParsingMode match {
case Uri.ParsingMode.Strict `query-fragment-char`
Expand Down Expand Up @@ -184,12 +188,13 @@ private[http] final class UriParser(val input: ParserInput,

// http://www.w3.org/TR/html401/interact/forms.html#h-17.13.4.1
def query: Rule1[Query] = {
def part = rule(
clearSBForDecoding() ~ oneOrMore('+' ~ appendSB(' ') | `query-char` ~ appendSB() | `pct-encoded`) ~ push(getDecodedString())
def part(`query-char`: CharPredicate) =
rule(clearSBForDecoding() ~
oneOrMore('+' ~ appendSB(' ') | `query-char` ~ appendSB() | `pct-encoded`) ~ push(getDecodedString())
| push(""))

def keyValuePair: Rule2[String, String] = rule {
part ~ ('=' ~ part | push(Query.EmptyValue))
part(`query-key-char`) ~ ('=' ~ part(`query-value-char`) | push(Query.EmptyValue))
}

// has a max value-stack depth of 3
Expand Down
23 changes: 6 additions & 17 deletions akka-http-core/src/test/java/akka/http/javadsl/model/UriTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,11 @@ public void testIllegalPathWithControlCharacter() {
@Test(expected = IllegalUriException.class)
public void testIllegalQuery() {
//#illegal-query
Uri.create("?a=b=c").query();
Uri.create("?a%b=c").query();
//IllegalUriException(
// "Illegal query: Invalid input '=', expected '+', query-char, 'EOI', '&' or pct-encoded (line 1, column 4)",
// "a=b=c\n" +
// " ^"
// " Illegal query: Invalid input '=', expected HEXDIG (line 1, column 4): a%b=c",
// "a%b=c\n" +
// " ^"
//)
//#illegal-query
}
Expand Down Expand Up @@ -264,24 +264,13 @@ public void testRelaxedMode() {
//#query-relaxed-mode-success
assertEquals(Query.create(Pair.create("a^", "b")), relaxed("a^=b"));
assertEquals(Query.create(Pair.create("a;", "b")), relaxed("a;=b"));
assertEquals(Query.create(Pair.create("a", "b=c")), relaxed("a=b=c"));
//#query-relaxed-mode-success
}

//#query-relaxed-mode-exception-1
@Test(expected = IllegalUriException.class)
public void testRelaxedModeException1() {
//double '=' in query string is invalid, even in relaxed mode
relaxed("a=b=c");
//IllegalUriException(
// "Illegal query: Invalid input '=', expected '+', query-char, 'EOI', '&' or pct-encoded (line 1, column 4)",
// "a=b=c\n" +
// " ^")
}
//#query-relaxed-mode-exception-1

//#query-relaxed-mode-exception-2
@Test(expected = IllegalUriException.class)
public void testRelaxedModeException2() {
//following '%', it should be percent encoding (HEXDIG), but "%b=" is not a valid percent encoding
//still invalid even in relaxed mode
relaxed("a%b=c");
Expand All @@ -290,6 +279,6 @@ public void testRelaxedModeException2() {
// "a%b=c\n" +
// " ^")
}
//#query-relaxed-mode-exception-2
//#query-relaxed-mode-exception-1

}
Original file line number Diff line number Diff line change
Expand Up @@ -382,16 +382,10 @@ class UriSpec extends WordSpec with Matchers {
//#query-relaxed-mode-success
relaxed("a^=b") shouldEqual ("a^", "b") +: Query.Empty
relaxed("a;=b") shouldEqual ("a;", "b") +: Query.Empty
relaxed("a=b=c") shouldEqual ("a", "b=c") +: Query.Empty
//#query-relaxed-mode-success

//#query-relaxed-mode-exception
//double '=' in query string is invalid, even in relaxed mode
the[IllegalUriException] thrownBy relaxed("a=b=c") shouldBe {
IllegalUriException(
"Illegal query: Invalid input '=', expected '+', query-char, 'EOI', '&' or pct-encoded (line 1, column 4)",
"a=b=c\n" +
" ^")
}
//following '%', it should be percent encoding (HEXDIG), but "%b=" is not a valid percent encoding
//still invalid even in relaxed mode
the[IllegalUriException] thrownBy relaxed("a%b=c") shouldBe {
Expand Down Expand Up @@ -615,14 +609,6 @@ class UriSpec extends WordSpec with Matchers {
" ^")
}
//#illegal-cases-immediate-exception

// illegal query
the[IllegalUriException] thrownBy Uri("?a=b=c").query() shouldBe {
IllegalUriException(
"Illegal query: Invalid input '=', expected '+', query-char, 'EOI', '&' or pct-encoded (line 1, column 4)",
"a=b=c\n" +
" ^")
}
}

// http://tools.ietf.org/html/rfc3986#section-5.4
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,15 @@ public void testStringParameterExtraction() {
.assertStatusCode(200)
.assertEntity("john");

route
.run(HttpRequest.create().withUri("/abc?stringParam=a%b"))
.assertStatusCode(400)
.assertEntity("The request content was malformed:\nThe request's query string is invalid: stringParam=a%b");

route
.run(HttpRequest.create().withUri("/abc?stringParam=a=b"))
.assertStatusCode(400)
.assertEntity("The request content was malformed:\nThe request's query string is invalid: stringParam=a=b");
.assertStatusCode(200)
.assertEntity("a=b");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@ class ParameterDirectivesSpec extends FreeSpec with GenericRoutingSpec with Insi
}
}
"cause a MalformedRequestContentRejection on invalid query strings" in {
Get("/?amount=1=") ~> {
Get("/?amount=1%2") ~> {
parameter("amount".as[Int].?) { echoComplete }
} ~> check {
inside(rejection) {
case MalformedRequestContentRejection("The request's query string is invalid: amount=1=", _)
case MalformedRequestContentRejection("The request's query string is invalid: amount=1%2", _)
}
}
}
Expand Down
2 changes: 0 additions & 2 deletions docs/src/main/paradox/java/http/common/uri-model.md
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,6 @@ However, even with the `Relaxed` mode, there are still invalid special character

@@snip [UriTest.java](../../../../../../../akka-http-core/src/test/java/akka/http/javadsl/model/UriTest.java) { #query-relaxed-mode-exception-1 }

@@snip [UriTest.java](../../../../../../../akka-http-core/src/test/java/akka/http/javadsl/model/UriTest.java) { #query-relaxed-mode-exception-2 }

Other than specifying the `mode` in the parameters, like when using directives, you can specify the `mode` in your configuration as follows.

```
Expand Down

0 comments on commit f77f140

Please sign in to comment.