Fix `JavaHelpers.updateRequestWithUri` by ihostage · Pull Request #12977 · playframework/playframework
Sergey, I am sorry I hold this back so long. I was taking a look again and I will just merge it. I will add another PR with the things that I discovered - reason is that we have various places where we build the query map:
| override val queryMap: Map[String, Seq[String]] = { | |
| val query: String = uri.getRawQuery | |
| if (query == null || query.length == 0) { | |
| Map.empty | |
| } else { | |
| query.split("&").foldLeft[Map[String, Seq[String]]](Map.empty) { | |
| case (acc, pair) => | |
| val idx: Int = pair.indexOf("=") | |
| val key: String = if (idx > 0) URLDecoder.decode(pair.substring(0, idx), "UTF-8") else pair | |
| val value: String = | |
| if (idx > 0 && pair.length > idx + 1) URLDecoder.decode(pair.substring(idx + 1), "UTF-8") else null | |
| acc.get(key) match { | |
| case None => acc.updated(key, Seq(value)) | |
| case Some(values) => acc.updated(key, values :+ value) | |
| } | |
| } | |
| } | |
| } |
| override lazy val queryMap: Map[String, Seq[String]] = FormUrlEncodedParser.parse(queryString) |
| override lazy val queryMap: Map[String, Seq[String]] = FormUrlEncodedParser.parse(queryString) |
| override lazy val queryMap: Map[String, Seq[String]] = { | |
| // Very rough parse of query string that doesn't decode | |
| if (requestUri.contains("?")) { | |
| requestUri | |
| .split("\\?", 2)(1) | |
| .split('&') | |
| .map { keyPair => | |
| keyPair.split("=", 2) match { | |
| case Array(key) => key -> "" | |
| case Array(key, value) => key -> value | |
| } | |
| } | |
| .groupBy(_._1) | |
| .map { | |
| case (name, values) => name -> values.map(_._2).toSeq | |
| } | |
| } else { | |
| Map.empty | |
| } | |
| } |
And the server backends itself:
| override lazy val queryMap: Map[String, Seq[String]] = { | |
| try { | |
| request.uri.query(mode = Uri.ParsingMode.Relaxed).toMultiMap | |
| } catch { | |
| case NonFatal(e) => | |
| logger.warn("Failed to parse query string; returning empty map.", e) | |
| Map.empty | |
| } | |
| } |
| override val queryMap: Map[String, Seq[String]] = { | |
| val decoder = new QueryStringDecoder(parsedQueryString) | |
| try { | |
| decoder.parameters().asScala.view.mapValues(_.asScala.toList).toMap | |
| } catch { | |
| case iae: IllegalArgumentException if iae.getMessage.startsWith("invalid hex byte") => throw iae | |
| case NonFatal(e) => | |
| logger.warn("Failed to parse query string; returning empty map.", e) | |
| Map.empty | |
| } | |
| } |
and maybe even more.
So when I looked at your pr I did some stupid testing and it turned out some of them behave slightly different in edge cases.
So I tought I will add more tests and fix it on top of your changes... Anyway so I have some changes lying around that I will maybe soon commit. Again it's just edge cases. Also I was thinking why can we not just re-use FormUrlEncodedParser.parse where possible instead of having different implementation.
Sorry Sergey for blocking you so long with this. My changes come later.